| Message ID | 20230608103523.102267-9-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: Support ->fill_link_info for kprobe_multi and perf_event links | expand |
On Thu, Jun 8, 2023 at 3:35 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > By introducing support for ->fill_link_info to the perf_event link, users > gain the ability to inspect it using `bpftool link show`. While the current > approach involves accessing this information via `bpftool perf show`, > consolidating link information for all link types in one place offers > greater convenience. Additionally, this patch extends support to the > generic perf event, which is not currently accommodated by > `bpftool perf show`. While only the perf type and config are exposed to > userspace, other attributes such as sample_period and sample_freq are > ignored. It's important to note that if kptr_restrict is set to 2, the > probed address will not be exposed, maintaining security measures. > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > --- > include/uapi/linux/bpf.h | 22 ++++++++++ > kernel/bpf/syscall.c | 98 ++++++++++++++++++++++++++++++++++++++++++ > tools/include/uapi/linux/bpf.h | 22 ++++++++++ > 3 files changed, 142 insertions(+) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index d99cc16..c3b821d 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -6443,6 +6443,28 @@ struct bpf_link_info { > __u32 count; > __u8 retprobe; > } kprobe_multi; > + union { > + struct { > + /* The name is: > + * a) uprobe: file name > + * b) kprobe: kernel function > + */ > + __aligned_u64 name; /* in/out: name buffer ptr */ > + __u32 name_len; > + __u32 offset; /* offset from the name */ > + __u64 addr; > + __u8 retprobe; > + } probe; /* uprobe, kprobe */ > + struct { > + /* in/out: tracepoint name buffer ptr */ > + __aligned_u64 tp_name; > + __u32 name_len; > + } tp; /* tracepoint */ > + struct { > + __u64 config; > + __u32 type; > + } event; /* generic perf event */ how should the user know which of those structs are relevant? we need some enum to specify what kind of perf_event link it is? > + } perf_event; > }; > } __attribute__((aligned(8))); > [...]
On Fri, Jun 9, 2023 at 7:12 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, Jun 8, 2023 at 3:35 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > By introducing support for ->fill_link_info to the perf_event link, users > > gain the ability to inspect it using `bpftool link show`. While the current > > approach involves accessing this information via `bpftool perf show`, > > consolidating link information for all link types in one place offers > > greater convenience. Additionally, this patch extends support to the > > generic perf event, which is not currently accommodated by > > `bpftool perf show`. While only the perf type and config are exposed to > > userspace, other attributes such as sample_period and sample_freq are > > ignored. It's important to note that if kptr_restrict is set to 2, the > > probed address will not be exposed, maintaining security measures. > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > --- > > include/uapi/linux/bpf.h | 22 ++++++++++ > > kernel/bpf/syscall.c | 98 ++++++++++++++++++++++++++++++++++++++++++ > > tools/include/uapi/linux/bpf.h | 22 ++++++++++ > > 3 files changed, 142 insertions(+) > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index d99cc16..c3b821d 100644 > > --- a/include/uapi/linux/bpf.h > > +++ b/include/uapi/linux/bpf.h > > @@ -6443,6 +6443,28 @@ struct bpf_link_info { > > __u32 count; > > __u8 retprobe; > > } kprobe_multi; > > + union { > > + struct { > > + /* The name is: > > + * a) uprobe: file name > > + * b) kprobe: kernel function > > + */ > > + __aligned_u64 name; /* in/out: name buffer ptr */ > > + __u32 name_len; > > + __u32 offset; /* offset from the name */ > > + __u64 addr; > > + __u8 retprobe; > > + } probe; /* uprobe, kprobe */ > > + struct { > > + /* in/out: tracepoint name buffer ptr */ > > + __aligned_u64 tp_name; > > + __u32 name_len; > > + } tp; /* tracepoint */ > > + struct { > > + __u64 config; > > + __u32 type; > > + } event; /* generic perf event */ > > how should the user know which of those structs are relevant? we need > some enum to specify what kind of perf_event link it is? > Do you mean that we add a new field 'type' into the union perf_event, as follows ? union { __u32 type; struct {} probe; /* BPF_LINK_TYPE_PERF_EVENT_PROBE */ struct {} tp; /* BPF_LINK_TYPE_PERF_EVENT_TP */ struct {} event; /* BPF_LINK_TYPE_PERF_EVENT_EVENT */ };
On Fri, Jun 9, 2023 at 5:53 PM Yafang Shao <laoar.shao@gmail.com> wrote: > > On Fri, Jun 9, 2023 at 7:12 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Thu, Jun 8, 2023 at 3:35 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > > > By introducing support for ->fill_link_info to the perf_event link, users > > > gain the ability to inspect it using `bpftool link show`. While the current > > > approach involves accessing this information via `bpftool perf show`, > > > consolidating link information for all link types in one place offers > > > greater convenience. Additionally, this patch extends support to the > > > generic perf event, which is not currently accommodated by > > > `bpftool perf show`. While only the perf type and config are exposed to > > > userspace, other attributes such as sample_period and sample_freq are > > > ignored. It's important to note that if kptr_restrict is set to 2, the > > > probed address will not be exposed, maintaining security measures. > > > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > > --- > > > include/uapi/linux/bpf.h | 22 ++++++++++ > > > kernel/bpf/syscall.c | 98 ++++++++++++++++++++++++++++++++++++++++++ > > > tools/include/uapi/linux/bpf.h | 22 ++++++++++ > > > 3 files changed, 142 insertions(+) > > > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > > index d99cc16..c3b821d 100644 > > > --- a/include/uapi/linux/bpf.h > > > +++ b/include/uapi/linux/bpf.h > > > @@ -6443,6 +6443,28 @@ struct bpf_link_info { > > > __u32 count; > > > __u8 retprobe; > > > } kprobe_multi; > > > + union { > > > + struct { > > > + /* The name is: > > > + * a) uprobe: file name > > > + * b) kprobe: kernel function > > > + */ > > > + __aligned_u64 name; /* in/out: name buffer ptr */ > > > + __u32 name_len; > > > + __u32 offset; /* offset from the name */ > > > + __u64 addr; > > > + __u8 retprobe; > > > + } probe; /* uprobe, kprobe */ > > > + struct { > > > + /* in/out: tracepoint name buffer ptr */ > > > + __aligned_u64 tp_name; > > > + __u32 name_len; > > > + } tp; /* tracepoint */ > > > + struct { > > > + __u64 config; > > > + __u32 type; > > > + } event; /* generic perf event */ > > > > how should the user know which of those structs are relevant? we need > > some enum to specify what kind of perf_event link it is? > > > > Do you mean that we add a new field 'type' into the union perf_event, > as follows ? > union { > __u32 type; > struct {} probe; /* BPF_LINK_TYPE_PERF_EVENT_PROBE */ > struct {} tp; /* BPF_LINK_TYPE_PERF_EVENT_TP */ > struct {} event; /* BPF_LINK_TYPE_PERF_EVENT_EVENT */ > }; > Correct it: struct { __u32 type; union { struct {} probe; /* BPF_LINK_TYPE_PERF_EVENT_PROBE */ struct {} tp; /* BPF_LINK_TYPE_PERF_EVENT_TP */ struct {} event; /* BPF_LINK_TYPE_PERF_EVENT_EVENT */ }; } perf_event;
On Fri, Jun 9, 2023 at 2:57 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > On Fri, Jun 9, 2023 at 5:53 PM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > On Fri, Jun 9, 2023 at 7:12 AM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Thu, Jun 8, 2023 at 3:35 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > > > > > By introducing support for ->fill_link_info to the perf_event link, users > > > > gain the ability to inspect it using `bpftool link show`. While the current > > > > approach involves accessing this information via `bpftool perf show`, > > > > consolidating link information for all link types in one place offers > > > > greater convenience. Additionally, this patch extends support to the > > > > generic perf event, which is not currently accommodated by > > > > `bpftool perf show`. While only the perf type and config are exposed to > > > > userspace, other attributes such as sample_period and sample_freq are > > > > ignored. It's important to note that if kptr_restrict is set to 2, the > > > > probed address will not be exposed, maintaining security measures. > > > > > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > > > --- > > > > include/uapi/linux/bpf.h | 22 ++++++++++ > > > > kernel/bpf/syscall.c | 98 ++++++++++++++++++++++++++++++++++++++++++ > > > > tools/include/uapi/linux/bpf.h | 22 ++++++++++ > > > > 3 files changed, 142 insertions(+) > > > > > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > > > index d99cc16..c3b821d 100644 > > > > --- a/include/uapi/linux/bpf.h > > > > +++ b/include/uapi/linux/bpf.h > > > > @@ -6443,6 +6443,28 @@ struct bpf_link_info { > > > > __u32 count; > > > > __u8 retprobe; > > > > } kprobe_multi; > > > > + union { > > > > + struct { > > > > + /* The name is: > > > > + * a) uprobe: file name > > > > + * b) kprobe: kernel function > > > > + */ > > > > + __aligned_u64 name; /* in/out: name buffer ptr */ > > > > + __u32 name_len; > > > > + __u32 offset; /* offset from the name */ > > > > + __u64 addr; > > > > + __u8 retprobe; > > > > + } probe; /* uprobe, kprobe */ > > > > + struct { > > > > + /* in/out: tracepoint name buffer ptr */ > > > > + __aligned_u64 tp_name; > > > > + __u32 name_len; > > > > + } tp; /* tracepoint */ > > > > + struct { > > > > + __u64 config; > > > > + __u32 type; > > > > + } event; /* generic perf event */ > > > > > > how should the user know which of those structs are relevant? we need > > > some enum to specify what kind of perf_event link it is? > > > > > > > Do you mean that we add a new field 'type' into the union perf_event, > > as follows ? > > union { > > __u32 type; > > struct {} probe; /* BPF_LINK_TYPE_PERF_EVENT_PROBE */ > > struct {} tp; /* BPF_LINK_TYPE_PERF_EVENT_TP */ > > struct {} event; /* BPF_LINK_TYPE_PERF_EVENT_EVENT */ > > }; > > > > Correct it: > > struct { > __u32 type; > union { > struct {} probe; /* BPF_LINK_TYPE_PERF_EVENT_PROBE */ > struct {} tp; /* BPF_LINK_TYPE_PERF_EVENT_TP */ > struct {} event; /* BPF_LINK_TYPE_PERF_EVENT_EVENT */ > }; > } perf_event; yes, something like this. Unless we want to leave perf_event {} to mean really perf event only, while kprobe/uprobe/tracepoint should be their own separate sections at the same level of nestedness as perf_Event and other cases. Not sure. > > -- > Regards > Yafang
On Sat, Jun 10, 2023 at 2:26 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Fri, Jun 9, 2023 at 2:57 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > On Fri, Jun 9, 2023 at 5:53 PM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > > > On Fri, Jun 9, 2023 at 7:12 AM Andrii Nakryiko > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > On Thu, Jun 8, 2023 at 3:35 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > > > > > > > By introducing support for ->fill_link_info to the perf_event link, users > > > > > gain the ability to inspect it using `bpftool link show`. While the current > > > > > approach involves accessing this information via `bpftool perf show`, > > > > > consolidating link information for all link types in one place offers > > > > > greater convenience. Additionally, this patch extends support to the > > > > > generic perf event, which is not currently accommodated by > > > > > `bpftool perf show`. While only the perf type and config are exposed to > > > > > userspace, other attributes such as sample_period and sample_freq are > > > > > ignored. It's important to note that if kptr_restrict is set to 2, the > > > > > probed address will not be exposed, maintaining security measures. > > > > > > > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > > > > --- > > > > > include/uapi/linux/bpf.h | 22 ++++++++++ > > > > > kernel/bpf/syscall.c | 98 ++++++++++++++++++++++++++++++++++++++++++ > > > > > tools/include/uapi/linux/bpf.h | 22 ++++++++++ > > > > > 3 files changed, 142 insertions(+) > > > > > > > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > > > > index d99cc16..c3b821d 100644 > > > > > --- a/include/uapi/linux/bpf.h > > > > > +++ b/include/uapi/linux/bpf.h > > > > > @@ -6443,6 +6443,28 @@ struct bpf_link_info { > > > > > __u32 count; > > > > > __u8 retprobe; > > > > > } kprobe_multi; > > > > > + union { > > > > > + struct { > > > > > + /* The name is: > > > > > + * a) uprobe: file name > > > > > + * b) kprobe: kernel function > > > > > + */ > > > > > + __aligned_u64 name; /* in/out: name buffer ptr */ > > > > > + __u32 name_len; > > > > > + __u32 offset; /* offset from the name */ > > > > > + __u64 addr; > > > > > + __u8 retprobe; > > > > > + } probe; /* uprobe, kprobe */ > > > > > + struct { > > > > > + /* in/out: tracepoint name buffer ptr */ > > > > > + __aligned_u64 tp_name; > > > > > + __u32 name_len; > > > > > + } tp; /* tracepoint */ > > > > > + struct { > > > > > + __u64 config; > > > > > + __u32 type; > > > > > + } event; /* generic perf event */ > > > > > > > > how should the user know which of those structs are relevant? we need > > > > some enum to specify what kind of perf_event link it is? > > > > > > > > > > Do you mean that we add a new field 'type' into the union perf_event, > > > as follows ? > > > union { > > > __u32 type; > > > struct {} probe; /* BPF_LINK_TYPE_PERF_EVENT_PROBE */ > > > struct {} tp; /* BPF_LINK_TYPE_PERF_EVENT_TP */ > > > struct {} event; /* BPF_LINK_TYPE_PERF_EVENT_EVENT */ > > > }; > > > > > > > Correct it: > > > > struct { > > __u32 type; > > union { > > struct {} probe; /* BPF_LINK_TYPE_PERF_EVENT_PROBE */ > > struct {} tp; /* BPF_LINK_TYPE_PERF_EVENT_TP */ > > struct {} event; /* BPF_LINK_TYPE_PERF_EVENT_EVENT */ > > }; > > } perf_event; > > yes, something like this. Unless we want to leave perf_event {} to > mean really perf event only, while kprobe/uprobe/tracepoint should be > their own separate sections at the same level of nestedness as > perf_Event and other cases. Not sure. > Thanks for your explanation. I will think about it.
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index d99cc16..c3b821d 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -6443,6 +6443,28 @@ struct bpf_link_info { __u32 count; __u8 retprobe; } kprobe_multi; + union { + struct { + /* The name is: + * a) uprobe: file name + * b) kprobe: kernel function + */ + __aligned_u64 name; /* in/out: name buffer ptr */ + __u32 name_len; + __u32 offset; /* offset from the name */ + __u64 addr; + __u8 retprobe; + } probe; /* uprobe, kprobe */ + struct { + /* in/out: tracepoint name buffer ptr */ + __aligned_u64 tp_name; + __u32 name_len; + } tp; /* tracepoint */ + struct { + __u64 config; + __u32 type; + } event; /* generic perf event */ + } perf_event; }; } __attribute__((aligned(8))); diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 80c9ec0..e349bdb 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -3303,9 +3303,107 @@ static void bpf_perf_link_dealloc(struct bpf_link *link) kfree(perf_link); } +static int bpf_perf_link_fill_name(const struct perf_event *event, + char __user *uname, u32 ulen, + u64 *probe_offset, u64 *probe_addr, + u32 *fd_type) +{ + const char *buf; + u32 prog_id; + size_t len; + int err; + + if (!ulen ^ !uname) + return -EINVAL; + if (!uname) + return 0; + + err = bpf_get_perf_event_info(event, &prog_id, fd_type, &buf, + probe_offset, probe_addr); + if (err) + return err; + + len = strlen(buf); + if (buf) { + err = bpf_copy_to_user(uname, buf, ulen, len); + if (err) + return err; + } else { + char zero = '\0'; + + if (put_user(zero, uname)) + return -EFAULT; + } + return 0; +} + +static int bpf_perf_link_fill_probe(const struct perf_event *event, + struct bpf_link_info *info) +{ + char __user *uname = u64_to_user_ptr(info->perf_event.probe.name); + u32 ulen = info->perf_event.probe.name_len; + u64 addr, off; + u32 type; + int err; + + err = bpf_perf_link_fill_name(event, uname, ulen, &off, &addr, &type); + if (err) + return err; + info->perf_event.probe.offset = off; + if (type == BPF_FD_TYPE_KRETPROBE || type == BPF_FD_TYPE_URETPROBE) + info->perf_event.probe.retprobe = 1; + else + info->perf_event.probe.retprobe = 0; + + if (kptr_restrict == 2) + return 0; + info->perf_event.probe.addr = addr; + return 0; +} + +static int bpf_perf_link_fill_tp(const struct perf_event *event, + struct bpf_link_info *info) +{ + char __user *uname = u64_to_user_ptr(info->perf_event.tp.tp_name); + u32 ulen = info->perf_event.tp.name_len; + u64 addr, off; + u32 type; + + return bpf_perf_link_fill_name(event, uname, ulen, &off, &addr, &type); +} + +static int bpf_perf_link_fill_event(const struct perf_event *event, + struct bpf_link_info *info) +{ + info->perf_event.event.type = event->attr.type; + info->perf_event.event.config = event->attr.config; + return 0; +} + +static int bpf_perf_link_fill_link_info(const struct bpf_link *link, + struct bpf_link_info *info) +{ + struct bpf_perf_link *perf_link; + const struct perf_event *event; + + perf_link = container_of(link, struct bpf_perf_link, link); + event = perf_get_event(perf_link->perf_file); + if (IS_ERR(event)) + return PTR_ERR(event); + + if (!event->prog) + return -EINVAL; + if (event->prog->type == BPF_PROG_TYPE_PERF_EVENT) + return bpf_perf_link_fill_event(event, info); + if (event->prog->type == BPF_PROG_TYPE_TRACEPOINT) + return bpf_perf_link_fill_tp(event, info); + return bpf_perf_link_fill_probe(event, info); +} + static const struct bpf_link_ops bpf_perf_link_lops = { .release = bpf_perf_link_release, .dealloc = bpf_perf_link_dealloc, + .fill_link_info = bpf_perf_link_fill_link_info, }; static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index d99cc16..c3b821d 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -6443,6 +6443,28 @@ struct bpf_link_info { __u32 count; __u8 retprobe; } kprobe_multi; + union { + struct { + /* The name is: + * a) uprobe: file name + * b) kprobe: kernel function + */ + __aligned_u64 name; /* in/out: name buffer ptr */ + __u32 name_len; + __u32 offset; /* offset from the name */ + __u64 addr; + __u8 retprobe; + } probe; /* uprobe, kprobe */ + struct { + /* in/out: tracepoint name buffer ptr */ + __aligned_u64 tp_name; + __u32 name_len; + } tp; /* tracepoint */ + struct { + __u64 config; + __u32 type; + } event; /* generic perf event */ + } perf_event; }; } __attribute__((aligned(8)));
By introducing support for ->fill_link_info to the perf_event link, users gain the ability to inspect it using `bpftool link show`. While the current approach involves accessing this information via `bpftool perf show`, consolidating link information for all link types in one place offers greater convenience. Additionally, this patch extends support to the generic perf event, which is not currently accommodated by `bpftool perf show`. While only the perf type and config are exposed to userspace, other attributes such as sample_period and sample_freq are ignored. It's important to note that if kptr_restrict is set to 2, the probed address will not be exposed, maintaining security measures. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- include/uapi/linux/bpf.h | 22 ++++++++++ kernel/bpf/syscall.c | 98 ++++++++++++++++++++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 22 ++++++++++ 3 files changed, 142 insertions(+)