| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87v9ljv4vs.fsf@toke.dk>
Date: Tue, 28 Apr 2020 20:31:35 +0200
From: Toke Høiland-Jørgensen <toke@...hat.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
Networking <netdev@...r.kernel.org>,
Alexei Starovoitov <ast@...com>,
Daniel Borkmann <daniel@...earbox.net>,
Kernel Team <kernel-team@...com>
Subject: Re: [PATCH v2 bpf-next 04/10] bpf: add support for BPF_OBJ_GET_INFO_BY_FD for bpf_link
Andrii Nakryiko <andrii.nakryiko@...il.com> writes:
> On Tue, Apr 28, 2020 at 2:46 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>>
>> Andrii Nakryiko <andriin@...com> writes:
>>
>> > Add ability to fetch bpf_link details through BPF_OBJ_GET_INFO_BY_FD command.
>> > Also enhance show_fdinfo to potentially include bpf_link type-specific
>> > information (similarly to obj_info).
>> >
>> > Also introduce enum bpf_link_type stored in bpf_link itself and expose it in
>> > UAPI. bpf_link_tracing also now will store and return bpf_attach_type.
>> >
>> > Signed-off-by: Andrii Nakryiko <andriin@...com>
>> > ---
>> > include/linux/bpf-cgroup.h | 2 -
>> > include/linux/bpf.h | 10 +-
>> > include/linux/bpf_types.h | 6 ++
>> > include/uapi/linux/bpf.h | 28 ++++++
>> > kernel/bpf/btf.c | 2 +
>> > kernel/bpf/cgroup.c | 45 ++++++++-
>> > kernel/bpf/syscall.c | 164 +++++++++++++++++++++++++++++----
>> > kernel/bpf/verifier.c | 2 +
>> > tools/include/uapi/linux/bpf.h | 31 +++++++
>> > 9 files changed, 266 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
>> > index d2d969669564..ab95824a1d99 100644
>> > --- a/include/linux/bpf-cgroup.h
>> > +++ b/include/linux/bpf-cgroup.h
>> > @@ -57,8 +57,6 @@ struct bpf_cgroup_link {
>> > enum bpf_attach_type type;
>> > };
>> >
>> > -extern const struct bpf_link_ops bpf_cgroup_link_lops;
>> > -
>> > struct bpf_prog_list {
>> > struct list_head node;
>> > struct bpf_prog *prog;
>> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> > index 875d1f0af803..701c4387c711 100644
>> > --- a/include/linux/bpf.h
>> > +++ b/include/linux/bpf.h
>> > @@ -1026,9 +1026,11 @@ extern const struct file_operations bpf_prog_fops;
>> > extern const struct bpf_verifier_ops _name ## _verifier_ops;
>> > #define BPF_MAP_TYPE(_id, _ops) \
>> > extern const struct bpf_map_ops _ops;
>> > +#define BPF_LINK_TYPE(_id, _name)
>> > #include <linux/bpf_types.h>
>> > #undef BPF_PROG_TYPE
>> > #undef BPF_MAP_TYPE
>> > +#undef BPF_LINK_TYPE
>> >
>> > extern const struct bpf_prog_ops bpf_offload_prog_ops;
>> > extern const struct bpf_verifier_ops tc_cls_act_analyzer_ops;
>> > @@ -1086,6 +1088,7 @@ int bpf_prog_new_fd(struct bpf_prog *prog);
>> > struct bpf_link {
>> > atomic64_t refcnt;
>> > u32 id;
>> > + enum bpf_link_type type;
>> > const struct bpf_link_ops *ops;
>> > struct bpf_prog *prog;
>> > struct work_struct work;
>> > @@ -1103,9 +1106,14 @@ struct bpf_link_ops {
>> > void (*dealloc)(struct bpf_link *link);
>> > int (*update_prog)(struct bpf_link *link, struct bpf_prog *new_prog,
>> > struct bpf_prog *old_prog);
>> > + void (*show_fdinfo)(const struct bpf_link *link, struct seq_file *seq);
>> > + int (*fill_link_info)(const struct bpf_link *link,
>> > + struct bpf_link_info *info,
>> > + const struct bpf_link_info *uinfo,
>> > + u32 info_len);
>> > };
>> >
>> > -void bpf_link_init(struct bpf_link *link,
>> > +void bpf_link_init(struct bpf_link *link, enum bpf_link_type type,
>> > const struct bpf_link_ops *ops, struct bpf_prog *prog);
>> > int bpf_link_prime(struct bpf_link *link, struct bpf_link_primer *primer);
>> > int bpf_link_settle(struct bpf_link_primer *primer);
>> > diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
>> > index ba0c2d56f8a3..8345cdf553b8 100644
>> > --- a/include/linux/bpf_types.h
>> > +++ b/include/linux/bpf_types.h
>> > @@ -118,3 +118,9 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_STACK, stack_map_ops)
>> > #if defined(CONFIG_BPF_JIT)
>> > BPF_MAP_TYPE(BPF_MAP_TYPE_STRUCT_OPS, bpf_struct_ops_map_ops)
>> > #endif
>> > +
>> > +BPF_LINK_TYPE(BPF_LINK_TYPE_RAW_TRACEPOINT, raw_tracepoint)
>> > +BPF_LINK_TYPE(BPF_LINK_TYPE_TRACING, tracing)
>> > +#ifdef CONFIG_CGROUP_BPF
>> > +BPF_LINK_TYPE(BPF_LINK_TYPE_CGROUP, cgroup)
>> > +#endif
>> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> > index 7e6541fceade..0eccafae55bb 100644
>> > --- a/include/uapi/linux/bpf.h
>> > +++ b/include/uapi/linux/bpf.h
>> > @@ -222,6 +222,15 @@ enum bpf_attach_type {
>> >
>> > #define MAX_BPF_ATTACH_TYPE __MAX_BPF_ATTACH_TYPE
>> >
>> > +enum bpf_link_type {
>> > + BPF_LINK_TYPE_UNSPEC = 0,
>> > + BPF_LINK_TYPE_RAW_TRACEPOINT = 1,
>> > + BPF_LINK_TYPE_TRACING = 2,
>> > + BPF_LINK_TYPE_CGROUP = 3,
>> > +
>> > + MAX_BPF_LINK_TYPE,
>> > +};
>> > +
>> > /* cgroup-bpf attach flags used in BPF_PROG_ATTACH command
>> > *
>> > * NONE(default): No further bpf programs allowed in the subtree.
>> > @@ -3612,6 +3621,25 @@ struct bpf_btf_info {
>> > __u32 id;
>> > } __attribute__((aligned(8)));
>> >
>> > +struct bpf_link_info {
>> > + __u32 type;
>> > + __u32 id;
>> > + __u32 prog_id;
>> > + union {
>> > + struct {
>> > + __aligned_u64 tp_name; /* in/out: tp_name buffer ptr */
>> > + __u32 tp_name_len; /* in/out: tp_name buffer len */
>> > + } raw_tracepoint;
>> > + struct {
>> > + __u32 attach_type;
>> > + } tracing;
>>
>> On the RFC I asked whether we could get the attach target here as well.
>> You said you'd look into it; what happened to that? :)
>>
>
> Right, sorry, forgot to mention this. I did take a look, but tracing
> links are quite diverse, so I didn't see one clear way to structure
> such "target" information, so I'd say we should push it into a
> separate patch/discussion. E.g., fentry/fexit/fmod_exit are attached
> to kernel functions (how do we represent that), freplace are attached
> to another BPF program (this is a bit clearer how to represent, but
> how do we combine that with fentry/fexit info?). LSM is also attached
> to kernel function, but who knows, maybe we want slightly more
> extended semantics for it. Either way, I don't see one best way to
> structure this information and would want to avoid blocking on this
> for this series. Also bpf_link_info is extensible, so it's not a
> problem to extend it in follow up patches.
>
> Does it make sense?
Yup, fair enough, I can live with deferring this to a later series :)
-Toke
Powered by blists - more mailing lists