The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure
@ 2026-08-05  4:04 Hui Zhu
  2026-08-05  4:04 ` [PATCH bpf-next v2 1/3] bpf: Fix UAF in bpf_trampoline_multi_detach " Hui Zhu
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Hui Zhu @ 2026-08-05  4:04 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, KP Singh, Matt Bobrowski,
	Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, bpf,
	linux-kernel, linux-trace-kernel
  Cc: 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH bpf-next v2 1/3] bpf: Fix UAF in bpf_trampoline_multi_detach on update failure
  2026-08-05  4:04 [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure Hui Zhu
@ 2026-08-05  4:04 ` Hui Zhu
  2026-08-05  4:04 ` [PATCH bpf-next v2 2/3] bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path rollback Hui Zhu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Hui Zhu @ 2026-08-05  4:04 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, KP Singh, Matt Bobrowski,
	Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, bpf,
	linux-kernel, linux-trace-kernel
  Cc: 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH bpf-next v2 2/3] bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path rollback
  2026-08-05  4:04 [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure Hui Zhu
  2026-08-05  4:04 ` [PATCH bpf-next v2 1/3] bpf: Fix UAF in bpf_trampoline_multi_detach " Hui Zhu
@ 2026-08-05  4:04 ` Hui Zhu
  2026-08-05  4:04 ` [PATCH bpf-next v2 3/3] bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure Hui Zhu
  2026-08-06  8:25 ` [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach " Jiri Olsa
  3 siblings, 0 replies; 7+ messages in thread
From: Hui Zhu @ 2026-08-05  4:04 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, KP Singh, Matt Bobrowski,
	Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, bpf,
	linux-kernel, linux-trace-kernel
  Cc: 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH bpf-next v2 3/3] bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure
  2026-08-05  4:04 [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure Hui Zhu
  2026-08-05  4:04 ` [PATCH bpf-next v2 1/3] bpf: Fix UAF in bpf_trampoline_multi_detach " Hui Zhu
  2026-08-05  4:04 ` [PATCH bpf-next v2 2/3] bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path rollback Hui Zhu
@ 2026-08-05  4:04 ` Hui Zhu
  2026-08-06  8:25 ` [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach " Jiri Olsa
  3 siblings, 0 replies; 7+ messages in thread
From: Hui Zhu @ 2026-08-05  4:04 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, KP Singh, Matt Bobrowski,
	Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, bpf,
	linux-kernel, linux-trace-kernel
  Cc: 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure
  2026-08-05  4:04 [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure Hui Zhu
                   ` (2 preceding siblings ...)
  2026-08-05  4:04 ` [PATCH bpf-next v2 3/3] bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure Hui Zhu
@ 2026-08-06  8:25 ` Jiri Olsa
  2026-08-07  2:00   ` Hui Zhu
  3 siblings, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2026-08-06  8:25 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Emil Tsalapatis,
	Ihor Solodrai, KP Singh, Matt Bobrowski, Steven Rostedt,
	Masami Hiramatsu, Mathieu Desnoyers, bpf, linux-kernel,
	linux-trace-kernel, 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


> 
> 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
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure
  2026-08-06  8:25 ` [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach " Jiri Olsa
@ 2026-08-07  2:00   ` Hui Zhu
  2026-08-07  8:19     ` Jiri Olsa
  0 siblings, 1 reply; 7+ messages in thread
From: Hui Zhu @ 2026-08-07  2:00 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Emil Tsalapatis,
	Ihor Solodrai, KP Singh, Matt Bobrowski, Steven Rostedt,
	Masami Hiramatsu, Mathieu Desnoyers, bpf, linux-kernel,
	linux-trace-kernel, 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
> >
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure
  2026-08-07  2:00   ` Hui Zhu
@ 2026-08-07  8:19     ` Jiri Olsa
  0 siblings, 0 replies; 7+ messages in thread
From: Jiri Olsa @ 2026-08-07  8:19 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Emil Tsalapatis,
	Ihor Solodrai, KP Singh, Matt Bobrowski, Steven Rostedt,
	Masami Hiramatsu, Mathieu Desnoyers, bpf, linux-kernel,
	linux-trace-kernel, Hui Zhu

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-07  8:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05  4:04 [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach on update failure Hui Zhu
2026-08-05  4:04 ` [PATCH bpf-next v2 1/3] bpf: Fix UAF in bpf_trampoline_multi_detach " Hui Zhu
2026-08-05  4:04 ` [PATCH bpf-next v2 2/3] bpf: Fix prog UAF in bpf_trampoline_multi_attach() register-path rollback Hui Zhu
2026-08-05  4:04 ` [PATCH bpf-next v2 3/3] bpf: Fix prog UAF in __bpf_trampoline_unlink_prog() on update failure Hui Zhu
2026-08-06  8:25 ` [PATCH bpf-next v2 0/3] bpf: Fix UAF in bpf_trampoline_multi_attach/detach " Jiri Olsa
2026-08-07  2:00   ` Hui Zhu
2026-08-07  8:19     ` Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox