> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index a9e328c0a1b3..2986e5cac743 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
[ ... ]
> +int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct
> bpf_tracing_multi_link *link)
> +{
> + struct bpf_tracing_multi_node *mnode;
> + struct fentry_multi_data data = {};
> + int i, cnt = link->nodes_cnt;
> +
> + data.unreg = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
> + data.modify = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
> +
> + if (!data.unreg || !data.modify) {
> + free_fentry_multi_data(&data);
> + return -ENOMEM;
> + }
If alloc_ftrace_hash() fails here under memory pressure, the function
returns -ENOMEM without detaching any trampolines or releasing their
refcounts.
The caller in the next patch (bpf_tracing_multi_link_release) does:
WARN_ON_ONCE(bpf_trampoline_multi_detach(link->prog, tr_link));
This drops the error on the floor. The link struct is then freed by
bpf_tracing_multi_link_dealloc(), but the trampolines remain attached
with elevated refcounts, permanently leaking them and their ftrace
hooks.
Would it make sense to pre-allocate these hashes during
bpf_trampoline_multi_attach() and store them in the link struct so
that the detach path cannot fail?
> +
> + trampoline_lock_all();
[ ... ]
> +rollback_unlink:
> + /*
> + * We can end up in here from 3 points from above code:
> + *
> + * - __bpf_trampoline_link_prog or update_ftrace_direct_add failed and
> + * we have some portion of linked trampolines without ftrace update
> + *
> + * - update_ftrace_direct_mod failed and we have all trampolines linked
> + * plus we already un-attached all new trampolines
> + *
> + * In both cases we need to unlink all trampolines from the new program
> + * and update modified (data.modify) sites, because those have
> previously
> + * some programs attached and the new trampoline needs to get attached.
> + */
This isn't a bug, but the comment says "3 points" and then describes
two scenarios in two bullet points. Maybe say "2 scenarios" or split
the first bullet into two to match?
> + ftrace_hash_clear(data.modify);
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/23480161822
