Re: [PATCH bpf-next 5/5] x86/bpf: enable EXECMEM_ROX_CACHE for BPF allocations

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Pages freed from the ROX cache via vfree() are returned to the page allocator with Read-Only permissions, causing kernel panics on subsequent allocations.
- [High] The return value of execmem_restore_rox() is ignored in __execmem_cache_free(), potentially leaving cached memory as Non-Executable (NX) and crashing the kernel when BPF programs are executed.
--

commit d59aa3f12b249d60a053835fa723486bb4d4f203
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

x86/bpf: enable EXECMEM_ROX_CACHE for BPF allocations
    
BPF core and x86 JIT use text poking and temporary writable buffers and
thus can handle ROX memory. Enable ROX cache for EXECMEM_BPF when
configuration and CPU features allow that.

> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index fb67217fddcd3..079f8c7e9e3cd 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -1107,10 +1107,10 @@ struct execmem_info __init *execmem_arch_setup(void)
>  				.alignment = MODULE_ALIGN,
>  			},
>  			[EXECMEM_BPF] = {
> -				.flags	= EXECMEM_KASAN_SHADOW,
> +				.flags	= flags,
>  				.start	= start,
>  				.end	= MODULES_END,
> -				.pgprot	= PAGE_KERNEL,
> +				.pgprot	= pgprot,

[Severity: Critical]
This is a pre-existing issue, but enabling the ROX cache for BPF
significantly increases the likelihood of triggering it. When memory is freed
back to the page allocator via vfree() from the ROX cache, it appears to
retain Read-Only permissions.

In mm/execmem.c:execmem_cache_populate_alloc(), memory is allocated without
the VM_FLUSH_RESET_PERMS flag:

    unsigned long vm_flags = VM_ALLOW_HUGE_VMAP;
    ...
    p = execmem_vmalloc(range, alloc_size, PAGE_KERNEL, vm_flags);

When the block is later cleaned up in mm/execmem.c:execmem_cache_clean(), it
is freed using vfree():

    execmem_set_direct_map_valid(vm, true);
    mas_store_gfp(&mas, NULL, GFP_KERNEL);
    vfree(area);

Since VM_FLUSH_RESET_PERMS was omitted, vfree() skips resetting the direct map
permissions. If a subsequent kernel subsystem allocates these same pages from
the buddy allocator and attempts to write to them, will this result in a Write
Protection fault and kernel panic?

[Severity: High]
This is also a pre-existing issue exposed by enabling the ROX cache. In the
error handling path, the return value of execmem_restore_rox() is ignored.

Looking at mm/execmem.c:__execmem_cache_free():

    execmem_fill_trapping_insns(ptr, size);
    execmem_restore_rox(ptr, size);
    
    err = execmem_cache_add_locked(ptr, size, gfp_mask);

If execmem_restore_rox() fails (for example, if set_memory_rox() returns
ENOMEM when allocating page tables for direct map splitting), the memory is
left as Non-Executable (NX).

Since this error is ignored and the memory is still added to the free list,
could BPF subsequently allocate this NX memory from the cache, assuming it is
executable, and crash the system when attempting to execute JIT code?

>  				.alignment = MODULE_ALIGN,
>  			},
>  			[EXECMEM_MODULE_DATA] = {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-execmem-x86-rox-bpf-v0-v1-0-45a0b0ed4fe9@kernel.org?part=5