> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index ae7e4fdfe2a3..957e5d7f9554 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c

[ ... ]

> @@ -1447,6 +1447,277 @@ int __weak arch_bpf_trampoline_size(const struct 
> btf_func_model *m, u32 flags,
>       return -ENOTSUPP;
>  }
>
> +#if defined(CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS) && \
> +    defined(CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS) && \
> +    defined(CONFIG_BPF_SYSCALL)

[ ... ]

> +int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
> +                             struct bpf_tracing_multi_link *link)
> +{
> +     struct bpf_tracing_multi_data *data = &link->data;
> +     struct bpf_attach_target_info tgt_info = {};
> +     struct btf *btf = prog->aux->attach_btf;
> +     struct bpf_tracing_multi_node *mnode;
> +     struct bpf_trampoline *tr;
> +     int i, err, rollback_cnt;
> +     u64 key;
> +
> +     for_each_mnode(mnode, link) {
> +             rollback_cnt = i;
> +
> +             err = bpf_check_attach_btf_id_multi(btf, prog, ids[i], 
> &tgt_info);
> +             if (err)
> +                     goto rollback_put;
> +
> +             key = bpf_trampoline_compute_key(NULL, btf, ids[i]);
> +
> +             tr = bpf_trampoline_get(key, &tgt_info);
> +             if (!tr) {
> +                     err = -ENOMEM;
> +                     goto rollback_put;
> +             }
> +
> +             mnode->trampoline = tr;
> +             mnode->node.link = &link->link;
> +
> +             cond_resched();
> +     }
> +
> +     err = init_tracing_multi_data(data);
> +     if (err) {
> +             rollback_cnt = link->nodes_cnt;
> +             goto rollback_put;
> +     }
> +
> +     trampoline_lock_all();
> +
> +     for_each_mnode(mnode, link) {
> +             bpf_trampoline_multi_attach_init(mnode->trampoline);
> +
> +             data->entry = &mnode->entry;
> +             err = __bpf_trampoline_link_prog(&mnode->node, 
> mnode->trampoline, NULL,
> +                                              &trampoline_multi_ops, data);
> +             if (err) {
> +                     rollback_cnt = i;
> +                     goto rollback_unlink;
> +             }
> +     }

[ ... ]

> +int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct 
> bpf_tracing_multi_link *link)
> +{
> +     struct bpf_tracing_multi_data *data = &link->data;
> +     struct bpf_tracing_multi_node *mnode;
> +     int i;
> +
> +     trampoline_lock_all();
> +
> +     for_each_mnode(mnode, link) {
> +             data->entry = &mnode->entry;
> +             bpf_trampoline_multi_attach_init(mnode->trampoline);
> +             WARN_ON_ONCE(__bpf_trampoline_unlink_prog(&mnode->node, 
> mnode->trampoline,
> +                                     NULL, &trampoline_multi_ops, data));
> +     }
> +
> +     if (ftrace_hash_count(data->unreg))
> +             WARN_ON_ONCE(update_ftrace_direct_del(&direct_ops, 
> data->unreg));
> +     if (ftrace_hash_count(data->modify))
> +             WARN_ON_ONCE(update_ftrace_direct_mod(&direct_ops, 
> data->modify, true));
> +
> +     for_each_mnode(mnode, link)
> +             bpf_trampoline_multi_attach_free(mnode->trampoline);
> +
> +     trampoline_unlock_all();
> +
> +     for_each_mnode(mnode, link)
> +             bpf_trampoline_put(mnode->trampoline);
> +
> +     clear_tracing_multi_data(data);
> +     return 0;
> +}

In v4, [email protected] raised a concern about the detach function
potentially failing due to memory allocation. The concern was that if
alloc_ftrace_hash() failed, the function would return -ENOMEM without
detaching any trampolines or releasing refcounts. Since the caller
(bpf_tracing_multi_link_release) uses WARN_ON_ONCE to drop the error,
the link struct would be freed while trampolines remained attached with
elevated refcounts, permanently leaking them and their ftrace hooks.

The reviewer suggested pre-allocating hashes during attach and storing
them in the link struct so that detach cannot fail.

In v7 (the current code), the bpf_tracing_multi_data structure is
embedded in the link structure and init_tracing_multi_data is called
during bpf_trampoline_multi_attach. The detach function now uses the
pre-allocated data from link->data rather than allocating new hashes,
which appears to resolve the original concern.

Reference: 
https://lore.kernel.org/bpf/711e8b90d27722e47cccfc30a1ccfe243ea7a696322997c15fda1a2147dba...@mail.kernel.org/


---
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/26881310426

Reply via email to