| Message ID | 160051618733.58048.1005452269573858636.stgit@toke.dk |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: Support multi-attach for freplace programs | expand |
On Sat, Sep 19, 2020 at 4:50 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > > From: Toke Høiland-Jørgensen <toke@redhat.com> > > 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. > > Acked-by: Andrii Nakryiko <andriin@fb.com> > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> > --- > include/linux/bpf.h | 15 +++++++++----- > kernel/bpf/btf.c | 6 +++--- > kernel/bpf/core.c | 9 ++++++--- > kernel/bpf/syscall.c | 49 +++++++++++++++++++++++++++++++++++++++-------- > kernel/bpf/trampoline.c | 12 ++++-------- > kernel/bpf/verifier.c | 9 +++++---- > 6 files changed, 68 insertions(+), 32 deletions(-) > [...] > @@ -741,7 +743,9 @@ struct bpf_prog_aux { > u32 max_rdonly_access; > u32 max_rdwr_access; > const struct bpf_ctx_arg_aux *ctx_arg_info; > - struct bpf_prog *linked_prog; > + struct mutex tgt_mutex; /* protects writing of tgt_* pointers below */ nit: not just writing, "accessing" would be a better word > + struct bpf_prog *tgt_prog; > + struct bpf_trampoline *tgt_trampoline; > bool verifier_zext; /* Zero extensions has been inserted by verifier. */ > bool offload_requested; > bool attach_btf_trace; /* true if attaching to BTF-enabled raw tp */ [...] > static bool may_access_direct_pkt_data(struct bpf_verifier_env *env, > @@ -11418,8 +11417,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, > static int check_attach_btf_id(struct bpf_verifier_env *env) > { > struct bpf_prog *prog = env->prog; > - struct bpf_prog *tgt_prog = prog->aux->linked_prog; > u32 btf_id = prog->aux->attach_btf_id; > + struct bpf_prog *tgt_prog = prog->aux->tgt_prog; > struct btf_func_model fmodel; > struct bpf_trampoline *tr; > const struct btf_type *t; > @@ -11483,7 +11482,9 @@ static int check_attach_btf_id(struct bpf_verifier_env *env) > if (!tr) > return -ENOMEM; > > - prog->aux->trampoline = tr; > + mutex_lock(&prog->aux->tgt_mutex); > + prog->aux->tgt_trampoline = tr; > + mutex_unlock(&prog->aux->tgt_mutex); I think here you don't need to lock mutex, because check_attach_btf_id() is called during verification before bpf_prog itself is visible to user-space, so there is no way to have concurrent access to it. If that wasn't the case, you'd need to take mutex lock before you assign tgt_prog local variable from prog->aux->tgt_prog above (and plus you'd need extra null checks and stuff). > return 0; > } > >
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > On Sat, Sep 19, 2020 at 4:50 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote: >> >> From: Toke Høiland-Jørgensen <toke@redhat.com> >> >> 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. >> >> Acked-by: Andrii Nakryiko <andriin@fb.com> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >> --- >> include/linux/bpf.h | 15 +++++++++----- >> kernel/bpf/btf.c | 6 +++--- >> kernel/bpf/core.c | 9 ++++++--- >> kernel/bpf/syscall.c | 49 +++++++++++++++++++++++++++++++++++++++-------- >> kernel/bpf/trampoline.c | 12 ++++-------- >> kernel/bpf/verifier.c | 9 +++++---- >> 6 files changed, 68 insertions(+), 32 deletions(-) >> > > [...] > >> @@ -741,7 +743,9 @@ struct bpf_prog_aux { >> u32 max_rdonly_access; >> u32 max_rdwr_access; >> const struct bpf_ctx_arg_aux *ctx_arg_info; >> - struct bpf_prog *linked_prog; >> + struct mutex tgt_mutex; /* protects writing of tgt_* pointers below */ > > nit: not just writing, "accessing" would be a better word Except it's not, really: the values are read without taking the mutex. This is fine because it is done in the verifier before the bpf_prog is visible to the rest of the kernel, but saying the mutex protects all accesses would be misleading, I think. I guess I could change it to "protects access to tgt_* pointers after prog becomes visible" or somesuch... >> + struct bpf_prog *tgt_prog; >> + struct bpf_trampoline *tgt_trampoline; >> bool verifier_zext; /* Zero extensions has been inserted by verifier. */ >> bool offload_requested; >> bool attach_btf_trace; /* true if attaching to BTF-enabled raw tp */ > > [...] > >> static bool may_access_direct_pkt_data(struct bpf_verifier_env *env, >> @@ -11418,8 +11417,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, >> static int check_attach_btf_id(struct bpf_verifier_env *env) >> { >> struct bpf_prog *prog = env->prog; >> - struct bpf_prog *tgt_prog = prog->aux->linked_prog; >> u32 btf_id = prog->aux->attach_btf_id; >> + struct bpf_prog *tgt_prog = prog->aux->tgt_prog; >> struct btf_func_model fmodel; >> struct bpf_trampoline *tr; >> const struct btf_type *t; >> @@ -11483,7 +11482,9 @@ static int check_attach_btf_id(struct bpf_verifier_env *env) >> if (!tr) >> return -ENOMEM; >> >> - prog->aux->trampoline = tr; >> + mutex_lock(&prog->aux->tgt_mutex); >> + prog->aux->tgt_trampoline = tr; >> + mutex_unlock(&prog->aux->tgt_mutex); > > I think here you don't need to lock mutex, because > check_attach_btf_id() is called during verification before bpf_prog > itself is visible to user-space, so there is no way to have concurrent > access to it. If that wasn't the case, you'd need to take mutex lock > before you assign tgt_prog local variable from prog->aux->tgt_prog > above (and plus you'd need extra null checks and stuff). Yeah, I did realise that (see above), but put it in because it doesn't hurt, and it makes the comment above (about protecting writing) actually be true :) But changing the wording to include 'after it becomes visible' would also fix this, so I'll remove the locking here... -Toke
On Tue, Sep 22, 2020 at 3:17 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > > Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > > > On Sat, Sep 19, 2020 at 4:50 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > >> > >> From: Toke Høiland-Jørgensen <toke@redhat.com> > >> > >> 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. > >> > >> Acked-by: Andrii Nakryiko <andriin@fb.com> > >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> > >> --- > >> include/linux/bpf.h | 15 +++++++++----- > >> kernel/bpf/btf.c | 6 +++--- > >> kernel/bpf/core.c | 9 ++++++--- > >> kernel/bpf/syscall.c | 49 +++++++++++++++++++++++++++++++++++++++-------- > >> kernel/bpf/trampoline.c | 12 ++++-------- > >> kernel/bpf/verifier.c | 9 +++++---- > >> 6 files changed, 68 insertions(+), 32 deletions(-) > >> > > > > [...] > > > >> @@ -741,7 +743,9 @@ struct bpf_prog_aux { > >> u32 max_rdonly_access; > >> u32 max_rdwr_access; > >> const struct bpf_ctx_arg_aux *ctx_arg_info; > >> - struct bpf_prog *linked_prog; > >> + struct mutex tgt_mutex; /* protects writing of tgt_* pointers below */ > > > > nit: not just writing, "accessing" would be a better word > > Except it's not, really: the values are read without taking the mutex. Huh? So you are taking a mutex in bpf_tracing_prog_attach before reading prog->aux->tgt_prog and prog->aux->tgt_trampoline just for fun?.. Why don't you read those pointers outside of mutex and let's have discussion about race conditions? > This is fine because it is done in the verifier before the bpf_prog is > visible to the rest of the kernel, but saying the mutex protects all > accesses would be misleading, I think. > Of course you don't need to take lock while you are constructing bpf_prog... It's like taking a lock inside a constructor in C++ before the outside world can ever access object's fields. No harm, but also pointless. > I guess I could change it to "protects access to tgt_* pointers after > prog becomes visible" or somesuch... > > >> + struct bpf_prog *tgt_prog; > >> + struct bpf_trampoline *tgt_trampoline; > >> bool verifier_zext; /* Zero extensions has been inserted by verifier. */ > >> bool offload_requested; > >> bool attach_btf_trace; /* true if attaching to BTF-enabled raw tp */ > > > > [...] > > > >> static bool may_access_direct_pkt_data(struct bpf_verifier_env *env, > >> @@ -11418,8 +11417,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, > >> static int check_attach_btf_id(struct bpf_verifier_env *env) > >> { > >> struct bpf_prog *prog = env->prog; > >> - struct bpf_prog *tgt_prog = prog->aux->linked_prog; > >> u32 btf_id = prog->aux->attach_btf_id; > >> + struct bpf_prog *tgt_prog = prog->aux->tgt_prog; > >> struct btf_func_model fmodel; > >> struct bpf_trampoline *tr; > >> const struct btf_type *t; > >> @@ -11483,7 +11482,9 @@ static int check_attach_btf_id(struct bpf_verifier_env *env) > >> if (!tr) > >> return -ENOMEM; > >> > >> - prog->aux->trampoline = tr; > >> + mutex_lock(&prog->aux->tgt_mutex); > >> + prog->aux->tgt_trampoline = tr; > >> + mutex_unlock(&prog->aux->tgt_mutex); > > > > I think here you don't need to lock mutex, because > > check_attach_btf_id() is called during verification before bpf_prog > > itself is visible to user-space, so there is no way to have concurrent > > access to it. If that wasn't the case, you'd need to take mutex lock > > before you assign tgt_prog local variable from prog->aux->tgt_prog > > above (and plus you'd need extra null checks and stuff). > > Yeah, I did realise that (see above), but put it in because it doesn't > hurt, and it makes the comment above (about protecting writing) actually > be true :) > See above about locking in a constructor analogy. But as is the code is split-brained: it accesses prog->aux->tgt_prog outside of mutex and prog->aux->tgt_trampoline inside the mutex. So when reading this the natural question is why it's not one way of doing things. Reading a field without a lock held is (in general) just as wrong as updating that field without lock. > But changing the wording to include 'after it becomes visible' would > also fix this, so I'll remove the locking here... I'd leave a comment that check_attach_btf_id is only called during BPF_PROG_LOAD before bpf_prog is visible to user-space, so there is no possibility of concurrently accessing its fields yet. That would make it clear why we don't need a lock. > > -Toke >
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > On Tue, Sep 22, 2020 at 3:17 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote: >> >> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: >> >> > On Sat, Sep 19, 2020 at 4:50 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote: >> >> >> >> From: Toke Høiland-Jørgensen <toke@redhat.com> >> >> >> >> 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. >> >> >> >> Acked-by: Andrii Nakryiko <andriin@fb.com> >> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >> >> --- >> >> include/linux/bpf.h | 15 +++++++++----- >> >> kernel/bpf/btf.c | 6 +++--- >> >> kernel/bpf/core.c | 9 ++++++--- >> >> kernel/bpf/syscall.c | 49 +++++++++++++++++++++++++++++++++++++++-------- >> >> kernel/bpf/trampoline.c | 12 ++++-------- >> >> kernel/bpf/verifier.c | 9 +++++---- >> >> 6 files changed, 68 insertions(+), 32 deletions(-) >> >> >> > >> > [...] >> > >> >> @@ -741,7 +743,9 @@ struct bpf_prog_aux { >> >> u32 max_rdonly_access; >> >> u32 max_rdwr_access; >> >> const struct bpf_ctx_arg_aux *ctx_arg_info; >> >> - struct bpf_prog *linked_prog; >> >> + struct mutex tgt_mutex; /* protects writing of tgt_* pointers below */ >> > >> > nit: not just writing, "accessing" would be a better word >> >> Except it's not, really: the values are read without taking the mutex. > > Huh? So you are taking a mutex in bpf_tracing_prog_attach before > reading prog->aux->tgt_prog and prog->aux->tgt_trampoline just for > fun?.. Why don't you read those pointers outside of mutex and let's > have discussion about race conditions? No, of course not. What I meant was that not everywhere that accesses the field takes the lock; it's not just check_attach_btf_id(), it's also resolve_prog_type() which is called from several places. Of course, as you point out this is all technically part of the 'constructor', as it's all inside the verifier. But you have to keep quite a lot of mental state to realise that, so I thought it would be confusing to have a comment that says the mutex protects all accesses to the field, and then have a bunch of places access the field without holding the mutex. However, just adding the "*after* prog becomes visible" bit to the comment helps with that, so I'm not arguing for keeping it, just explaining why I did it the way I did initially :) -Toke
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 9d444021f160..7aabea7fab31 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -614,8 +614,8 @@ static __always_inline unsigned int bpf_dispatcher_nop_func( } #ifdef CONFIG_BPF_JIT struct bpf_trampoline *bpf_trampoline_lookup(u64 key); -int bpf_trampoline_link_prog(struct bpf_prog *prog); -int bpf_trampoline_unlink_prog(struct bpf_prog *prog); +int bpf_trampoline_link_prog(struct bpf_prog *prog, struct bpf_trampoline *tr); +int bpf_trampoline_unlink_prog(struct bpf_prog *prog, struct bpf_trampoline *tr); struct bpf_trampoline *bpf_trampoline_get(u64 key, void *addr, struct btf_func_model *fmodel); void bpf_trampoline_put(struct bpf_trampoline *tr); @@ -666,11 +666,13 @@ static inline struct bpf_trampoline *bpf_trampoline_lookup(u64 key) { return NULL; } -static inline int bpf_trampoline_link_prog(struct bpf_prog *prog) +static inline int bpf_trampoline_link_prog(struct bpf_prog *prog, + struct bpf_trampoline *tr) { return -ENOTSUPP; } -static inline int bpf_trampoline_unlink_prog(struct bpf_prog *prog) +static inline int bpf_trampoline_unlink_prog(struct bpf_prog *prog, + struct bpf_trampoline *tr) { return -ENOTSUPP; } @@ -741,7 +743,9 @@ struct bpf_prog_aux { u32 max_rdonly_access; u32 max_rdwr_access; const struct bpf_ctx_arg_aux *ctx_arg_info; - struct bpf_prog *linked_prog; + struct mutex tgt_mutex; /* protects writing of tgt_* pointers below */ + struct bpf_prog *tgt_prog; + struct bpf_trampoline *tgt_trampoline; bool verifier_zext; /* Zero extensions has been inserted by verifier. */ bool offload_requested; bool attach_btf_trace; /* true if attaching to BTF-enabled raw tp */ @@ -749,7 +753,6 @@ struct bpf_prog_aux { bool sleepable; bool tail_call_reachable; enum bpf_tramp_prog_type trampoline_prog_type; - struct bpf_trampoline *trampoline; struct hlist_node tramp_hlist; /* BTF_KIND_FUNC_PROTO for valid attach_btf_id */ const struct btf_type *attach_func_proto; diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 2ace56c99c36..9228af9917a8 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -3706,7 +3706,7 @@ struct btf *btf_parse_vmlinux(void) struct btf *bpf_prog_get_target_btf(const struct bpf_prog *prog) { - struct bpf_prog *tgt_prog = prog->aux->linked_prog; + struct bpf_prog *tgt_prog = prog->aux->tgt_prog; if (tgt_prog) { return tgt_prog->aux->btf; @@ -3733,7 +3733,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, struct bpf_insn_access_aux *info) { const struct btf_type *t = prog->aux->attach_func_proto; - struct bpf_prog *tgt_prog = prog->aux->linked_prog; + struct bpf_prog *tgt_prog = prog->aux->tgt_prog; struct btf *btf = bpf_prog_get_target_btf(prog); const char *tname = prog->aux->attach_func_name; struct bpf_verifier_log *log = info->log; @@ -4572,7 +4572,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog, return -EFAULT; } if (prog_type == BPF_PROG_TYPE_EXT) - prog_type = prog->aux->linked_prog->type; + prog_type = prog->aux->tgt_prog->type; t = btf_type_by_id(btf, t->type); if (!t || !btf_type_is_func_proto(t)) { diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index c4811b139caa..0eb5f7501e29 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -99,6 +99,7 @@ struct bpf_prog *bpf_prog_alloc_no_stats(unsigned int size, gfp_t gfp_extra_flag INIT_LIST_HEAD_RCU(&fp->aux->ksym.lnode); mutex_init(&fp->aux->used_maps_mutex); + mutex_init(&fp->aux->tgt_mutex); return fp; } @@ -255,6 +256,7 @@ void __bpf_prog_free(struct bpf_prog *fp) { if (fp->aux) { mutex_destroy(&fp->aux->used_maps_mutex); + mutex_destroy(&fp->aux->tgt_mutex); free_percpu(fp->aux->stats); kfree(fp->aux->poke_tab); kfree(fp->aux); @@ -2138,7 +2140,8 @@ static void bpf_prog_free_deferred(struct work_struct *work) if (aux->prog->has_callchain_buf) put_callchain_buffers(); #endif - bpf_trampoline_put(aux->trampoline); + if (aux->tgt_trampoline) + bpf_trampoline_put(aux->tgt_trampoline); for (i = 0; i < aux->func_cnt; i++) bpf_jit_free(aux->func[i]); if (aux->func_cnt) { @@ -2154,8 +2157,8 @@ void bpf_prog_free(struct bpf_prog *fp) { struct bpf_prog_aux *aux = fp->aux; - if (aux->linked_prog) - bpf_prog_put(aux->linked_prog); + if (aux->tgt_prog) + bpf_prog_put(aux->tgt_prog); INIT_WORK(&aux->work, bpf_prog_free_deferred); schedule_work(&aux->work); } diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 2ce32cad5c8e..4af35a59d0d9 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -2161,7 +2161,9 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr) err = PTR_ERR(tgt_prog); goto free_prog_nouncharge; } - prog->aux->linked_prog = tgt_prog; + mutex_lock(&prog->aux->tgt_mutex); + prog->aux->tgt_prog = tgt_prog; + mutex_unlock(&prog->aux->tgt_mutex); } prog->aux->offload_requested = !!attr->prog_ifindex; @@ -2498,11 +2500,22 @@ struct bpf_link *bpf_link_get_from_fd(u32 ufd) struct bpf_tracing_link { struct bpf_link link; enum bpf_attach_type attach_type; + struct bpf_trampoline *trampoline; + struct bpf_prog *tgt_prog; }; static void bpf_tracing_link_release(struct bpf_link *link) { - WARN_ON_ONCE(bpf_trampoline_unlink_prog(link->prog)); + struct bpf_tracing_link *tr_link = + container_of(link, struct bpf_tracing_link, link); + + WARN_ON_ONCE(bpf_trampoline_unlink_prog(link->prog, + tr_link->trampoline)); + + bpf_trampoline_put(tr_link->trampoline); + + if (tr_link->tgt_prog) + bpf_prog_put(tr_link->tgt_prog); } static void bpf_tracing_link_dealloc(struct bpf_link *link) @@ -2545,7 +2558,9 @@ static const struct bpf_link_ops bpf_tracing_link_lops = { static int bpf_tracing_prog_attach(struct bpf_prog *prog) { struct bpf_link_primer link_primer; + struct bpf_prog *tgt_prog = NULL; struct bpf_tracing_link *link; + struct bpf_trampoline *tr; int err; switch (prog->type) { @@ -2583,19 +2598,37 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog) &bpf_tracing_link_lops, prog); link->attach_type = prog->expected_attach_type; - err = bpf_link_prime(&link->link, &link_primer); - if (err) { - kfree(link); - goto out_put_prog; + 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) + goto out_unlock; - 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; } + link->tgt_prog = tgt_prog; + link->trampoline = tr; + + prog->aux->tgt_prog = NULL; + prog->aux->tgt_trampoline = NULL; + mutex_unlock(&prog->aux->tgt_mutex); + return bpf_link_settle(&link_primer); +out_unlock: + mutex_unlock(&prog->aux->tgt_mutex); + kfree(link); out_put_prog: bpf_prog_put(prog); return err; diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c index e86d32f7f7dc..3145615647a5 100644 --- a/kernel/bpf/trampoline.c +++ b/kernel/bpf/trampoline.c @@ -261,14 +261,12 @@ static enum bpf_tramp_prog_type bpf_attach_type_to_tramp(struct bpf_prog *prog) } } -int bpf_trampoline_link_prog(struct bpf_prog *prog) +int bpf_trampoline_link_prog(struct bpf_prog *prog, struct bpf_trampoline *tr) { enum bpf_tramp_prog_type kind; - struct bpf_trampoline *tr; int err = 0; int cnt; - tr = prog->aux->trampoline; kind = bpf_attach_type_to_tramp(prog); mutex_lock(&tr->mutex); if (tr->extension_prog) { @@ -301,7 +299,7 @@ int bpf_trampoline_link_prog(struct bpf_prog *prog) } hlist_add_head(&prog->aux->tramp_hlist, &tr->progs_hlist[kind]); tr->progs_cnt[kind]++; - err = bpf_trampoline_update(prog->aux->trampoline); + err = bpf_trampoline_update(tr); if (err) { hlist_del(&prog->aux->tramp_hlist); tr->progs_cnt[kind]--; @@ -312,13 +310,11 @@ int bpf_trampoline_link_prog(struct bpf_prog *prog) } /* bpf_trampoline_unlink_prog() should never fail. */ -int bpf_trampoline_unlink_prog(struct bpf_prog *prog) +int bpf_trampoline_unlink_prog(struct bpf_prog *prog, struct bpf_trampoline *tr) { enum bpf_tramp_prog_type kind; - struct bpf_trampoline *tr; int err; - tr = prog->aux->trampoline; kind = bpf_attach_type_to_tramp(prog); mutex_lock(&tr->mutex); if (kind == BPF_TRAMP_REPLACE) { @@ -330,7 +326,7 @@ int bpf_trampoline_unlink_prog(struct bpf_prog *prog) } hlist_del(&prog->aux->tramp_hlist); tr->progs_cnt[kind]--; - err = bpf_trampoline_update(prog->aux->trampoline); + err = bpf_trampoline_update(tr); out: mutex_unlock(&tr->mutex); return err; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 412b0810807f..7a53736e67b4 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -2635,8 +2635,7 @@ static int check_map_access(struct bpf_verifier_env *env, u32 regno, static enum bpf_prog_type resolve_prog_type(struct bpf_prog *prog) { - return prog->aux->linked_prog ? prog->aux->linked_prog->type - : prog->type; + return prog->aux->tgt_prog ? prog->aux->tgt_prog->type : prog->type; } static bool may_access_direct_pkt_data(struct bpf_verifier_env *env, @@ -11418,8 +11417,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, static int check_attach_btf_id(struct bpf_verifier_env *env) { struct bpf_prog *prog = env->prog; - struct bpf_prog *tgt_prog = prog->aux->linked_prog; u32 btf_id = prog->aux->attach_btf_id; + struct bpf_prog *tgt_prog = prog->aux->tgt_prog; struct btf_func_model fmodel; struct bpf_trampoline *tr; const struct btf_type *t; @@ -11483,7 +11482,9 @@ static int check_attach_btf_id(struct bpf_verifier_env *env) if (!tr) return -ENOMEM; - prog->aux->trampoline = tr; + mutex_lock(&prog->aux->tgt_mutex); + prog->aux->tgt_trampoline = tr; + mutex_unlock(&prog->aux->tgt_mutex); return 0; }