Re: [RFC PATCH 3/6] arm64: mm: fix restoring linear map permissions on execmem cache clean

["Adrian Barnaś" <abarnas@google.com>]

On Fri, Jun 12, 2026 at 10:17:55AM +0300, Mike Rapoport wrote:
>On Thu, Jun 11, 2026 at 01:54:13PM +0000, Brendan Jackman wrote:
>> On Thu Jun 11, 2026 at 1:01 PM UTC, =?UTF-8?q?Adrian=20Barna=C5=9B?= wrote:
>> > Strip the read-only attribute from the selected memory range when
>> > restoring the linear map after an execmem cache clean.
>> >
>> > An execmem cache clean is performed when a cache block becomes empty
>> > after unloading a module. When making the memory valid again, the linear
>> > memory alias must also have its read-only attribute cleared.
>> >
>> > Without this change, the linear memory alias remains read-only even
>> > after the execmem cache block itself is freed, which prevents subsequent
>> > allocations from writing to that memory.
>> >
>> > Signed-off-by: Adrian Barnaś <abarnas@google.com>
>> > ---
>> >  arch/arm64/mm/pageattr.c | 17 ++++++++++++++++-
>> >  1 file changed, 16 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
>> > index 88720bbba892..eaefdf90b0d5 100644
>> > --- a/arch/arm64/mm/pageattr.c
>> > +++ b/arch/arm64/mm/pageattr.c
>> > @@ -239,6 +239,13 @@ int set_memory_x(unsigned long addr, int numpages)
>> >  					__pgprot(PTE_PXN));
>> >  }
>> >
>> > +static int set_memory_default(unsigned long addr, int numpages)
>> > +{
>> > +	return __change_memory_common(addr, PAGE_SIZE * numpages,
>> > +				      __pgprot(PTE_VALID),
>> > +				      __pgprot(PTE_RDONLY));
>> > +}
>> > +
>> >  int set_memory_valid(unsigned long addr, int numpages, int enable)
>> >  {
>> >  	if (enable)
>> > @@ -362,7 +369,15 @@ int set_direct_map_valid_noflush(struct page *page, unsigned nr, bool valid)
>> >  	if (!can_set_direct_map())
>> >  		return 0;
>> >
>> > -	return set_memory_valid(addr, nr, valid);
>> > +	/*
>> > +	 * Execmem cache uses this function to reset permissions on linear mapping
>> > +	 * when freeing unused cache block. On x86 it makes memory RW which is
>> > +	 * desirable.
>>
>> Hm, maybe desirable for execmem but that doesn't really mean the x86
>> behaviour is correct. Maybe it makes more sense to change the x86
>> to align with the arm64 behaviour here?
>>
>> BTW we should probably document this API a little bit, I never thought
>> abut what "valid" actually means until now. I had thought of it as "I
>> can access this memory" but that's an unclear concept and now I realise
>> "valid" is a technical concept in Arm that's confusing. And it's extra
>> confusing if the kernel API uses "valid" to mean a _different_ thing.
>
>I've got confused too and that's how set_direct_map_valid() got into x86
>with a different semantics than on arm64.
>
>What execmem really needs is set_direct_map_default() variant that gets
>nr_pages.
>
>AFAIR, set_direct_map_default() has a single 'page' parameter because it
>was added to reset permissions for the direct map alias for vmalloc()'ed
>pages before there was VMALLOC_HUGE and each page had to be reset
>independently anyway.
>
>Maybe it's time to add nr_pages to set_direct_map_valid().
>
>> Also, shouldn't execmem be using set_memory_default_noflush() before
>> freeing anyway? I guess that shoudl even be documented as "if you change
>> anything you need to call this before you free the page".
>
>It does on x86 because there set_direct_map_valid() is the same as
>set_direct_map_default().
>
>> > On ARM64 set_memory_valid() just change valid bit which
>> > +	 * leave direct mapping read-only so use set_memory_default instead.
>> > +	 */
>> > +
>> > +	return valid ? set_memory_default(addr, nr) :
>> > +		       set_memory_valid(addr, nr, false);
>> >  }
>> >
>> >  #ifdef CONFIG_DEBUG_PAGEALLOC
>>
>>
>
>-- 
>Sincerely yours,
>Mike.

Hi Mike, Brendan,

Thanks for taking a look at the RFC.

I was also quite confused by this initially. I spent some time debugging 
until I realized why unloading all the modules was causing the kernel to 
crash.

The reason I took this approach was that I wanted to send out a working 
prototype for arm64 that wouldn't interfere with the existing, working 
implementation on x86.

Following your suggestion, I can put together a preparatory patch series 
to refactor the set_direct_map_* APIs to accept a nr_pages parameter. 
This refactoring would also allow us to drop the redundant 
set_area_direct_map helper. I could then rebase the rox_cache series on 
top of that.

Does this sound like a good path forward?

Thanks,
Adrian