On Mon, 2024-09-23 at 21:40 +0800, Leon Hwang wrote:
[...]
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 8a4117f6d7610..18b3f9216b050 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3292,8 +3292,11 @@ static void bpf_tracing_link_release(struct bpf_link *link)
> bpf_trampoline_put(tr_link->trampoline);
>
> /* tgt_prog is NULL if target is a kernel function */
> - if (tr_link->tgt_prog)
> + if (tr_link->tgt_prog) {
> + if (link->prog->type == BPF_PROG_TYPE_EXT)
> + tr_link->tgt_prog->aux->is_extended = false;
> bpf_prog_put(tr_link->tgt_prog);
> + }
> }
>
> static void bpf_tracing_link_dealloc(struct bpf_link *link)
> @@ -3523,6 +3526,8 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> if (prog->aux->dst_trampoline && tr != prog->aux->dst_trampoline)
> /* we allocated a new trampoline, so free the old one */
> bpf_trampoline_put(prog->aux->dst_trampoline);
> + if (prog->type == BPF_PROG_TYPE_EXT)
> + tgt_prog->aux->is_extended = true;
>
> prog->aux->dst_prog = NULL;
> prog->aux->dst_trampoline = NULL;
Sorry, this might be a silly question, I do not fully understand how
programs and trampolines are protected against concurrent update.
Sequence of actions in bpf_tracing_prog_attach():
a. call bpf_trampoline_link_prog(&link->link, tr)
this returns success if `tr->extension_prog` is NULL,
meaning trampoline is "free";
b. update tgt_prog->aux->is_extended = true.
Sequence of actions in bpf_tracing_link_release():
c. call bpf_trampoline_unlink_prog(&tr_link->link, tr_link->trampoline)
this sets `tr->extension_prog` to NULL;
d. update tr_link->tgt_prog->aux->is_extended = false.
In a concurrent environment, is it possible to have actions ordered as:
- thread #1: does bpf_tracing_link_release(link pointing to tgt_prog)
- thread #2: does bpf_tracing_prog_attach(some_prog, tgt_prog)
- thread #1: (c) tr->extension_prog is set to NULL
- thread #2: (a) tr->extension_prog is set to some_prog
- thread #2: (b) tgt_prog->aux->is_extended = true
- thread #1: (d) tgt_prog->aux->is_extended = false
Thus, loosing the is_extended mark?
(As far as I understand bpf_trampoline_compute_key() call in
bpf_tracing_prog_attach() it is possible for threads #1 and #2 to
operate on a same trampoline object).
