Re: [PATCH bpf-next v2 4/5] bpf, x86: make sure allocation in arch_bpf_trampoline_size() is writable

[Mike Rapoport <rppt@kernel.org>]

On Sat, Jul 11, 2026 at 10:02:45AM +0000, sashiko-bot@kernel.org wrote:
>
> bpf, x86: make sure allocation in arch_bpf_trampoline_size() is writable
>     
> arch_bpf_trampoline_size() allocates a buffer to get actual size required
> for a trampoline.
>     
> This buffer must be in the module address space because
> __arch_prepare_bpf_trampoline() calculates rel32 offsets relatively to
> that buffer.
> 
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > index de7515ea1beae..b2feec81e2312 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -3703,13 +3703,12 @@ int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> >  	int ret;
> >  
> >  	/* Allocate a temporary buffer for __arch_prepare_bpf_trampoline().
> > -	 * This will NOT cause fragmentation in direct map, as we do not
> > -	 * call set_memory_*() on this buffer.
> >  	 *
> >  	 * We cannot use kvmalloc here, because we need image to be in
> >  	 * module memory range.
> > +	 * Since it must be writable use bpf_jit_alloc_exec_rw().
> >  	 */
> > -	image = bpf_jit_alloc_exec(PAGE_SIZE);
> > +	image = bpf_jit_alloc_exec_rw(PAGE_SIZE);
> 
> [Severity: Medium]
> Does this allocation cause unnecessary page table modifications and hugepage 
> shattering?

No.
 
> Allocating via bpf_jit_alloc_exec_rw() forces page table permission changes
> (set_memory_nx and set_memory_rw), which triggers cross-CPU TLB flushes
> and permanently splits 2MB hugepages in the ROX cache's direct map. When
> the buffer is immediately freed, these operations are repeated.

2M page is split temporarily. When the buffer is freed and ROX is restored,
2M page is collapsed.
 
> [Severity: High]
> This is a pre-existing issue, but there appears to be a flaw when freeing
> memory to the ROX cache in __execmem_cache_free():
> 
> 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);
> ...
> }
> 
> Does this ignore the return value of execmem_restore_rox()?

Answered this one already:
https://lore.kernel.org/bpf/ak-ZubgH13_MrXy4@kernel.org/

-- 
Sincerely yours,
Mike.