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

[Mike Rapoport <rppt@kernel.org>]

On Fri, Jun 26, 2026 at 06:59:44PM +0000, sashiko-bot@kernel.org wrote:
> 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?

The call to execmem_set_direct_map_valid() resets the direct map
permissions so there's no need to reset them in vfree().

> [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).

set_memory_rox() cannot fail here.

The only failure path in set_memory is an allocation failure when there is
a splitting of large pages. At this point a 2M page would be already split
by execmem_alloc_rw() so execmem_restore_rox() updates permissions on 4k
pages and collapses them into 2M page once the entire 2M page is ROX.
     
> 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

-- 
Sincerely yours,
Mike.