Message-ID: <678f6b3f-8d1a-4253-8125-28b8ee89842b@arm.com>
Date: Tue, 27 Jan 2026 11:00:14 +0000
From: Ryan Roberts <ryan.roberts@....com>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 will@...nel.org, catalin.marinas@....com, mark.rutland@....com,
 Anshuman Khandual <anshuman.khandual@....com>,
 Liz Prucka <lizprucka@...gle.com>, Seth Jenkins <sethjenkins@...gle.com>,
 Kees Cook <kees@...nel.org>, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2 08/10] arm64: mm: Don't abuse memblock NOMAP to check
 for overlaps

On 27/01/2026 10:47, Ard Biesheuvel wrote:
> On Tue, 27 Jan 2026 at 11:39, Ryan Roberts <ryan.roberts@....com> wrote:
>>
>> On 27/01/2026 10:27, Ard Biesheuvel wrote:
>>> On Tue, 27 Jan 2026 at 11:21, Ryan Roberts <ryan.roberts@....com> wrote:
>>>>
>>>> On 26/01/2026 09:26, Ard Biesheuvel wrote:
>>>>> From: Ard Biesheuvel <ardb@...nel.org>
>>>>>
>>>>> Now that the DRAM mapping routines respect existing table mappings and
>>>>> contiguous block and page mappings, it is no longer needed to fiddle
>>>>> with the memblock tables to set and clear the NOMAP attribute. Instead,
>>>>> map the kernel text and rodata alias first, avoiding contiguous
>>>>> mappings, so that they will not be added later when mapping the
>>>>> memblocks.
>>>>
>>>> Should we do something similar for kfence? Currently we have
>>>> arm64_kfence_alloc_pool() which marks some memory NOMAP then
>>>> arm64_kfence_map_pool() which PTE-maps it and clears NOMAP. Presumably we could
>>>> rationalize into a single function that does it all, prior to mapping the bulk
>>>> of the linear map?
>>>>
>>>
>>> Yeah, good point - I did not spot that but I will address it in the
>>> next revision.
>>>
>>>>>
>>>>> Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
>>>>> ---
>>>>>  arch/arm64/mm/mmu.c | 27 ++++++++------------
>>>>>  1 file changed, 10 insertions(+), 17 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>>>> index 80587cd47ce7..18415d4743bf 100644
>>>>> --- a/arch/arm64/mm/mmu.c
>>>>> +++ b/arch/arm64/mm/mmu.c
>>>>> @@ -1149,12 +1149,17 @@ static void __init map_mem(void)
>>>>>               flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>>>>>
>>>>>       /*
>>>>> -      * Take care not to create a writable alias for the
>>>>> -      * read-only text and rodata sections of the kernel image.
>>>>> -      * So temporarily mark them as NOMAP to skip mappings in
>>>>> -      * the following for-loop
>>>>> +      * Map the linear alias of the [_text, __init_begin) interval
>>>>> +      * as non-executable now, and remove the write permission in
>>>>> +      * mark_linear_text_alias_ro() above (which will be called after
>>>>> +      * alternative patching has completed). This makes the contents
>>>>> +      * of the region accessible to subsystems such as hibernate,
>>>>> +      * but protects it from inadvertent modification or execution.
>>>>> +      * Note that contiguous mappings cannot be remapped in this way,
>>>>> +      * so we should avoid them here.
>>>>>        */
>>>>> -     memblock_mark_nomap(kernel_start, kernel_end - kernel_start);
>>>>> +     __map_memblock(kernel_start, kernel_end, PAGE_KERNEL,
>>>>> +                    flags | NO_CONT_MAPPINGS);
>>>>
>>>> So the reason to disallow cont mappings is because we need to modify the
>>>> permissions later? It _is_ safe to change permissions on a live contiguous
>>>> mapping in this way. That was clarified in the architecture a couple of years
>>>> back and we rely on it for contpte_wrprotect_ptes(); see comment there.
>>>>
>>>
>>> OK, good to know - I was hoping to get your take on this ...
>>>
>>>> I think we could relax this?
>>>>
>>>
>>> OK, I suppose that means that we can drop the NO_CONT_MAPPINGS here,
>>> but we still need to map the kernel text/rodata alias initially to
>>> ensure that no block mappings are created that would need to broken
>>> down, right?
>>
>> Yes, but...
>>
>> I think your intent is that the multiple __map_memblock() calls are just
>> controlling the allowed leaf mapping sizes.
> 
> Indeed.
> 
>> It becomes problematic if the 2
>> calls use different permissions... which they do.
>>
>> PAGE_KERNEL vs pgprot_tagged(PAGE_KERNEL).
>>
>> Is it possible that the text/rodata section ends up tagged, which is not intended?
>>
> 
> 
> OK so toggling the R/O attribute on a live contiguous mapping is
> permitted (provided that ultimately, the entire contiguous region is
> updated accordingly) but the same doesn't apply to MT_NORMAL vs
> MT_NORMAL_TAGGED, right?

This is the rule:

RJQQTC
For a TLB lookup in a contiguous region mapped by translation table entries that
have consistent values for the Contiguous bit, but have the OA, attributes, or
permissions misprogrammed, that TLB lookup is permitted to produce an OA, access
permissions, and memory attributes that are consistent with any one of the
programmed translation table values.

So, yes, it is fine to modify the attributes (and I assume the memory type
counts as attributes) on the live mapping...

> 
> So let's use just the same prot and flags for the initial mapping, and
> use MT_NORMAL_TAGGED when we remap the text alias R/O.

...but that wasn't my point. I belive the intent is that all of the linear map
uses MT_NORMAL_TAGGED except for the kernel text and rodata, which uses
MT_NORMAL. In some configs (e.g. force_pte_mapping() == true), your change will
result in all the linear map, including text and rodata being MT_NORMAL_TAGGED.

Thanks,
Ryan

Powered by blists - more mailing lists