Re: [PATCH bpf 1/2] bpf: Do not release trampoline image in case off unregister error

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [Critical] Use-After-Free of BPF programs due to leaked trampoline image
- [High] Permanent memory leak of trampoline executable image
- [High] Use-After-Free of ftrace_ops and bpf_trampoline
--

commit 3c4d55e24f28fa5f7f6dd937603366805532d1a4
Author: Jiri Olsa <jolsa@kernel.org>

bpf: Do not release trampoline image in case off unregister error

If unregister_fentry fails we still have trampoline image attached
to a function, so releasing it could trigger crash. Releasing the
trampoline image only when the unregister succeeds.

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index f02254a21585fa..01082ecc5c4f5f 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -618,8 +618,10 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
>  
>  	if (total == 0) {
>  		err = unregister_fentry(tr, orig_flags, tr->cur_image->image);
> -		bpf_tramp_image_put(tr->cur_image);
> -		tr->cur_image = NULL;
> +		if (!err) {
> +			bpf_tramp_image_put(tr->cur_image);
> +			tr->cur_image = NULL;
> +		}

If unregister_fentry() fails, the ftrace or fentry hook remains active because
the image isn't put. Does the teardown path, like bpf_tracing_link_release(),
still proceed to tear down the link and free the bpf_prog structures?

If the BPF programs are freed but the leaked JIT executable image is still
attached and active, could subsequent calls to the hooked kernel function
execute the trampoline image, dereference the hardcoded pointers, and execute
freed BPF programs?

When unregister_fentry() fails and bpf_tramp_image_put() is skipped, does the
error propagate up to the caller where bpf_trampoline_put() might eventually
free the parent struct bpf_trampoline?

If the parent trampoline is destroyed without freeing tr->cur_image, does this
leak the struct bpf_tramp_image and its associated JIT memory page?

Additionally, if unregister_fentry() fails for an ftrace-managed trampoline,
tr->fops appears to remain registered in the ftrace subsystem.

If the link teardown ignores the error and unconditionally calls
bpf_trampoline_put(tr), will direct_ops_free() free tr->fops and tr?

Could this leave the ftrace subsystem with dangling pointers in its internal
tracking structures, potentially causing a use-after-free regression during
subsequent ftrace operations?

>  		goto out;
>  	}
>

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