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

[Mike Rapoport <rppt@kernel.org>]

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.