Re: [PATCHv2 bpf-next] bpf, x86: Use global buffer for trampoline size generation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Concurrent calls to arch_bpf_trampoline_size() write to the globally shared, unsynchronized `trampoline_size_image` buffer, resulting in a data race (Undefined Behavior) and cache-line contention.
--

commit 4c6aeb9264cf2877d767fb9a021802485dd9d3a7
Author: Jiri Olsa <jolsa@kernel.org>

bpf, x86: Use global buffer for trampoline size generation

This commit changes the BPF trampoline size calculation to use a single,
globally allocated executable memory buffer instead of allocating a temporary
one per invocation. This change aims to improve attachment time during tracing
by eliminating the overhead of repeated memory allocations and frees.

Link: https://lore.kernel.org/bpf/20260907160538.922450-1-jolsa@kernel.org/

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index bba351944202c..9f9b1456dab0d 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
[ ... ]
> @@ -35,6 +36,21 @@ void __asan_store8(void *p);
>  
>  static bool all_callee_regs_used[4] = {true, true, true, true};
>  
> +static void *trampoline_size_image;
> +
> +static int __init init_trampoline_size_image(void)
> +{
> +	/*
> +	 * The generated trampoline contains calls and jumps with 32bit relative
> +	 * offsets, so the scratch image must be in the execmem range.
> +	 * On x86, module data and executable memory share the same address range,
> +	 * so using EXECMEM_MODULE_DATA to get writable memory.
> +	 */
> +	trampoline_size_image = execmem_alloc(EXECMEM_MODULE_DATA, PAGE_SIZE);

[Severity: Medium]
Is this global allocation safe against concurrent access? Multiple threads
could compute trampoline sizes concurrently, for example when attaching BPF
programs to kernel functions that hash to different buckets in
trampoline_locks, or when concurrently updating different struct_ops maps.

> +	return trampoline_size_image ? 0 : -ENOMEM;
> +}
> +late_initcall(init_trampoline_size_image);
[ ... ]
> @@ -4000,24 +4016,14 @@ int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
>  			     struct bpf_tramp_nodes *tnodes, void *func_addr)
>  {
>  	struct bpf_tramp_image im;
> -	void *image;
> -	int ret;
>  
> -	/* Allocate a temporary buffer for __arch_prepare_bpf_trampoline().
> -	 *
> -	 * We cannot use kvmalloc here, because we need image to be in
> -	 * module memory range.
> -	 * Since it must be writable use execmem_alloc(EXECMEM_MODULE_DATA)
> -	 * that returns writable memory in the module address space.
> -	 */
> -	image = execmem_alloc(EXECMEM_MODULE_DATA, PAGE_SIZE);
> -	if (!image)
> +	if (!trampoline_size_image)
>  		return -ENOMEM;
>  
> -	ret = __arch_prepare_bpf_trampoline(&im, image, image + PAGE_SIZE, image,
> -					    m, flags, tnodes, func_addr);
> -	execmem_free(image);
> -	return ret;
> +	return __arch_prepare_bpf_trampoline(&im, trampoline_size_image,
> +					     trampoline_size_image + PAGE_SIZE,
> +					     trampoline_size_image, m, flags,
> +					     tnodes, func_addr);
>  }

[Severity: Medium]
When __arch_prepare_bpf_trampoline() writes to trampoline_size_image, could
this lead to a data race if called concurrently?

Since trampoline_size_image is globally shared and no lock protects it here,
writers in emit_code() might overwrite each other's data:

arch_bpf_trampoline_size()
  __arch_prepare_bpf_trampoline()
    emit_code()
      *(u32 *)ptr = bytes;

While the memory contents aren't executed, concurrent writes will cause
undefined behavior and cache-line ping-ponging, which could trigger KCSAN
warnings and crash systems with panic_on_warn enabled. Should this use a
per-CPU buffer or have explicit synchronization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910094353.1256316-1-jolsa@kernel.org?part=1