Message ID | 20220508032117.2783209-4-kuifeng@fb.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | BPF |
Headers | show |
Series | Attach a cookie to a tracing program. | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Clearly marked for bpf-next, async |
netdev/apply | fail | Patch does not apply to bpf-next |
bpf/vmtest-bpf-next-PR | fail | merge-conflict |
On Sat, May 7, 2022 at 8:21 PM Kui-Feng Lee <kuifeng@fb.com> wrote: > > Pass a cookie along with BPF_LINK_CREATE requests. > > Add a bpf_cookie field to struct bpf_tracing_link to attach a cookie. > The cookie of a bpf_tracing_link is available by calling > bpf_get_attach_cookie when running the BPF program of the attached > link. > > The value of a cookie will be set at bpf_tramp_run_ctx by the > trampoline of the link. > > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com> > --- > arch/x86/net/bpf_jit_comp.c | 12 ++++++++++-- > include/linux/bpf.h | 1 + > include/uapi/linux/bpf.h | 9 +++++++++ > kernel/bpf/bpf_lsm.c | 17 +++++++++++++++++ > kernel/bpf/syscall.c | 12 ++++++++---- > kernel/bpf/trampoline.c | 7 +++++-- > kernel/trace/bpf_trace.c | 17 +++++++++++++++++ > tools/include/uapi/linux/bpf.h | 9 +++++++++ > 8 files changed, 76 insertions(+), 8 deletions(-) > LGTM with a suggestion for some follow up clean up. Acked-by: Andrii Nakryiko <andrii@kernel.org> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index bf4576a6938c..52a5eba2d5e8 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -1764,13 +1764,21 @@ static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog, > struct bpf_tramp_link *l, int stack_size, > bool save_ret) > { > + u64 cookie = 0; > u8 *prog = *pprog; > u8 *jmp_insn; > int ctx_cookie_off = offsetof(struct bpf_tramp_run_ctx, bpf_cookie); > struct bpf_prog *p = l->link.prog; > > - /* mov rdi, 0 */ > - emit_mov_imm64(&prog, BPF_REG_1, 0, 0); > + if (l->link.type == BPF_LINK_TYPE_TRACING) { It would probably be nicer to put cookie field into struct bpf_tramp_link instead so that the JIT compiler doesn't have to do this special handling. It also makes sense that struct bpf_trampoline *trampoline is moved into struct bpf_tramp_link itself (given trampoline is always there for bpf_tramp_link). > + struct bpf_tracing_link *tr_link = > + container_of(l, struct bpf_tracing_link, link); > + > + cookie = tr_link->cookie; > + } > + > + /* mov rdi, cookie */ > + emit_mov_imm64(&prog, BPF_REG_1, (long) cookie >> 32, (u32) (long) cookie); > > /* Prepare struct bpf_tramp_run_ctx. > * [...]
On Mon, 2022-05-09 at 11:58 -0700, Andrii Nakryiko wrote: > On Sat, May 7, 2022 at 8:21 PM Kui-Feng Lee <kuifeng@fb.com> wrote: > > > > Pass a cookie along with BPF_LINK_CREATE requests. > > > > Add a bpf_cookie field to struct bpf_tracing_link to attach a > > cookie. > > The cookie of a bpf_tracing_link is available by calling > > bpf_get_attach_cookie when running the BPF program of the attached > > link. > > > > The value of a cookie will be set at bpf_tramp_run_ctx by the > > trampoline of the link. > > > > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com> > > --- > > arch/x86/net/bpf_jit_comp.c | 12 ++++++++++-- > > include/linux/bpf.h | 1 + > > include/uapi/linux/bpf.h | 9 +++++++++ > > kernel/bpf/bpf_lsm.c | 17 +++++++++++++++++ > > kernel/bpf/syscall.c | 12 ++++++++---- > > kernel/bpf/trampoline.c | 7 +++++-- > > kernel/trace/bpf_trace.c | 17 +++++++++++++++++ > > tools/include/uapi/linux/bpf.h | 9 +++++++++ > > 8 files changed, 76 insertions(+), 8 deletions(-) > > > > LGTM with a suggestion for some follow up clean up. > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > > diff --git a/arch/x86/net/bpf_jit_comp.c > > b/arch/x86/net/bpf_jit_comp.c > > index bf4576a6938c..52a5eba2d5e8 100644 > > --- a/arch/x86/net/bpf_jit_comp.c > > +++ b/arch/x86/net/bpf_jit_comp.c > > @@ -1764,13 +1764,21 @@ static int invoke_bpf_prog(const struct > > btf_func_model *m, u8 **pprog, > > struct bpf_tramp_link *l, int > > stack_size, > > bool save_ret) > > { > > + u64 cookie = 0; > > u8 *prog = *pprog; > > u8 *jmp_insn; > > int ctx_cookie_off = offsetof(struct bpf_tramp_run_ctx, > > bpf_cookie); > > struct bpf_prog *p = l->link.prog; > > > > - /* mov rdi, 0 */ > > - emit_mov_imm64(&prog, BPF_REG_1, 0, 0); > > + if (l->link.type == BPF_LINK_TYPE_TRACING) { > > It would probably be nicer to put cookie field into struct > bpf_tramp_link instead so that the JIT compiler doesn't have to do > this special handling. It also makes sense that struct bpf_trampoline > *trampoline is moved into struct bpf_tramp_link itself (given > trampoline is always there for bpf_tramp_link). It will increase the size of bpf_tramp_link a little bit, but they are not used by bpf_struct_ops. > > > + struct bpf_tracing_link *tr_link = > > + container_of(l, struct bpf_tracing_link, > > link); > > + > > + cookie = tr_link->cookie; > > + } > > + > > + /* mov rdi, cookie */ > > + emit_mov_imm64(&prog, BPF_REG_1, (long) cookie >> 32, (u32) > > (long) cookie); > > > > /* Prepare struct bpf_tramp_run_ctx. > > * > > [...]
On Tue, May 10, 2022 at 9:44 AM Kui-Feng Lee <kuifeng@fb.com> wrote: > > On Mon, 2022-05-09 at 11:58 -0700, Andrii Nakryiko wrote: > > On Sat, May 7, 2022 at 8:21 PM Kui-Feng Lee <kuifeng@fb.com> wrote: > > > > > > Pass a cookie along with BPF_LINK_CREATE requests. > > > > > > Add a bpf_cookie field to struct bpf_tracing_link to attach a > > > cookie. > > > The cookie of a bpf_tracing_link is available by calling > > > bpf_get_attach_cookie when running the BPF program of the attached > > > link. > > > > > > The value of a cookie will be set at bpf_tramp_run_ctx by the > > > trampoline of the link. > > > > > > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com> > > > --- > > > arch/x86/net/bpf_jit_comp.c | 12 ++++++++++-- > > > include/linux/bpf.h | 1 + > > > include/uapi/linux/bpf.h | 9 +++++++++ > > > kernel/bpf/bpf_lsm.c | 17 +++++++++++++++++ > > > kernel/bpf/syscall.c | 12 ++++++++---- > > > kernel/bpf/trampoline.c | 7 +++++-- > > > kernel/trace/bpf_trace.c | 17 +++++++++++++++++ > > > tools/include/uapi/linux/bpf.h | 9 +++++++++ > > > 8 files changed, 76 insertions(+), 8 deletions(-) > > > > > > > LGTM with a suggestion for some follow up clean up. > > > > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > > > > diff --git a/arch/x86/net/bpf_jit_comp.c > > > b/arch/x86/net/bpf_jit_comp.c > > > index bf4576a6938c..52a5eba2d5e8 100644 > > > --- a/arch/x86/net/bpf_jit_comp.c > > > +++ b/arch/x86/net/bpf_jit_comp.c > > > @@ -1764,13 +1764,21 @@ static int invoke_bpf_prog(const struct > > > btf_func_model *m, u8 **pprog, > > > struct bpf_tramp_link *l, int > > > stack_size, > > > bool save_ret) > > > { > > > + u64 cookie = 0; > > > u8 *prog = *pprog; > > > u8 *jmp_insn; > > > int ctx_cookie_off = offsetof(struct bpf_tramp_run_ctx, > > > bpf_cookie); > > > struct bpf_prog *p = l->link.prog; > > > > > > - /* mov rdi, 0 */ > > > - emit_mov_imm64(&prog, BPF_REG_1, 0, 0); > > > + if (l->link.type == BPF_LINK_TYPE_TRACING) { > > > > It would probably be nicer to put cookie field into struct > > bpf_tramp_link instead so that the JIT compiler doesn't have to do > > this special handling. It also makes sense that struct bpf_trampoline > > *trampoline is moved into struct bpf_tramp_link itself (given > > trampoline is always there for bpf_tramp_link). > > It will increase the size of bpf_tramp_link a little bit, but they are > not used by bpf_struct_ops. > It feels like the right tradeoff to keep architecture-specific trampoline code oblivious to these details. Some day structs_ops might support cookies as well. And either way 8 bytes for struct_ops link isn't a big deal. > > > > > + struct bpf_tracing_link *tr_link = > > > + container_of(l, struct bpf_tracing_link, > > > link); > > > + > > > + cookie = tr_link->cookie; > > > + } > > > + > > > + /* mov rdi, cookie */ > > > + emit_mov_imm64(&prog, BPF_REG_1, (long) cookie >> 32, (u32) > > > (long) cookie); > > > > > > /* Prepare struct bpf_tramp_run_ctx. > > > * > > > > [...] >
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c index bf4576a6938c..52a5eba2d5e8 100644 --- a/arch/x86/net/bpf_jit_comp.c +++ b/arch/x86/net/bpf_jit_comp.c @@ -1764,13 +1764,21 @@ static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog, struct bpf_tramp_link *l, int stack_size, bool save_ret) { + u64 cookie = 0; u8 *prog = *pprog; u8 *jmp_insn; int ctx_cookie_off = offsetof(struct bpf_tramp_run_ctx, bpf_cookie); struct bpf_prog *p = l->link.prog; - /* mov rdi, 0 */ - emit_mov_imm64(&prog, BPF_REG_1, 0, 0); + if (l->link.type == BPF_LINK_TYPE_TRACING) { + struct bpf_tracing_link *tr_link = + container_of(l, struct bpf_tracing_link, link); + + cookie = tr_link->cookie; + } + + /* mov rdi, cookie */ + emit_mov_imm64(&prog, BPF_REG_1, (long) cookie >> 32, (u32) (long) cookie); /* Prepare struct bpf_tramp_run_ctx. * diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 29c3188195a6..13d80a4aa45b 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1109,6 +1109,7 @@ struct bpf_tracing_link { enum bpf_attach_type attach_type; struct bpf_trampoline *trampoline; struct bpf_prog *tgt_prog; + u64 cookie; }; struct bpf_link_primer { diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index ff9af73859ca..a70e1fd3b3a1 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -1490,6 +1490,15 @@ union bpf_attr { __aligned_u64 addrs; __aligned_u64 cookies; } kprobe_multi; + struct { + /* this is overliad with the target_btf_id above. */ + __u32 target_btf_id; + /* black box user-provided value passed through + * to BPF program at the execution time and + * accessible through bpf_get_attach_cookie() BPF helper + */ + __u64 cookie; + } tracing; }; } link_create; diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c index 064eccba641d..c1351df9f7ee 100644 --- a/kernel/bpf/bpf_lsm.c +++ b/kernel/bpf/bpf_lsm.c @@ -117,6 +117,21 @@ static const struct bpf_func_proto bpf_ima_file_hash_proto = { .allowed = bpf_ima_inode_hash_allowed, }; +BPF_CALL_1(bpf_get_attach_cookie, void *, ctx) +{ + struct bpf_trace_run_ctx *run_ctx; + + run_ctx = container_of(current->bpf_ctx, struct bpf_trace_run_ctx, run_ctx); + return run_ctx->bpf_cookie; +} + +static const struct bpf_func_proto bpf_get_attach_cookie_proto = { + .func = bpf_get_attach_cookie, + .gpl_only = false, + .ret_type = RET_INTEGER, + .arg1_type = ARG_PTR_TO_CTX, +}; + static const struct bpf_func_proto * bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) { @@ -141,6 +156,8 @@ bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) return prog->aux->sleepable ? &bpf_ima_inode_hash_proto : NULL; case BPF_FUNC_ima_file_hash: return prog->aux->sleepable ? &bpf_ima_file_hash_proto : NULL; + case BPF_FUNC_get_attach_cookie: + return bpf_prog_has_trampoline(prog) ? &bpf_get_attach_cookie_proto : NULL; default: return tracing_prog_func_proto(func_id, prog); } diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 5ed9a15daaee..dd0f4d51bcf6 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -2921,7 +2921,8 @@ static const struct bpf_link_ops bpf_tracing_link_lops = { static int bpf_tracing_prog_attach(struct bpf_prog *prog, int tgt_prog_fd, - u32 btf_id) + u32 btf_id, + u64 bpf_cookie) { struct bpf_link_primer link_primer; struct bpf_prog *tgt_prog = NULL; @@ -2986,6 +2987,7 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, bpf_link_init(&link->link.link, BPF_LINK_TYPE_TRACING, &bpf_tracing_link_lops, prog); link->attach_type = prog->expected_attach_type; + link->cookie = bpf_cookie; mutex_lock(&prog->aux->dst_mutex); @@ -3271,7 +3273,7 @@ static int bpf_raw_tp_link_attach(struct bpf_prog *prog, tp_name = prog->aux->attach_func_name; break; } - return bpf_tracing_prog_attach(prog, 0, 0); + return bpf_tracing_prog_attach(prog, 0, 0, 0); case BPF_PROG_TYPE_RAW_TRACEPOINT: case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE: if (strncpy_from_user(buf, user_tp_name, sizeof(buf) - 1) < 0) @@ -4524,7 +4526,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr) case BPF_PROG_TYPE_EXT: ret = bpf_tracing_prog_attach(prog, attr->link_create.target_fd, - attr->link_create.target_btf_id); + attr->link_create.target_btf_id, + attr->link_create.tracing.cookie); break; case BPF_PROG_TYPE_LSM: case BPF_PROG_TYPE_TRACING: @@ -4539,7 +4542,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr) else ret = bpf_tracing_prog_attach(prog, attr->link_create.target_fd, - attr->link_create.target_btf_id); + attr->link_create.target_btf_id, + attr->link_create.tracing.cookie); break; case BPF_PROG_TYPE_FLOW_DISSECTOR: case BPF_PROG_TYPE_SK_LOOKUP: diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c index baf1b65d523e..0e9b3aefc34a 100644 --- a/kernel/bpf/trampoline.c +++ b/kernel/bpf/trampoline.c @@ -30,9 +30,12 @@ static DEFINE_MUTEX(trampoline_mutex); bool bpf_prog_has_trampoline(const struct bpf_prog *prog) { enum bpf_attach_type eatype = prog->expected_attach_type; + enum bpf_prog_type ptype = prog->type; - return eatype == BPF_TRACE_FENTRY || eatype == BPF_TRACE_FEXIT || - eatype == BPF_MODIFY_RETURN; + return (ptype == BPF_PROG_TYPE_TRACING && + (eatype == BPF_TRACE_FENTRY || eatype == BPF_TRACE_FEXIT || + eatype == BPF_MODIFY_RETURN)) || + (ptype == BPF_PROG_TYPE_LSM && eatype == BPF_LSM_MAC); } void *bpf_jit_alloc_exec_page(void) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index f15b826f9899..6377ed23e17f 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1091,6 +1091,21 @@ static const struct bpf_func_proto bpf_get_attach_cookie_proto_pe = { .arg1_type = ARG_PTR_TO_CTX, }; +BPF_CALL_1(bpf_get_attach_cookie_tracing, void *, ctx) +{ + struct bpf_trace_run_ctx *run_ctx; + + run_ctx = container_of(current->bpf_ctx, struct bpf_trace_run_ctx, run_ctx); + return run_ctx->bpf_cookie; +} + +static const struct bpf_func_proto bpf_get_attach_cookie_proto_tracing = { + .func = bpf_get_attach_cookie_tracing, + .gpl_only = false, + .ret_type = RET_INTEGER, + .arg1_type = ARG_PTR_TO_CTX, +}; + BPF_CALL_3(bpf_get_branch_snapshot, void *, buf, u32, size, u64, flags) { #ifndef CONFIG_X86 @@ -1719,6 +1734,8 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) return bpf_prog_has_trampoline(prog) ? &bpf_get_func_ret_proto : NULL; case BPF_FUNC_get_func_arg_cnt: return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_cnt_proto : NULL; + case BPF_FUNC_get_attach_cookie: + return bpf_prog_has_trampoline(prog) ? &bpf_get_attach_cookie_proto_tracing : NULL; default: fn = raw_tp_prog_func_proto(func_id, prog); if (!fn && prog->expected_attach_type == BPF_TRACE_ITER) diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index ff9af73859ca..a70e1fd3b3a1 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -1490,6 +1490,15 @@ union bpf_attr { __aligned_u64 addrs; __aligned_u64 cookies; } kprobe_multi; + struct { + /* this is overliad with the target_btf_id above. */ + __u32 target_btf_id; + /* black box user-provided value passed through + * to BPF program at the execution time and + * accessible through bpf_get_attach_cookie() BPF helper + */ + __u64 cookie; + } tracing; }; } link_create;
Pass a cookie along with BPF_LINK_CREATE requests. Add a bpf_cookie field to struct bpf_tracing_link to attach a cookie. The cookie of a bpf_tracing_link is available by calling bpf_get_attach_cookie when running the BPF program of the attached link. The value of a cookie will be set at bpf_tramp_run_ctx by the trampoline of the link. Signed-off-by: Kui-Feng Lee <kuifeng@fb.com> --- arch/x86/net/bpf_jit_comp.c | 12 ++++++++++-- include/linux/bpf.h | 1 + include/uapi/linux/bpf.h | 9 +++++++++ kernel/bpf/bpf_lsm.c | 17 +++++++++++++++++ kernel/bpf/syscall.c | 12 ++++++++---- kernel/bpf/trampoline.c | 7 +++++-- kernel/trace/bpf_trace.c | 17 +++++++++++++++++ tools/include/uapi/linux/bpf.h | 9 +++++++++ 8 files changed, 76 insertions(+), 8 deletions(-)