Re: [PATCH 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 BPF trampoline size calculations cause data races and severe cacheline bouncing on the globally shared `trampoline_size_image` buffer.
--

commit d4cd8458939c8e8adafba04b2627a4ef1338ac21
Author: Jiri Olsa <jolsa@kernel.org>
Subject: bpf, x86: Use global buffer for trampoline size generation

This commit replaces the per-invocation allocation of a temporary
trampoline buffer in arch_bpf_trampoline_size() with a globally shared
buffer allocated during initialization. This reduces the time it takes
to calculate the required size by reusing the writable scratch page.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index bba351944202c..13ef0d53ca296 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c

[ ... ]

> @@ -4000,24 +4010,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]
Can this lead to concurrent modifications of the shared scratch buffer?

If multiple threads attach trampolines or load BPF struct_ops programs
simultaneously, they might both invoke arch_bpf_trampoline_size() and
execute lockless writes into trampoline_size_image (such as inside
emit_code()).

For example, bpf_struct_ops_map_update_elem() calls into the trampoline size
calculation while holding only a per-map lock, allowing parallel updates
across different maps.

While the generated bytes are ultimately discarded and the calculation
depends on local pointer arithmetic rather than the buffer's contents, do
these concurrent lockless writes risk severe cacheline bouncing? This could
degrade concurrent attachment performance and will likely trigger KCSAN data
race warnings.

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