[PATCH bpf-next 0/2] bpf: Fix two trampoline image UAFs in bpf_trampoline_multi_detach()

[PATCH bpf-next 0/2] bpf: Fix two trampoline image UAFs in bpf_trampoline_multi_detach()

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

While auditing the error paths added by commit aef4dfa790b2 ("bpf: Add
bpf_trampoline_multi_attach/detach functions"), I found two independent
ways bpf_trampoline_multi_detach() can free a trampoline image that
ftrace is still actively calling into.

Patch 1 fixes the case where __bpf_trampoline_unlink_prog() itself
fails before touching ftrace (e.g. -ENOMEM building the new image):
cur_image is left equal to old_image, but
bpf_trampoline_multi_attach_free() frees old_image unconditionally.

Patch 2 fixes the narrower case where the per-trampoline unlink
succeeds (cur_image has already been speculatively advanced) but the
later batched update_ftrace_direct_del()/update_ftrace_direct_mod()
call fails. Patch 1's old_image != cur_image check alone isn't enough
here since ftrace was never actually updated; the affected
trampolines need to be rolled back to old_image instead of freed.

Both paths are already WARN_ON_ONCE()'d and need an allocation or
ftrace failure to trigger, so they're unlikely to fire in practice,
but when they do, the current code frees a live image out from under
ftrace.

Hui Zhu (2):
  bpf: Fix stale old_image UAF on trampoline update failure
  bpf: Fix UAF in bpf_trampoline_multi_detach when ftrace update fails

 kernel/bpf/trampoline.c | 54 +++++++++++++++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH bpf-next 1/2] bpf: Fix stale old_image UAF on trampoline update failure

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

In bpf_trampoline_multi_detach(), if __bpf_trampoline_unlink_prog()
fails (e.g. -ENOMEM in bpf_tramp_image_alloc), bpf_trampoline_update()
returns before touching ftrace, so cur_image still equals old_image
and ftrace still points to it. The failure is only WARN_ON_ONCE'd and
the loop continues; afterwards bpf_trampoline_multi_attach_free()
unconditionally frees old_image via bpf_tramp_image_put(). Once the
RCU grace period elapses, the still-active image is freed while
ftrace keeps calling into it - a UAF that can crash the kernel or be
exploited via slab reuse.

The attach path avoids this with bpf_trampoline_multi_attach_rollback(),
but that can't be reused here: on this failure path cur_image ==
old_image, so calling it would free the active image the same way.
Re-adding the prog to the trampoline's hlist isn't safe either, since
bpf_tracing_multi_link_release() frees the link (and its embedded
tramp_node) immediately after detach returns.

Fix it by only freeing old_image in bpf_trampoline_multi_attach_free()
when it differs from cur_image. On success cur_image is updated to a
new image (or NULL), so old_image != cur_image correctly identifies a
stale image. On this failure path they're equal, so the image is left
alone until a later successful update replaces it.

Fixes: aef4dfa790b2 ("bpf: Add bpf_trampoline_multi_attach/detach functions")
Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 kernel/bpf/trampoline.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 6eadf64f7ec9..a78fbf726fad 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -1585,7 +1585,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 correctly identifies a stale image that
+	 * is safe to free.
+	 */
+	if (tr->multi_attach.old_image &&
+	    tr->multi_attach.old_image != tr->cur_image)
 		bpf_tramp_image_put(tr->multi_attach.old_image);
 
 	tr->multi_attach.old_image = NULL;
-- 
2.43.0

[PATCH bpf-next 2/2] bpf: Fix UAF in bpf_trampoline_multi_detach when ftrace update fails

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

After the per-mnode unlink loop, the return values of the batch
update_ftrace_direct_del()/update_ftrace_direct_mod() calls are only
WARN_ON_ONCE()'d, not checked. bpf_trampoline_multi_attach_free() is
then called unconditionally for every mnode, freeing old_image
whenever the single-point unlink succeeded (old_image != cur_image).

If the batch update fails, ftrace still points to old_image for the
affected IPs, so freeing it is a UAF.

Capture the two return values and, for mnodes whose single-point
unlink succeeded but whose batch update failed, call
bpf_trampoline_multi_attach_rollback() instead of
bpf_trampoline_multi_attach_free(). cur_image == NULL identifies the
unreg path (set by unregister_fentry_multi when total == 0),
cur_image != NULL identifies the modify path (set by
modify_fentry_multi when total > 0), so err_unreg/err_mod can be
matched to the right mnodes. Rollback restores cur_image = old_image,
the image ftrace is still actually calling.

This mirrors the existing error handling in
bpf_trampoline_multi_attach()'s rollback_unlink path.

Fixes: aef4dfa790b2 ("bpf: Add bpf_trampoline_multi_attach/detach functions")
Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 kernel/bpf/trampoline.c | 42 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index a78fbf726fad..44ef25beb4cc 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -1723,7 +1723,7 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
 {
 	struct bpf_tracing_multi_data *data = &link->data;
 	struct bpf_tracing_multi_node *mnode;
-	int i;
+	int i, err_unreg = 0, err_mod = 0;
 
 	trampoline_lock_all();
 
@@ -1734,13 +1734,41 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
 					NULL, &trampoline_multi_ops, data));
 	}
 
-	if (ftrace_hash_count(data->unreg))
-		WARN_ON_ONCE(update_ftrace_direct_del(&direct_ops, data->unreg));
-	if (ftrace_hash_count(data->modify))
-		WARN_ON_ONCE(update_ftrace_direct_mod(&direct_ops, data->modify, true));
+	if (ftrace_hash_count(data->unreg)) {
+		err_unreg = update_ftrace_direct_del(&direct_ops, data->unreg);
+		WARN_ON_ONCE(err_unreg);
+	}
+	if (ftrace_hash_count(data->modify)) {
+		err_mod = update_ftrace_direct_mod(&direct_ops, data->modify, true);
+		WARN_ON_ONCE(err_mod);
+	}
 
-	for_each_mnode(mnode, link)
-		bpf_trampoline_multi_attach_free(mnode->trampoline);
+	for_each_mnode(mnode, link) {
+		struct bpf_trampoline *tr = mnode->trampoline;
+
+		/* If the batch ftrace update failed for this mnode's path,
+		 * ftrace still points to old_image. Use rollback to restore
+		 * cur_image to old_image (putting the new cur_image if any)
+		 * so the trampoline keeps the image ftrace is calling.
+		 *
+		 * This relies on update_ftrace_direct_del/mod being atomic:
+		 * on failure, NO IPs in the hash are modified in ftrace (all
+		 * validation/allocation happens before any ftrace record is
+		 * touched). If this assumption is broken in the future (i.e.,
+		 * partial success becomes possible), this rollback logic would
+		 * need to be revisited.
+		 *
+		 * cur_image == NULL indicates the unreg path (total == 0);
+		 * cur_image != NULL indicates the modify path (total > 0).
+		 */
+		if (tr->multi_attach.old_image &&
+		    tr->multi_attach.old_image != tr->cur_image &&
+		    ((err_unreg && !tr->cur_image) ||
+		     (err_mod && tr->cur_image)))
+			bpf_trampoline_multi_attach_rollback(tr);
+		else
+			bpf_trampoline_multi_attach_free(tr);
+	}
 
 	trampoline_unlock_all();
 
-- 
2.43.0

Re: [PATCH bpf-next 2/2] bpf: Fix UAF in bpf_trampoline_multi_detach when ftrace update fails

[Jiri Olsa]

On Thu, Jul 16, 2026 at 04:43:39PM +0800, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> After the per-mnode unlink loop, the return values of the batch
> update_ftrace_direct_del()/update_ftrace_direct_mod() calls are only
> WARN_ON_ONCE()'d, not checked. bpf_trampoline_multi_attach_free() is
> then called unconditionally for every mnode, freeing old_image
> whenever the single-point unlink succeeded (old_image != cur_image).
> 
> If the batch update fails, ftrace still points to old_image for the
> affected IPs, so freeing it is a UAF.
> 
> Capture the two return values and, for mnodes whose single-point
> unlink succeeded but whose batch update failed, call
> bpf_trampoline_multi_attach_rollback() instead of
> bpf_trampoline_multi_attach_free(). cur_image == NULL identifies the
> unreg path (set by unregister_fentry_multi when total == 0),
> cur_image != NULL identifies the modify path (set by
> modify_fentry_multi when total > 0), so err_unreg/err_mod can be
> matched to the right mnodes. Rollback restores cur_image = old_image,
> the image ftrace is still actually calling.
> 
> This mirrors the existing error handling in
> bpf_trampoline_multi_attach()'s rollback_unlink path.
> 
> Fixes: aef4dfa790b2 ("bpf: Add bpf_trampoline_multi_attach/detach functions")
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> ---
>  kernel/bpf/trampoline.c | 42 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 35 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index a78fbf726fad..44ef25beb4cc 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -1723,7 +1723,7 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
>  {
>  	struct bpf_tracing_multi_data *data = &link->data;
>  	struct bpf_tracing_multi_node *mnode;
> -	int i;
> +	int i, err_unreg = 0, err_mod = 0;
>  
>  	trampoline_lock_all();
>  
> @@ -1734,13 +1734,41 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
>  					NULL, &trampoline_multi_ops, data));
>  	}
>  
> -	if (ftrace_hash_count(data->unreg))
> -		WARN_ON_ONCE(update_ftrace_direct_del(&direct_ops, data->unreg));
> -	if (ftrace_hash_count(data->modify))
> -		WARN_ON_ONCE(update_ftrace_direct_mod(&direct_ops, data->modify, true));
> +	if (ftrace_hash_count(data->unreg)) {
> +		err_unreg = update_ftrace_direct_del(&direct_ops, data->unreg);
> +		WARN_ON_ONCE(err_unreg);
> +	}
> +	if (ftrace_hash_count(data->modify)) {
> +		err_mod = update_ftrace_direct_mod(&direct_ops, data->modify, true);
> +		WARN_ON_ONCE(err_mod);
> +	}
>  
> -	for_each_mnode(mnode, link)
> -		bpf_trampoline_multi_attach_free(mnode->trampoline);
> +	for_each_mnode(mnode, link) {
> +		struct bpf_trampoline *tr = mnode->trampoline;
> +
> +		/* If the batch ftrace update failed for this mnode's path,
> +		 * ftrace still points to old_image. Use rollback to restore
> +		 * cur_image to old_image (putting the new cur_image if any)
> +		 * so the trampoline keeps the image ftrace is calling.
> +		 *
> +		 * This relies on update_ftrace_direct_del/mod being atomic:
> +		 * on failure, NO IPs in the hash are modified in ftrace (all
> +		 * validation/allocation happens before any ftrace record is
> +		 * touched). If this assumption is broken in the future (i.e.,
> +		 * partial success becomes possible), this rollback logic would
> +		 * need to be revisited.
> +		 *
> +		 * cur_image == NULL indicates the unreg path (total == 0);
> +		 * cur_image != NULL indicates the modify path (total > 0).
> +		 */
> +		if (tr->multi_attach.old_image &&

I think tr->multi_attach.old_image is always != NULL in here?

> +		    tr->multi_attach.old_image != tr->cur_image &&
> +		    ((err_unreg && !tr->cur_image) ||
> +		     (err_mod && tr->cur_image)))
> +			bpf_trampoline_multi_attach_rollback(tr);

it's good that we won't free the image that's used, but after the link
is detached, its program will be freed, so I expect the trampoline execution
is likely to crash anyway.. did you try to simulate the errors?

I remember suggesting to increase the prog's refcount to prevent that
but I never sent that change.. I think also standard trampolines have
the same issue

> +		else
> +			bpf_trampoline_multi_attach_free(tr);
> +	}
>  
>  	trampoline_unlock_all();

also we could have bpf_trampoline_multi_detach returning void,
without the WARN_ON_ONCE in bpf_tracing_multi_link_release,
we already have the warnings in here

thanks,
jirka