Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] bpf: Use-After-Free of trampoline images when text poke batch fails in detach --
commit 357aa776c5f19ab7b39da8fb3843fb0141fc3b7f Author: Leon Hwang <[email protected]> bpf: Add bpf_trampoline_multi_prog_attach/detach() This commit introduces multi-attach and detach helpers for BPF trampolines. These helpers prepare target trampolines, apply text pokes in batch, and commit the staged trampoline images. > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c > --- a/kernel/bpf/trampoline.c > +++ b/kernel/bpf/trampoline.c [ ... ] > +static void bpf_trampoline_multi_prog_rollback(struct bpf_trampoline *tr) > +{ > + if (tr->multi_attach.new_image) > + bpf_tramp_image_put(tr->multi_attach.new_image); > + > + tr->flags = tr->multi_attach.old_flags; > + tr->multi_attach.old_image = NULL; [Severity: Critical] If __bpf_trampoline_unlink_prog() fails and calls bpf_trampoline_multi_prog_rollback(), old_image is cleared for the node. Does this guarantee that a subsequent bpf_text_poke_batch() will fail due to a NOP expectation mismatch? > + tr->multi_attach.new_image = NULL; > + tr->multi_attach.old_flags = 0; > +} [ ... ] > +void bpf_trampoline_multi_prog_detach(struct bpf_tracing_multi_link *link) > +{ > + struct bpf_tracing_multi_node *mnode; > + int i, err; > + > + trampoline_lock_all(); > + > + for_each_mnode(mnode, link) { > + bpf_trampoline_multi_attach_init(mnode->trampoline); > + err = __bpf_trampoline_unlink_prog(&mnode->node, > mnode->trampoline, NULL, > + &trampoline_multi_prog_ops, > NULL); > + if (WARN_ONCE(err, "__bpf_trampoline_unlink_prog failed: %d\n", > err)) > + bpf_trampoline_multi_prog_rollback(mnode->trampoline); > + } > + > + for_each_mnode(mnode, link) { > + link->pokes[i] = &mnode->poke; > + bpf_text_poke_init(link->pokes[i], mnode->trampoline); > + } > + > + WARN_ON_ONCE(bpf_text_poke_batch(link->pokes, link->nodes_cnt)); [Severity: Critical] If bpf_text_poke_batch() fails here (for example, due to a hardware or memory error during patching, or a NOP mismatch from a prior unlink failure), the failure is wrapped in WARN_ON_ONCE but otherwise ignored. The code then proceeds to unconditionally call bpf_trampoline_multi_prog_commit() for all nodes, which frees old_image via bpf_tramp_image_put(). Since the kernel text remains unmodified after a failed poke batch, it will continue to jump to the freed old_image. Can this lead to a use-after-free on traced paths? > + > + for_each_mnode(mnode, link) > + bpf_trampoline_multi_prog_commit(mnode->trampoline); > + > + trampoline_unlock_all(); > + > + for_each_mnode(mnode, link) > + bpf_trampoline_put(mnode->trampoline); > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
