Re: [RFC PATCH 2/9] mm: execmem: Allow huge vmappings to be avoided for execmem caches

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Omitting VM_ALLOW_HUGE_VMAP breaks the ROX cache shrinker by stripping PMD_SIZE alignment, causing a permanent memory leak of cached module memory.

Pre-existing issues:
- [High] Fallback allocations in execmem_cache_populate_alloc() leak memory due to failing the PMD_SIZE alignment checks in the cache shrinker.
- [High] Ignored return value of execmem_restore_rox() allows memory to be silently returned to the ROX cache with RW permissions.
--

--- Patch [2]: [RFC PATCH 2/9] mm: execmem: Allow huge vmappings to be avoided for execmem caches ---
commit 3d4b59f7ca74d71807be337626018d273686c686
Author: Ard Biesheuvel <ardb@kernel.org>

mm: execmem: Allow huge vmappings to be avoided for execmem caches
    
Only recent arm64 systems are guaranteed to be able to manipulate the
permissions on live kernel mappings that may use huge mappings. Older
ones can only do so on live mappings that are mapped down to pages.

In order to make execmem caches work on arm64 despite this distinction,
allow huge vmappings to be omitted when allocating the caches.

> diff --git a/mm/execmem.c b/mm/execmem.c
> index 6db0c1d82a9d6..5790d4a8532af 100644
> --- a/mm/execmem.c
> +++ b/mm/execmem.c
> @@ -276,7 +276,8 @@ static void *__execmem_cache_alloc(struct execmem_range *range, size_t size)
>  
>  static void *execmem_cache_populate_alloc(struct execmem_range *range, size_t size)
>  {
> -	unsigned long vm_flags = VM_ALLOW_HUGE_VMAP;
> +	unsigned long vm_flags = (range->flags & EXECMEM_NO_HUGE_VMAP)
> +						? 0 : VM_ALLOW_HUGE_VMAP;

[Severity: High]
Does dropping VM_ALLOW_HUGE_VMAP here cause a permanent memory leak in the
ROX cache?

When VM_ALLOW_HUGE_VMAP is omitted, the allocation may fall back to the
standard alignment (typically PAGE_SIZE) instead of being PMD_SIZE aligned. 

However, the shrinker in execmem_cache_clean() strictly requires PMD_SIZE
alignment to free the memory:

mm/execmem.c:execmem_cache_clean() {
	...
	mas_for_each(&mas, area, ULONG_MAX) {
		size_t size = mas_range_len(&mas);

		if (IS_ALIGNED(size, PMD_SIZE) &&
		    IS_ALIGNED(mas.index, PMD_SIZE)) {
			struct vm_struct *vm = find_vm_area(area);
	...
}

If the starting address (mas.index) of the chunk allocated without
VM_ALLOW_HUGE_VMAP is not PMD_SIZE aligned, the shrinker will skip it,
never returning it to the vmalloc subsystem.

>  	struct mutex *mutex = &execmem_cache.mutex;
>  	struct vm_struct *vm;
>  	size_t alloc_size;
> 	int err = -ENOMEM;
> 	void *p;
> 
> 	alloc_size = round_up(size, PMD_SIZE);
> 	p = execmem_vmalloc(range, alloc_size, PAGE_KERNEL, vm_flags);
> 	if (!p) {
> 		alloc_size = size;
> 		p = execmem_vmalloc(range, alloc_size, PAGE_KERNEL, vm_flags);

[Severity: High]
This is a pre-existing issue, but does this exact-size fallback allocation
leak memory for the same reason?

If the PMD_SIZE rounded allocation fails under pressure, the fallback
allocates the exact requested size. Since this fallback size is not
guaranteed to be PMD_SIZE aligned, it will also permanently fail the
IS_ALIGNED(size, PMD_SIZE) check in execmem_cache_clean(), leaking the
vmalloc space.

[ ... ]

> static int __execmem_cache_free(struct ma_state *mas, void *ptr, gfp_t gfp_mask)
> {
> 	size_t size = mas_range_len(mas);
> 	int err;
> 
> 	err = execmem_force_rw(ptr, size);
> 	if (err)
> 		return err;
> 
> 	execmem_fill_trapping_insns(ptr, size);
> 	execmem_restore_rox(ptr, size);
> 
> 	err = execmem_cache_add_locked(ptr, size, gfp_mask);

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to ignore the
return value of execmem_restore_rox() here?

If restoring the ROX permissions fails (for example, if set_memory_rox() fails
internally), the chunk is still added back to the free_areas tree. Subsequent
allocations could then receive memory that is still writable, silently breaking
the W^X security guarantee.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822135323.795946-11-ardb+git@google.com?part=2