Re: [PATCH resend] powerpc/code-patching: Avoid r/w mapping of the zero page

["Christophe Leroy (CS GROUP)" <chleroy@kernel.org>]

Le 20/05/2026 à 11:40, Ard Biesheuvel a écrit :
> 
> On Wed, 20 May 2026, at 11:36, Christophe Leroy (CS GROUP) wrote:
>> Le 20/05/2026 à 10:54, Ard Biesheuvel a écrit :
>>> The only remaining use of map_patch_area() is mapping the zero page, and
>>> immediately unmapping it again so that the intermediate page table
>>> levels are all guaranteed to be populated.
>>>
>>> The use of the zero page here is completely arbitrary, and not harmful
>>> per se, but currently, it creates a writable mapping, and does so in a
>>> manner that requires that the empty_zero_page[] symbol is not
>>> const-qualified.
>>>
>>> Given that this is about to change, and that map_patch_area() now never
>>> maps anything other than the zero page, let's simplify the code and
>>> - take the PA of empty_zero_page directly
>>> - create a read-only temporary mapping.
>>>
>>> This allows empty_zero_page[] to be repainted as const u8[] in a
>>> subsequent patch, without making substantial changes to this code
>>> patching logic.
>>>
>>> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: Nicholas Piggin <npiggin@gmail.com>
>>> Cc: "Christophe Leroy (CS GROUP)" <chleroy@kernel.org>
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> ---
>>> Build tested only (Clang)
>>>
>>> Resending from my non-Google email. Apologies if you are receiving this
>>> twice.
>>>
>>>    arch/powerpc/lib/code-patching.c | 11 +++++------
>>>    1 file changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
>>> index f84e0337cc02..13a8acf851f1 100644
>>> --- a/arch/powerpc/lib/code-patching.c
>>> +++ b/arch/powerpc/lib/code-patching.c
>>> @@ -60,7 +60,7 @@ struct patch_context {
>>>    
>>>    static DEFINE_PER_CPU(struct patch_context, cpu_patching_context);
>>>    
>>> -static int map_patch_area(void *addr, unsigned long text_poke_addr);
>>> +static int map_patch_area(unsigned long text_poke_addr);
>>>    static void unmap_patch_area(unsigned long addr);
>>>    
>>>    static bool mm_patch_enabled(void)
>>> @@ -117,7 +117,7 @@ static int text_area_cpu_up(unsigned int cpu)
>>>    
>>>    	// Map/unmap the area to ensure all page tables are pre-allocated
>>>    	addr = (unsigned long)area->addr;
>>> -	err = map_patch_area(empty_zero_page, addr);
>>> +	err = map_patch_area(addr);
>>
>> I would get rid of map_patch_area() completely and just do:
>>
>> 	err = map_kernel_page(addr, __pa_symbol(empty_zero_page), PAGE_KERNEL_RO);
>>
> 
> I think retaining the symmetry of map_patch_area() and unmap_patch_area()
> makes sense too.

Could also drop unmap_patch_area() and use unmap_kernel_page() instead.

> 
>>
>> By the way I don't know how vital it is to map in read-only but in case
>> it is see commit da3a3b0a0e38 ("powerpc/32s: map kasan zero shadow with
>> PAGE_READONLY instead of PAGE_KERNEL_RO")
>>
> 
> In the end, it doesn't make any difference, given that the mapping is removed
> again immediately. It is just neater to use _RO to map a const object, rather
> than explicitly creating a read-write mapping of something that should really
> never be written to.
> 

Ok