On Mon, Apr 12, 2021 at 9:28 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> Currently we don't allow re-attaching of trampolines. Once
> it's detached, it can't be re-attach even when the program
> is still loaded.
>
> Adding the possibility to re-attach the loaded tracing and
> lsm programs.
>
> Fixing missing unlock with proper cleanup goto jump reported
> by Julia.
>
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Reported-by: Julia Lawall <julia.lawall@xxxxxxx>
> Acked-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> kernel/bpf/syscall.c | 23 +++++++++++++++++------
> kernel/bpf/trampoline.c | 2 +-
> 2 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6428634da57e..f02c6a871b4f 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2645,14 +2645,25 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> * target_btf_id using the link_create API.
> *
> * - if tgt_prog == NULL when this function was called using the old
> - * raw_tracepoint_open API, and we need a target from prog->aux
> - *
> - * The combination of no saved target in prog->aux, and no target
> - * specified on load is illegal, and we reject that here.
> + * raw_tracepoint_open API, and we need a target from prog->aux
> + *
> + * - if prog->aux->dst_trampoline and tgt_prog is NULL, the program
> + * was detached and is going for re-attachment.
> */
> if (!prog->aux->dst_trampoline && !tgt_prog) {
> - err = -ENOENT;
> - goto out_unlock;
> + /*
> + * Allow re-attach for TRACING and LSM programs. If it's
> + * currently linked, bpf_trampoline_link_prog will fail.
> + * EXT programs need to specify tgt_prog_fd, so they
> + * re-attach in separate code path.
> + */
> + if (prog->type != BPF_PROG_TYPE_TRACING &&
> + prog->type != BPF_PROG_TYPE_LSM) {
> + err = -EINVAL;
> + goto out_unlock;
> + }
> + btf_id = prog->aux->attach_btf_id;
> + key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf, btf_id);
> }
>
> if (!prog->aux->dst_trampoline ||
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 1f3a4be4b175..48b8b9916aa2 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -437,7 +437,7 @@ int bpf_trampoline_unlink_prog(struct bpf_prog *prog, struct bpf_trampoline *tr)
> tr->extension_prog = NULL;
> goto out;
> }
> - hlist_del(&prog->aux->tramp_hlist);
> + hlist_del_init(&prog->aux->tramp_hlist);
there is another hlist_del few lines above in error handling path of
bpf_trampoline_link_prog(), it should probably be also updated to
hlist_del_init(), no?
> tr->progs_cnt[kind]--;
> err = bpf_trampoline_update(tr);
> out:
> --
> 2.30.2
>
