Re: [PATCH bpf v2] bpf: keep trampoline progs alive until image release

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch restricts the `bpf_tramp_image` program reference counting fix to `CONFIG_PREEMPTION=y`, leaving `CONFIG_PREEMPTION=n` kernels vulnerable to a Use-After-Free if `bpf_trampoline_update()` fails during program unlinking.
--

--- Patch [1]: [PATCH bpf v2] bpf: keep trampoline progs alive until image release ---
commit e97f38fbbba7062b73e2785973928c70f36f1661
Author: Junseo Lim <zirajs7@gmail.com>

bpf: keep trampoline progs alive until image release

arch_prepare_bpf_trampoline() embeds program pointers in the generated
image. After bpf_trampoline_update() replaces the image,
bpf_tramp_image_put() can keep the old image executable past a normal RCU
grace period, allowing a detached non-sleepable prog to be freed before a
preempted task finishes, leading to a use-after-free. This patch keeps
image-local prog refs and drops them from bpf_tramp_image_free().

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1376,6 +1376,11 @@ struct bpf_tramp_image {
>  		struct rcu_head rcu;
>  		struct work_struct work;
>  	};
> +#ifdef CONFIG_PREEMPTION
> +	/* Programs called from this image must outlive deferred image freeing. */
> +	struct bpf_prog *progs[BPF_MAX_TRAMP_LINKS];
> +	int nr_progs;
> +#endif
>  };

[ ... ]

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -529,8 +529,36 @@ bpf_trampoline_get_progs(const struct bpf_trampoline *tr, int *total, bool *ip_a
>  	return tnodes;
>  }
>  
> +static void bpf_tramp_image_get_progs(struct bpf_tramp_image *im,
> +				      struct bpf_tramp_nodes *tnodes)
> +{
> +#ifdef CONFIG_PREEMPTION
        ^^^^

[Severity: High]
This is a pre-existing issue, but does restricting this reference counting to
CONFIG_PREEMPTION=y leave CONFIG_PREEMPTION=n kernels vulnerable to a
use-after-free if bpf_trampoline_update() fails during program unlinking?

If a user detaches a BPF tracing program, bpf_tracing_link_release() calls
__bpf_trampoline_unlink_prog(), which removes the program from the trampoline
list and calls bpf_trampoline_update().

If bpf_trampoline_update() fails (for example, due to -ENOMEM in
bpf_tramp_image_alloc()), the error is absorbed by a WARN_ON_ONCE. The link's
reference to the program is subsequently dropped, and the program is freed.

However, because the update failed, the old executable image remains active
and continues to execute the embedded pointers to the freed program memory.

Could the CONFIG_PREEMPTION ifdef guards be removed so that the image always
holds references to its programs, preventing the use-after-free for
CONFIG_PREEMPTION=n kernels as well?

> +	int i, kind;
> +
> +	for (kind = 0; kind < BPF_TRAMP_MAX; kind++)
> +		for (i = 0; i < tnodes[kind].nr_nodes; i++) {
> +			struct bpf_prog *prog = tnodes[kind].nodes[i]->link->prog;
> +
> +			bpf_prog_inc(prog);
> +			im->progs[im->nr_progs++] = prog;
> +		}
> +#endif
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820022831.18601-1-zirajs7@gmail.com?part=1