Re: [PATCH bpf-next v2 09/21] bpf: Add bpf_trampoline_multi_prog_attach/detach()

[sashiko-bot@kernel.org]

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 <leon.hwang@linux.dev>

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/20261012154024.29423-1-leon.hwang@linux.dev?part=9