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

[Hui Zhu]

From: Hui Zhu <zhuhui@xxxxxxxxxx>

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