Re: [PATCH v4] bpf: Fix UAF in bpf_trampoline_multi_attach_free on update failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Ignoring errors from `__bpf_trampoline_unlink_prog()` during detach leads to a Use-After-Free of the BPF program.
- [High] Ignoring errors from `update_ftrace_direct_del/mod()` during detach or attach rollback leads to a Use-After-Free of the trampoline image.
--

--- Patch [1]: [PATCH v4] bpf: Fix UAF in bpf_trampoline_multi_attach_free on update failure ---
commit 12049949b83ebddc54a6fdad57fa0b635706dbda
Author: Hui Zhu <zhuhui@kylinos.cn>
bpf: Fix UAF in bpf_trampoline_multi_attach_free on update failure

When bpf_trampoline_update() fails before modify_fentry_multi() or
unregister_fentry_multi() is called, cur_image is unchanged and ftrace still
calls into it. Freeing old_image in that case causes a UAF. This patch only
frees old_image when it differs from cur_image, and makes
bpf_trampoline_multi_detach() return void.

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index e07af35ed0402..90b70ea0d3709 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -1632,7 +1632,17 @@ static void bpf_trampoline_multi_attach_init(struct bpf_trampoline *tr)
>  
>  static void bpf_trampoline_multi_attach_free(struct bpf_trampoline *tr)
>  {
> -	if (tr->multi_attach.old_image)
> +	/*
> +	 * Only free old_image if it is no longer the active image.
> +	 * When bpf_trampoline_update() fails before modify_fentry_multi()/
> +	 * unregister_fentry_multi() is called, cur_image is unchanged
> +	 * (cur_image == old_image) and ftrace still points to it. Freeing
> +	 * it would cause a UAF when ftrace calls into the freed memory.
> +	 * On success, cur_image is either a new image or NULL, so
> +	 * old_image != cur_image means the image is stale.
> +	 */
> +	if (tr->multi_attach.old_image &&
> +	    tr->multi_attach.old_image != tr->cur_image)

[Severity: High]
This is a pre-existing issue, but does this condition still allow a
use-after-free of the trampoline image if ftrace updates fail?

If __bpf_trampoline_unlink_prog() succeeds in bpf_trampoline_multi_detach(),
the active image is updated and old_image != cur_image will be true.

However, if the subsequent update to ftrace fails due to memory pressure:

    if (ftrace_hash_count(data->unreg))
        WARN_ON_ONCE(update_ftrace_direct_del(&direct_ops, data->unreg));

ftrace will return an error without updating its hooks, leaving ftrace
pointing to the original active image (old_image).

Since old_image != cur_image is true, won't bpf_trampoline_multi_attach_free()
then free old_image while ftrace is still executing it?

A similar issue appears to exist during error rollback in
kernel/bpf/trampoline.c:bpf_trampoline_multi_attach() if
update_ftrace_direct_del() fails, leaving ftrace executing newly allocated
images that are subsequently freed.

>  		bpf_tramp_image_put(tr->multi_attach.old_image);
>  
>  	tr->multi_attach.old_image = NULL;

[ ... ]

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 891897f8a1b35..29260951aa871 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -3687,7 +3687,7 @@ static void bpf_tracing_multi_link_release(struct bpf_link *link)
>  	struct bpf_tracing_multi_link *tr_link =
>  		container_of(link, struct bpf_tracing_multi_link, link);
>  
> -	WARN_ON_ONCE(bpf_trampoline_multi_detach(link->prog, tr_link));
> +	bpf_trampoline_multi_detach(link->prog, tr_link);

[Severity: High]
This is a pre-existing issue, but does dropping the program reference here
lead to a use-after-free if __bpf_trampoline_unlink_prog() fails?

In kernel/bpf/trampoline.c:bpf_trampoline_multi_detach(), if unlink fails:

    err = __bpf_trampoline_unlink_prog(&mnode->node, mnode->trampoline, NULL,
                                       &trampoline_multi_ops, data);
    WARN_ONCE(err, "__bpf_trampoline_unlink_prog failed: %d\n", err);

the detach path ignores the error. The ftrace direct hook remains active and
the active trampoline image remains unchanged, keeping its hardcoded call
to the BPF program.

Since bpf_tracing_multi_link_release() proceeds to drop the link's reference
to the BPF program, if the program's refcount drops to zero and it is freed,
could ftrace continue to jump into the freed program?

>  }
>  
>  static void bpf_tracing_multi_link_dealloc(struct bpf_link *link)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810061329.224276-1-hui.zhu@linux.dev?part=1