[PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure

[PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

This series fixes several use-after-free issues in the BPF trampoline
multi-attach/detach error paths, where ftrace direct-call updates can
fail and leave ftrace pointing at freed memory.

Patch 1 addresses two UAF scenarios in bpf_trampoline_multi_detach():
the single-point unlink failure path (old_image == cur_image) and the
batch ftrace update failure path. A new pinned_prog field in struct
bpf_tramp_image keeps the bpf_prog alive while ftrace may still
reference its image. bpf_trampoline_multi_detach() is made to return
void, since callers cannot usefully react to failures, and
bpf_trampoline_put() is taught to leak the trampoline when cur_image
was left behind by a rollback, so ftrace keeps a valid target.

Patch 2 fixes a similar UAF in bpf_trampoline_multi_attach() rollback:
when the register-path undo fails, ftrace still calls into cur_image,
so the prog is pinned on cur_image instead of being rolled back.

Patch 3 fixes the common __bpf_trampoline_unlink_prog() path, covering
both multi (bpf_trampoline_multi_detach) and non-multi
(bpf_tracing_link_release, bpf_shim_tramp_link_release) callers.

Hui Zhu (3):
  bpf: Fix UAF in bpf_trampoline_multi_detach on update failure
  bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path
    rollback
  bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure

 include/linux/bpf.h      |  20 +++--
 kernel/bpf/trampoline.c  | 183 +++++++++++++++++++++++++++++++++++----
 kernel/trace/bpf_trace.c |   2 +-
 3 files changed, 183 insertions(+), 22 deletions(-)

Changelog:
v2:
Folded v1's two detach patches into patch 1.
According to the comments of Jiri Olsa, Pin the prog (pinned_prog) on
cur_image so it stays alive while ftrace may still call into it.
Make bpf_trampoline_multi_detach() return void.
Fix the same UAF in standard (non-multi) trampolines.
According to the comments of sashiko, Fix the prog UAF in
bpf_trampoline_multi_attach() rollback.
Leak the trampoline in bpf_trampoline_put() when cur_image is left
by a rollback.

-- 
2.53.0

[PATCH bpf-next v2 1/3] bpf: Fix UAF in bpf_trampoline_multi_detach on update failure

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

Two UAF scenarios exist in bpf_trampoline_multi_detach() error paths:

1. If __bpf_trampoline_unlink_prog() fails, cur_image == old_image and
   ftrace still points to it, but bpf_trampoline_multi_attach_free()
   unconditionally frees old_image. Fix: only free old_image when it
   differs from cur_image.

2. If the batch update_ftrace_direct_del/mod() fails, ftrace still
   points to old_image, but _free() frees it. Fix: use rollback
   (restores cur_image = old_image) instead of _free() for affected
   mnodes.

Rollback keeps the image alive but the caller still frees the prog
whose call is baked into it. Pin the prog on old_image via a new
pinned_prog field in struct bpf_tramp_image (released in
bpf_tramp_image_free()).

bpf_trampoline_put() must not free a trampoline whose cur_image was
left behind by rollback -- ftrace may still call into it. Leak the
trampoline instead (it's already unlinked from lookup tables).

pinned_prog is a single pointer: if multiple progs need pinning on the
same image (rare), only the last is tracked and earlier refs are leaked
(not a UAF). This is an intentional trade-off.

Also make bpf_trampoline_multi_detach() return void since callers
cannot usefully react to failures.

Fixes: aef4dfa790b2 ("bpf: Add bpf_trampoline_multi_attach/detach functions")
Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 include/linux/bpf.h      |  20 +++++--
 kernel/bpf/trampoline.c  | 112 +++++++++++++++++++++++++++++++++++----
 kernel/trace/bpf_trace.c |   2 +-
 3 files changed, 118 insertions(+), 16 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 73bacfc6444d..cd32c6f54eeb 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1372,6 +1372,17 @@ struct bpf_tramp_image {
 		struct rcu_head rcu;
 		struct work_struct work;
 	};
+	/*
+	 * Extra reference on the bpf_prog whose call is baked into this
+	 * image's machine code, held only when a required ftrace
+	 * direct-call update failed while retiring/replacing this image
+	 * (see bpf_trampoline_multi_attach()/_detach() in trampoline.c).
+	 * ftrace may still be directing calls into this image, so neither
+	 * the image nor the pinned prog can be freed until a later,
+	 * successful ftrace update proves this image is no longer in use.
+	 * Released in bpf_tramp_image_free() alongside the image itself.
+	 */
+	struct bpf_prog *pinned_prog;
 };
 
 struct bpf_trampoline {
@@ -1518,8 +1529,8 @@ int arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int num_func
 
 int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
 				struct bpf_tracing_multi_link *link);
-int bpf_trampoline_multi_detach(struct bpf_prog *prog,
-				struct bpf_tracing_multi_link *link);
+void bpf_trampoline_multi_detach(struct bpf_prog *prog,
+				 struct bpf_tracing_multi_link *link);
 void bpf_trampoline_set_flags(struct bpf_trampoline *tr, u32 flags);
 
 /*
@@ -1639,10 +1650,9 @@ static inline int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
 {
 	return -ENOTSUPP;
 }
-static inline int bpf_trampoline_multi_detach(struct bpf_prog *prog,
-					      struct bpf_tracing_multi_link *link)
+static inline void bpf_trampoline_multi_detach(struct bpf_prog *prog,
+					       struct bpf_tracing_multi_link *link)
 {
-	return -ENOTSUPP;
 }
 static inline void bpf_trampoline_set_flags(struct bpf_trampoline *tr, u32 flags) {}
 #endif
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index ed7999ad6c66..c08d1a09e638 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -535,6 +535,14 @@ static void bpf_tramp_image_free(struct bpf_tramp_image *im)
 	arch_free_bpf_trampoline(im->image, im->size);
 	bpf_jit_uncharge_modmem(im->size);
 	percpu_ref_exit(&im->pcref);
+	/*
+	 * This image is confirmed no longer reachable from ftrace (that's
+	 * why we're freeing it), so it's now safe to drop the reference we
+	 * pinned on its behalf while it may have still been live - see
+	 * bpf_trampoline_multi_attach()/_detach().
+	 */
+	if (im->pinned_prog)
+		bpf_prog_put(im->pinned_prog);
 	kfree_rcu(im, rcu);
 }
 
@@ -1216,6 +1224,28 @@ void bpf_trampoline_put(struct bpf_trampoline *tr)
 	 */
 	hlist_del(&tr->hlist_key);
 	hlist_del(&tr->hlist_ip);
+
+	/*
+	 * tr->cur_image should already be NULL here. A non-NULL value means
+	 * bpf_trampoline_multi_attach_rollback() left an image behind
+	 * because a required ftrace direct-call update failed (see
+	 * bpf_trampoline_multi_detach()), so ftrace may still be calling
+	 * into it - and, in turn, into the bpf_prog pinned in
+	 * tr->cur_image->pinned_prog. We have no reliable way to confirm
+	 * ftrace has since stopped referencing it, so freeing
+	 * tr->cur_image (and dropping the pinned prog's reference) here
+	 * would risk a use-after-free.
+	 *
+	 * tr has just been unlinked from the lookup tables above, so any
+	 * future attach to this function allocates a fresh trampoline;
+	 * this one, its stuck image, and the pinned prog reference are
+	 * deliberately leaked instead of freed. This is rare (it only
+	 * happens after a genuine ftrace direct-call update failure) and
+	 * bounded (at most one image), so it is far preferable to a UAF.
+	 */
+	if (WARN_ON_ONCE(tr->cur_image))
+		goto out;
+
 	direct_ops_free(tr);
 	kfree(tr);
 out:
@@ -1595,7 +1625,18 @@ 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;
@@ -1719,11 +1760,11 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
 	return err;
 }
 
-int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_link *link)
+void bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_link *link)
 {
 	struct bpf_tracing_multi_data *data = &link->data;
 	struct bpf_tracing_multi_node *mnode;
-	int i, err;
+	int i, err, err_unreg = 0, err_mod = 0;
 
 	trampoline_lock_all();
 
@@ -1735,13 +1776,65 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
 		WARN_ONCE(err, "__bpf_trampoline_unlink_prog failed: %d\n", err);
 	}
 
-	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.
+		 *
+		 * A link only reaches detach after a successful attach, so
+		 * tr->cur_image (captured above as old_image) is always
+		 * non-NULL here; the NULL check only mirrors the one in
+		 * bpf_trampoline_multi_attach_free()/_rollback()'s shared
+		 * pattern and guards against tr->multi_attach being reused
+		 * without a prior _init() call.
+		 *
+		 * 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).
+		 *
+		 * Rollback alone only prevents freeing the trampoline image
+		 * while ftrace may still branch into it; it does not keep
+		 * the underlying bpf_prog alive, and the caller tears down
+		 * link->prog once this function returns. So pin @prog (whose
+		 * call is baked into old_image's machine code) on old_image
+		 * before restoring it as cur_image: the pin is released once
+		 * old_image is eventually retired for real by a later,
+		 * successful update on this trampoline (see
+		 * bpf_trampoline_multi_attach_free() and
+		 * bpf_tramp_image_free()), or safely leaked alongside the
+		 * image if the trampoline is torn down first instead (see
+		 * bpf_trampoline_put()).
+		 */
+		if (tr->multi_attach.old_image &&
+		    tr->multi_attach.old_image != tr->cur_image &&
+		    ((err_unreg && !tr->cur_image) ||
+		     (err_mod && tr->cur_image))) {
+			WARN_ON_ONCE(tr->multi_attach.old_image->pinned_prog);
+			bpf_prog_inc(prog);
+			tr->multi_attach.old_image->pinned_prog = prog;
+			bpf_trampoline_multi_attach_rollback(tr);
+		} else {
+			bpf_trampoline_multi_attach_free(tr);
+		}
+	}
 
 	trampoline_unlock_all();
 
@@ -1749,7 +1842,6 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
 		bpf_trampoline_put(mnode->trampoline);
 
 	clear_tracing_multi_data(data);
-	return 0;
 }
 
 #undef for_each_mnode_cnt
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 891897f8a1b3..29260951aa87 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);
 }
 
 static void bpf_tracing_multi_link_dealloc(struct bpf_link *link)
-- 
2.53.0

Re: [PATCH bpf-next v2 1/3] bpf: Fix UAF in bpf_trampoline_multi_detach on update failure

[sashiko-bot]

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

New issues:
- [Low] Multi-line comment opening `/*` is not on its own line.

Pre-existing issues:
- [High] Incomplete fix: Use-After-Free of `bpf_prog` when `__bpf_trampoline_unlink_prog()` fails.
- [High] Use-After-Free of trampoline image in `bpf_trampoline_multi_attach` error path.
--

--- Patch [1]: [PATCH bpf-next v2 1/3] bpf: Fix UAF in bpf_trampoline_multi_detach on update failure ---
commit 26ad7c4ab94aa890dec18cbc899359aabf65c015
Author: Hui Zhu <zhuhui@kylinos.cn>

    bpf: Fix UAF in bpf_trampoline_multi_detach on update failure
    
    The commit attempts to fix Use-After-Free issues in the error paths of
    bpf_trampoline_multi_detach() by changing when old_image is freed and
    introducing a rollback mechanism when ftrace updates fail. It pins the
    bpf_prog onto the image to avoid freeing it while ftrace might still
    direct calls to it.

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index ed7999ad6c66c..c08d1a09e6381 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
[ ... ]
> @@ -1719,11 +1760,11 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
>  	return err;
>  }
>  
> -int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_link *link)
> +void bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_link *link)
>  {
>  	struct bpf_tracing_multi_data *data = &link->data;
>  	struct bpf_tracing_multi_node *mnode;
> -	int i, err;
> +	int i, err, err_unreg = 0, err_mod = 0;
>  
>  	trampoline_lock_all();
>  
> @@ -1735,13 +1776,65 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
>  		WARN_ONCE(err, "__bpf_trampoline_unlink_prog failed: %d\n", err);
>  	}
>  
> -	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

[Severity: Low]
Does this multi-line comment violate the BPF subsystem guidelines? The
opening /* of a multi-line comment must be on its own line.

> +		 * cur_image to old_image (putting the new cur_image if any)
> +		 * so the trampoline keeps the image ftrace is calling.
[ ... ]
> +		 * image if the trampoline is torn down first instead (see
> +		 * bpf_trampoline_put()).
> +		 */
> +		if (tr->multi_attach.old_image &&
> +		    tr->multi_attach.old_image != tr->cur_image &&
> +		    ((err_unreg && !tr->cur_image) ||
> +		     (err_mod && tr->cur_image))) {

[Severity: High]
This is a pre-existing issue, but does this incomplete fix lead to a
Use-After-Free of the bpf_prog structure if __bpf_trampoline_unlink_prog()
fails?

When __bpf_trampoline_unlink_prog() fails, old_image equals cur_image,
so this condition evaluates to false, bypassing the logic to pin the
bpf_prog onto the retained image. This allows the caller to release its
reference to the bpf_prog and free it, even though the active ftrace
trampoline image still contains a direct machine-code call to the freed
prog.

A similar missing pinning logic exists in the non-multi caller of
__bpf_trampoline_unlink_prog() where it directly returns the failure of
bpf_trampoline_update() without pinning the prog:

kernel/bpf/trampoline.c:__bpf_trampoline_unlink_prog() {
    ...
	bpf_trampoline_remove_prog(tr, node);
	return bpf_trampoline_update(tr, true /* lock_direct_mutex */, ops, data);
}

> +			WARN_ON_ONCE(tr->multi_attach.old_image->pinned_prog);
> +			bpf_prog_inc(prog);
> +			tr->multi_attach.old_image->pinned_prog = prog;
> +			bpf_trampoline_multi_attach_rollback(tr);
> +		} else {
> +			bpf_trampoline_multi_attach_free(tr);
> +		}
> +	}
>  
>  	trampoline_unlock_all();

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
bpf_trampoline_multi_attach() cause a Use-After-Free of the trampoline
image?

If update_ftrace_direct_del() fails during rollback:

kernel/bpf/trampoline.c:bpf_trampoline_multi_attach() {
    ...
	if (ftrace_hash_count(data->modify)) {
		err = update_ftrace_direct_mod(&direct_ops, data->modify, true);
		if (err) {
			if (ftrace_hash_count(data->reg))
				WARN_ON_ONCE(update_ftrace_direct_del(&direct_ops, data->reg));
			goto rollback_unlink;
		}
	}
    ...
}

It proceeds blindly to rollback_unlink, which calls
bpf_trampoline_multi_attach_rollback():

kernel/bpf/trampoline.c:bpf_trampoline_multi_attach_rollback() {
    ...
	if (tr->cur_image)
		bpf_tramp_image_put(tr->cur_image);
    ...
}

Does this unconditionally free the newly allocated trampoline images even
when update_ftrace_direct_del() fails, leading to a UAF since ftrace
is still actively pointing to them?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785902527.git.zhuhui@kylinos.cn?part=1

[PATCH bpf-next v2 2/3] bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path rollback

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

In bpf_trampoline_multi_attach(), if update_ftrace_direct_mod() fails,
the rollback tries to undo update_ftrace_direct_add() for register-path
mnodes via update_ftrace_direct_del(). If that undo also fails, ftrace
still calls into rtr->cur_image, but the unconditional rollback frees
it -- a UAF of both the image and the prog baked into it.

Fix: for register-path mnodes (old_image == NULL) whose undo failed
while cur_image is set, pin the prog on cur_image instead of rolling
back, reusing the pinned_prog mechanism from the detach path.

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

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index c08d1a09e638..7fac27374ece 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -1668,7 +1668,7 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
 	struct btf *btf = prog->aux->attach_btf;
 	struct bpf_tracing_multi_node *mnode;
 	struct bpf_trampoline *tr;
-	int i, err, rollback_cnt;
+	int i, err, rollback_cnt, err_undo_reg = 0;
 	u64 key;
 
 	for_each_mnode(mnode, link) {
@@ -1728,8 +1728,10 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
 	if (ftrace_hash_count(data->modify)) {
 		err = update_ftrace_direct_mod(&direct_ops, data->modify, true);
 		if (err) {
-			if (ftrace_hash_count(data->reg))
-				WARN_ON_ONCE(update_ftrace_direct_del(&direct_ops, data->reg));
+			if (ftrace_hash_count(data->reg)) {
+				err_undo_reg = update_ftrace_direct_del(&direct_ops, data->reg);
+				WARN_ON_ONCE(err_undo_reg);
+			}
 			goto rollback_unlink;
 		}
 	}
@@ -1744,8 +1746,47 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
 
 rollback_unlink:
 	for_each_mnode_cnt(mnode, link, rollback_cnt) {
-		bpf_trampoline_remove_prog(mnode->trampoline, &mnode->node);
-		bpf_trampoline_multi_attach_rollback(mnode->trampoline);
+		struct bpf_trampoline *rtr = mnode->trampoline;
+		/*
+		 * register_fentry_multi()/modify_fentry_multi() set
+		 * rtr->cur_image before any ftrace call is made, and
+		 * bpf_trampoline_multi_attach_init() captured whatever was
+		 * live before that into rtr->multi_attach.old_image. A NULL
+		 * old_image means this ip had no prior direct caller, i.e.
+		 * this mnode went through the "register" (data->reg) path
+		 * rather than "modify" (data->modify).
+		 */
+		bool via_register = !rtr->multi_attach.old_image;
+
+		bpf_trampoline_remove_prog(rtr, &mnode->node);
+
+		/*
+		 * If this mnode used the register path and the
+		 * update_ftrace_direct_del() above meant to undo its
+		 * earlier, successful update_ftrace_direct_add() failed,
+		 * ftrace is still actually calling into rtr->cur_image
+		 * (which has @prog's call baked into its machine code) even
+		 * though this attach is being reported as failed. Freeing
+		 * rtr->cur_image via the normal rollback (which would also
+		 * let the caller free @prog once this function returns its
+		 * error) would be a use-after-free, so instead pin @prog on
+		 * it and leave rtr->cur_image untouched: rtr->multi_attach
+		 * is a scratch area only meaningful between _init() and
+		 * _free()/_rollback(), so skipping _rollback() here does
+		 * not leave it in an inconsistent state (old_image is NULL
+		 * on the register path anyway). This image (and the pinned
+		 * prog reference) is subsequently either properly retired by
+		 * a later, successful update on the same trampoline, or
+		 * safely leaked when the trampoline is torn down - see
+		 * bpf_trampoline_multi_attach_free() and bpf_trampoline_put().
+		 */
+		if (via_register && err_undo_reg && rtr->cur_image) {
+			WARN_ON_ONCE(rtr->cur_image->pinned_prog);
+			bpf_prog_inc(prog);
+			rtr->cur_image->pinned_prog = prog;
+		} else {
+			bpf_trampoline_multi_attach_rollback(rtr);
+		}
 	}
 
 	trampoline_unlock_all();
-- 
2.53.0

Re: [PATCH bpf-next v2 2/3] bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path rollback

[sashiko-bot]

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

New issues:
- [Medium] Memory leak of pinned BPF program on consecutive ftrace update failures.

Pre-existing issues:
- [High] Use-after-free of BPF program if trampoline image allocation fails during detach.
--

--- Patch [2]: [PATCH bpf-next v2 2/3] bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path rollback ---
commit d47fe23104c85648ed4814b819490191dd04b447
Author: Hui Zhu <zhuhui@kylinos.cn>

bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path rollback

This commit fixes a use-after-free in bpf_trampoline_multi_attach() where
ftrace rollback failures could lead to the trampoline image and attached
program being unconditionally freed while still active. It resolves this by
pinning the program to the current image if the register-path undo fails.

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index c08d1a09e6381..7fac27374ece1 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c

[ ... ]

> @@ -1744,8 +1746,47 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
>  
>  rollback_unlink:
>  	for_each_mnode_cnt(mnode, link, rollback_cnt) {

[ ... ]

> +		if (via_register && err_undo_reg && rtr->cur_image) {
> +			WARN_ON_ONCE(rtr->cur_image->pinned_prog);
> +			bpf_prog_inc(prog);
> +			rtr->cur_image->pinned_prog = prog;

[Severity: Medium]
Can this result in a memory leak if multiple operations fail on the same
trampoline?

Since pinned_prog is a single pointer, if multiple programs attached to the
same trampoline experience ftrace update failures, the previous program
appears to be overwritten without releasing its reference.

Is there a way to track multiple pinned programs to prevent permanently
leaking the bpf_prog and its associated resources in this rollback path?

> +		} else {
> +			bpf_trampoline_multi_attach_rollback(rtr);
> +		}
>  	}
>  
>  	trampoline_unlock_all();

[Severity: High]
This isn't a bug introduced by this patch, but does a similar use-after-free
exist during detach if trampoline image allocation fails?

If a user detaches a program but bpf_tramp_image_alloc() fails (e.g. from
memory pressure), __bpf_trampoline_unlink_prog() removes the program from
the active list but leaves the image unchanged:

kernel/bpf/trampoline.c:__bpf_trampoline_unlink_prog() {
	...
	bpf_trampoline_remove_prog(tr, node);
	return bpf_trampoline_update(tr, true /* lock_direct_mutex */, ops, data);
}

In bpf_trampoline_multi_detach(), the rollback logic requires old_image to
differ from cur_image to pin the program:

kernel/bpf/trampoline.c:bpf_trampoline_multi_detach() {
	...
		if (tr->multi_attach.old_image &&
		    tr->multi_attach.old_image != tr->cur_image &&
	...
}

Because the image allocation failed, old_image == cur_image, which bypasses
the program pinning logic. The caller bpf_tracing_multi_link_release() will
then unconditionally drop the program reference via bpf_prog_put(). 

Could this allow ftrace to execute the freed BPF program memory since the
active ftrace image still contains direct calls to it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785902527.git.zhuhui@kylinos.cn?part=2

[PATCH bpf-next v2 3/3] bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

If bpf_trampoline_update() fails in __bpf_trampoline_unlink_prog(),
cur_image is unchanged and still has node->link->prog's call baked
into it. All callers only WARN_ON_ONCE() the failure and then
unconditionally free the prog -- a UAF.

Fix: on failure, pin node->link->prog onto tr->cur_image via the
pinned_prog mechanism, so it outlives the link. This covers both the
multi (bpf_trampoline_multi_detach) and non-multi
(bpf_tracing_link_release, bpf_shim_tramp_link_release) paths.

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

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 7fac27374ece..6071261fd66b 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -991,7 +991,25 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_node *node,
 		return err;
 	}
 	bpf_trampoline_remove_prog(tr, node);
-	return bpf_trampoline_update(tr, true /* lock_direct_mutex */, ops, data);
+	err = bpf_trampoline_update(tr, true /* lock_direct_mutex */, ops, data);
+	/*
+	 * If the update above failed, tr->cur_image is unchanged, i.e. ftrace
+	 * (or the direct jump, for the standard ops) is still actually
+	 * calling into an image with node->link->prog's call baked into its
+	 * machine code, even though this unlink is being reported as failed.
+	 * Every caller of this function only WARN_ON_ONCE()'s a failure here
+	 * and then unconditionally frees the underlying bpf_prog, which
+	 * would be a use-after-free the next time that image is executed.
+	 * Pin an extra reference on the prog onto tr->cur_image so it
+	 * outlives this link, mirroring the fix applied to
+	 * bpf_trampoline_multi_attach()/bpf_trampoline_multi_detach().
+	 */
+	if (err && tr->cur_image) {
+		WARN_ON_ONCE(tr->cur_image->pinned_prog);
+		bpf_prog_inc(node->link->prog);
+		tr->cur_image->pinned_prog = node->link->prog;
+	}
+	return err;
 }
 
 /* bpf_trampoline_unlink_prog() should never fail. */
-- 
2.53.0

Re: [PATCH bpf-next v2 3/3] bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Unaddressed Use-After-Free for BPF_TRAMP_REPLACE programs if bpf_arch_text_poke() fails in __bpf_trampoline_unlink_prog.
- [Medium] Dangling pointer and Denial of Service if bpf_arch_text_poke() fails during BPF_TRAMP_REPLACE program attachment.
--

--- Patch [3]: [PATCH bpf-next v2 3/3] bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure ---
commit 58c3b5c6808d88a8686306a02c5a18f2f2dd6edf
Author: Hui Zhu <zhuhui@kylinos.cn>

bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure

This commit fixes a use-after-free issue in __bpf_trampoline_unlink_prog()
by pinning the program to tr->cur_image when bpf_trampoline_update() fails.
This ensures the program outlives the link and prevents a use-after-free
when the image is executed.

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 7fac27374ece1..6071261fd66b8 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -991,7 +991,25 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_node *node,
>  		return err;
>  	}

[Severity: Critical]
This is a pre-existing issue, but does the BPF_TRAMP_REPLACE branch above
suffer from a similar Use-After-Free if bpf_arch_text_poke() fails?

If the text poke fails, this branch returns early and bypasses the newly
added UAF mitigation logic. Because the poke failed, the target function
still contains a live jump to the extension program's JITed code. The caller
(bpf_tracing_link_release) will log a warning and proceed to unconditionally
drop its reference to the bpf_prog, freeing it. Will this result in a
use-after-free the next time the target function is executed?

[Severity: Medium]
This is also a pre-existing issue, but is there a similar failure handling
problem during attachment in __bpf_trampoline_link_prog()?

kernel/bpf/trampoline.c:__bpf_trampoline_link_prog() {
    ...
		tr->extension_prog = node->link->prog;
		return bpf_arch_text_poke(tr->func.addr, BPF_MOD_NOP,
    ...
}

If bpf_arch_text_poke() fails here, tr->extension_prog remains non-NULL
but the program reference is eventually dropped, leaving a dangling pointer.
Any future attach attempts to this trampoline will hit the early
if (tr->extension_prog) check and permanently return -EBUSY. Could this lead
to a local denial of service for the affected tracing hook?

>  	bpf_trampoline_remove_prog(tr, node);
> -	return bpf_trampoline_update(tr, true /* lock_direct_mutex */, ops, data);
> +	err = bpf_trampoline_update(tr, true /* lock_direct_mutex */, ops, data);
> +	/*
> +	 * If the update above failed, tr->cur_image is unchanged, i.e. ftrace
> +	 * (or the direct jump, for the standard ops) is still actually
> +	 * calling into an image with node->link->prog's call baked into its
> +	 * machine code, even though this unlink is being reported as failed.
> +	 * Every caller of this function only WARN_ON_ONCE()'s a failure here
> +	 * and then unconditionally frees the underlying bpf_prog, which
> +	 * would be a use-after-free the next time that image is executed.
> +	 * Pin an extra reference on the prog onto tr->cur_image so it
> +	 * outlives this link, mirroring the fix applied to
> +	 * bpf_trampoline_multi_attach()/bpf_trampoline_multi_detach().
> +	 */
> +	if (err && tr->cur_image) {
> +		WARN_ON_ONCE(tr->cur_image->pinned_prog);
> +		bpf_prog_inc(node->link->prog);
> +		tr->cur_image->pinned_prog = node->link->prog;
> +	}
> +	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785902527.git.zhuhui@kylinos.cn?part=3

Re: [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure

[Jiri Olsa]

On Wed, Aug 05, 2026 at 12:04:05PM +0800, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> This series fixes several use-after-free issues in the BPF trampoline
> multi-attach/detach error paths, where ftrace direct-call updates can
> fail and leave ftrace pointing at freed memory.

hi,
I need to stare at it bit more, but tbh I'm not sure the benefit of
preventing hypothetical crash is worth the extra complexity on the
detach side

IIUC we can't reproduce this error without instrumenting the code, right?

jirka


> 
> Patch 1 addresses two UAF scenarios in bpf_trampoline_multi_detach():
> the single-point unlink failure path (old_image == cur_image) and the
> batch ftrace update failure path. A new pinned_prog field in struct
> bpf_tramp_image keeps the bpf_prog alive while ftrace may still
> reference its image. bpf_trampoline_multi_detach() is made to return
> void, since callers cannot usefully react to failures, and
> bpf_trampoline_put() is taught to leak the trampoline when cur_image
> was left behind by a rollback, so ftrace keeps a valid target.
> 
> Patch 2 fixes a similar UAF in bpf_trampoline_multi_attach() rollback:
> when the register-path undo fails, ftrace still calls into cur_image,
> so the prog is pinned on cur_image instead of being rolled back.
> 
> Patch 3 fixes the common __bpf_trampoline_unlink_prog() path, covering
> both multi (bpf_trampoline_multi_detach) and non-multi
> (bpf_tracing_link_release, bpf_shim_tramp_link_release) callers.
> 
> Hui Zhu (3):
>   bpf: Fix UAF in bpf_trampoline_multi_detach on update failure
>   bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path
>     rollback
>   bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure
> 
>  include/linux/bpf.h      |  20 +++--
>  kernel/bpf/trampoline.c  | 183 +++++++++++++++++++++++++++++++++++----
>  kernel/trace/bpf_trace.c |   2 +-
>  3 files changed, 183 insertions(+), 22 deletions(-)
> 
> Changelog:
> v2:
> Folded v1's two detach patches into patch 1.
> According to the comments of Jiri Olsa, Pin the prog (pinned_prog) on
> cur_image so it stays alive while ftrace may still call into it.
> Make bpf_trampoline_multi_detach() return void.
> Fix the same UAF in standard (non-multi) trampolines.
> According to the comments of sashiko, Fix the prog UAF in
> bpf_trampoline_multi_attach() rollback.
> Leak the trampoline in bpf_trampoline_put() when cur_image is left
> by a rollback.
> 
> -- 
> 2.53.0
> 

Re: [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure

[Hui Zhu]

> 
> On Wed, Aug 05, 2026 at 12:04:05PM +0800, Hui Zhu wrote:
> 
> > 
> > From: Hui Zhu <zhuhui@kylinos.cn>
> >  
> >  This series fixes several use-after-free issues in the BPF trampoline
> >  multi-attach/detach error paths, where ftrace direct-call updates can
> >  fail and leave ftrace pointing at freed memory.
> > 
> hi,
> I need to stare at it bit more, but tbh I'm not sure the benefit of
> preventing hypothetical crash is worth the extra complexity on the
> detach side
> 
> IIUC we can't reproduce this error without instrumenting the code, right?
> 
> jirka

Hi Jiri,

You're right. I went through the failure paths and the realistic
triggers basically don't exist for a normal user:

The allocations are all GFP_KERNEL (reclaim + OOM handle them),
and bpf_jit_charge_modmem() lets CAP_BPF callers exceed the JIT
limit, so ENOMEM doesn't get there.
-E2BIG is attach-time, before cur_image is set, so no UAF.
SHARE_IPMODIFY -EAGAIN needs livepatch on the same function and
is retried in bpf_trampoline_update(); the multi path where it
could escape needs a second failure on the undo del, which doesn't
do ipmodify negotiation, so it doesn't reach the UAF either.
The rest is bugs or not user-driven.

So this is fault-injection territory, and I won't claim it's
a customer bug.

I'd like to drop patches 2 and 3 and the prog-side machinery
(pinned_prog + rollback + the trampoline leak).
And keep only the one-line image-side fix in patch 1: only free
old_image when it differs from cur_image.
It's obviously correct: if cur_image == old_image, ftrace is still
calling into it, so freeing it is wrong. And it costs almost nothing.

Would you prefer I proceed with just this single patch,
or drop the entire series instead?

Best,
Hui

> 
> > 
> > Patch 1 addresses two UAF scenarios in bpf_trampoline_multi_detach():
> >  the single-point unlink failure path (old_image == cur_image) and the
> >  batch ftrace update failure path. A new pinned_prog field in struct
> >  bpf_tramp_image keeps the bpf_prog alive while ftrace may still
> >  reference its image. bpf_trampoline_multi_detach() is made to return
> >  void, since callers cannot usefully react to failures, and
> >  bpf_trampoline_put() is taught to leak the trampoline when cur_image
> >  was left behind by a rollback, so ftrace keeps a valid target.
> >  
> >  Patch 2 fixes a similar UAF in bpf_trampoline_multi_attach() rollback:
> >  when the register-path undo fails, ftrace still calls into cur_image,
> >  so the prog is pinned on cur_image instead of being rolled back.
> >  
> >  Patch 3 fixes the common __bpf_trampoline_unlink_prog() path, covering
> >  both multi (bpf_trampoline_multi_detach) and non-multi
> >  (bpf_tracing_link_release, bpf_shim_tramp_link_release) callers.
> >  
> >  Hui Zhu (3):
> >  bpf: Fix UAF in bpf_trampoline_multi_detach on update failure
> >  bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path
> >  rollback
> >  bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure
> >  
> >  include/linux/bpf.h | 20 +++--
> >  kernel/bpf/trampoline.c | 183 +++++++++++++++++++++++++++++++++++----
> >  kernel/trace/bpf_trace.c | 2 +-
> >  3 files changed, 183 insertions(+), 22 deletions(-)
> >  
> >  Changelog:
> >  v2:
> >  Folded v1's two detach patches into patch 1.
> >  According to the comments of Jiri Olsa, Pin the prog (pinned_prog) on
> >  cur_image so it stays alive while ftrace may still call into it.
> >  Make bpf_trampoline_multi_detach() return void.
> >  Fix the same UAF in standard (non-multi) trampolines.
> >  According to the comments of sashiko, Fix the prog UAF in
> >  bpf_trampoline_multi_attach() rollback.
> >  Leak the trampoline in bpf_trampoline_put() when cur_image is left
> >  by a rollback.
> >  
> >  -- 
> >  2.53.0
> >
>

Re: [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure

[Jiri Olsa]

On Fri, Aug 07, 2026 at 02:00:11AM +0000, Hui Zhu wrote:
> > 
> > On Wed, Aug 05, 2026 at 12:04:05PM +0800, Hui Zhu wrote:
> > 
> > > 
> > > From: Hui Zhu <zhuhui@kylinos.cn>
> > >  
> > >  This series fixes several use-after-free issues in the BPF trampoline
> > >  multi-attach/detach error paths, where ftrace direct-call updates can
> > >  fail and leave ftrace pointing at freed memory.
> > > 
> > hi,
> > I need to stare at it bit more, but tbh I'm not sure the benefit of
> > preventing hypothetical crash is worth the extra complexity on the
> > detach side
> > 
> > IIUC we can't reproduce this error without instrumenting the code, right?
> > 
> > jirka
> 
> Hi Jiri,
> 
> You're right. I went through the failure paths and the realistic
> triggers basically don't exist for a normal user:
> 
> The allocations are all GFP_KERNEL (reclaim + OOM handle them),
> and bpf_jit_charge_modmem() lets CAP_BPF callers exceed the JIT
> limit, so ENOMEM doesn't get there.
> -E2BIG is attach-time, before cur_image is set, so no UAF.
> SHARE_IPMODIFY -EAGAIN needs livepatch on the same function and
> is retried in bpf_trampoline_update(); the multi path where it
> could escape needs a second failure on the undo del, which doesn't
> do ipmodify negotiation, so it doesn't reach the UAF either.
> The rest is bugs or not user-driven.
> 
> So this is fault-injection territory, and I won't claim it's
> a customer bug.
> 
> I'd like to drop patches 2 and 3 and the prog-side machinery
> (pinned_prog + rollback + the trampoline leak).
> And keep only the one-line image-side fix in patch 1: only free
> old_image when it differs from cur_image.

right, that one looks good

> It's obviously correct: if cur_image == old_image, ftrace is still
> calling into it, so freeing it is wrong. And it costs almost nothing.
> 
> Would you prefer I proceed with just this single patch,
> or drop the entire series instead?

also we can change bpf_trampoline_multi_detach to return void
and drop the WARN_ON_ONCE on that call

thanks,
jirka