From: Hui Zhu 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 --- 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