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

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

On Fri, Jun 19, 2026 at 09:33:18AM +0100, Ryan Roberts wrote:
>On 18/06/2026 16:05, Ryan Roberts wrote:
>> On 11/06/2026 14:01, Adrian Barnaś 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));
>>
>> This is not sufficient to convert an invalid entry to valid. As well as setting
>> the PTE_VALID bit, you would also need to clear the PTE_PRESENT_INVALID and set
>> PTE_MAYBE_NG.
>>
>> e.g:
>>
>> int set_memory_valid(unsigned long addr, int numpages, int enable)
>> {
>> 	if (enable)
>> 		return __change_memory_common(addr, PAGE_SIZE * numpages,
>> 					__pgprot(PTE_PRESENT_VALID_KERNEL),
>> 					__pgprot(PTE_PRESENT_INVALID));
>>

Thanks, I will fix that

>>
>>> +}
>>> +
>>>  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. 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);
>>
>> Surely execmem should just be using set_direct_map_default_noflush() if that's
>> the behaviour it wants?
>>
>> I think that the current implementation of set_direct_map_default_noflush()
>> doesn't undo the effects of set_memory_nx() / set_memory_x(). That might be
>> worth checking?
>
>It's also worth mentioning that set_direct_map_valid_noflush() has "noflush" in
>the name, implies it doesn't expect/require any TLB flushing to occur. But the
>implementation will perform tlb flushing for any case that is not just a
>invalid->valid transition (which for the existing impl is the case when
>valid=true and for your changes is never the case - see __change_memory_common).
>
>But execmem doesn't do any tlb flushing so it looks to me like it actually
>requires that set_direct_map_valid_noflush() handles the tlb flushing? All seems
>a bit fishy and probably warrants a cleanup to make things clearer.
>

I think that the clean approach would be to have the `set_direct_map_default`
function (without `noflush`) that does flushing as needed (like the current
one on ARM64). I am not entirely sure but x86 seems to handle permission
faults gracefully while on ARM64 those would cause panics. Is that correct?


>>
>> Thanks,
>> Ryan
>>
>>
>>>  }
>>>
>>>  #ifdef CONFIG_DEBUG_PAGEALLOC
>>
>


Thanks,
Adrian