On Tue, Sep 15, 2020 at 5:50 PM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
>
> From: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
>
> In preparation for allowing multiple attachments of freplace programs, move
> the references to the target program and trampoline into the
> bpf_tracing_link structure when that is created. To do this atomically,
> introduce a new mutex in prog->aux to protect writing to the two pointers
> to target prog and trampoline, and rename the members to make it clear that
> they are related.
>
> With this change, it is no longer possible to attach the same tracing
> program multiple times (detaching in-between), since the reference from the
> tracing program to the target disappears on the first attach. However,
> since the next patch will let the caller supply an attach target, that will
> also make it possible to attach to the same place multiple times.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
> ---
Seems much more straightforward to me with mutex. And I don't have to
worry about various transient NULL states.
Acked-by: Andrii Nakryiko <andriin@xxxxxx>
> include/linux/bpf.h | 15 +++++++++------
> kernel/bpf/btf.c | 6 +++---
> kernel/bpf/core.c | 9 ++++++---
> kernel/bpf/syscall.c | 46 ++++++++++++++++++++++++++++++++++++++++------
> kernel/bpf/trampoline.c | 12 ++++--------
> kernel/bpf/verifier.c | 9 +++++----
> 6 files changed, 67 insertions(+), 30 deletions(-)
>
[...]
> @@ -2583,19 +2598,38 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog)
> &bpf_tracing_link_lops, prog);
> link->attach_type = prog->expected_attach_type;
>
> + mutex_lock(&prog->aux->tgt_mutex);
> +
> + if (!prog->aux->tgt_trampoline) {
> + err = -ENOENT;
> + goto out_unlock;
> + }
> + tr = prog->aux->tgt_trampoline;
> + tgt_prog = prog->aux->tgt_prog;
> +
> err = bpf_link_prime(&link->link, &link_primer);
> if (err) {
> - kfree(link);
> - goto out_put_prog;
> + goto out_unlock;
> }
nit: unnecessary {} now
>
> - err = bpf_trampoline_link_prog(prog);
> + err = bpf_trampoline_link_prog(prog, tr);
> if (err) {
> bpf_link_cleanup(&link_primer);
> - goto out_put_prog;
> + link = NULL;
> + goto out_unlock;
> }
[...]
