| Message ID | 20230612151608.99661-2-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 Mon, Jun 12, 2023 at 03:15:59PM +0000, Yafang Shao wrote: SNIP > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 2bc41e6..742047c 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -2548,9 +2548,36 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link) > kfree(kmulti_link); > } > > +static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link, > + struct bpf_link_info *info) > +{ > + u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs); > + struct bpf_kprobe_multi_link *kmulti_link; > + u32 ucount = info->kprobe_multi.count; > + > + if (!uaddrs ^ !ucount) > + return -EINVAL; > + > + kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link); > + if (!uaddrs) { > + info->kprobe_multi.count = kmulti_link->cnt; > + return 0; > + } > + > + if (ucount < kmulti_link->cnt) > + return -EINVAL; > + info->kprobe_multi.flags = kmulti_link->fp.flags; > + if (!kallsyms_show_value(current_cred())) > + return 0; > + if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64))) > + return -EFAULT; > + return 0; > +} > + > static const struct bpf_link_ops bpf_kprobe_multi_link_lops = { > .release = bpf_kprobe_multi_link_release, > .dealloc = bpf_kprobe_multi_link_dealloc, > + .fill_link_info = bpf_kprobe_multi_link_fill_link_info, > }; > > static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv) > @@ -2890,6 +2917,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > return err; > } > > + link->fp.flags = flags; hum this looks wrong, we can't use fprobe flags to store our flags you should add flags to bpf_kprobe_multi_link jirka > return bpf_link_settle(&link_primer); > > error: > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h > index a7b5e91..23691ea 100644 > --- a/tools/include/uapi/linux/bpf.h > +++ b/tools/include/uapi/linux/bpf.h > @@ -6438,6 +6438,11 @@ struct bpf_link_info { > __s32 priority; > __u32 flags; > } netfilter; > + struct { > + __aligned_u64 addrs; /* in/out: addresses buffer ptr */ > + __u32 count; > + __u32 flags; > + } kprobe_multi; > }; > } __attribute__((aligned(8))); > > -- > 1.8.3.1 >
On Thu, Jun 15, 2023 at 4:29 PM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Mon, Jun 12, 2023 at 03:15:59PM +0000, Yafang Shao wrote: > > SNIP > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > index 2bc41e6..742047c 100644 > > --- a/kernel/trace/bpf_trace.c > > +++ b/kernel/trace/bpf_trace.c > > @@ -2548,9 +2548,36 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link) > > kfree(kmulti_link); > > } > > > > +static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link, > > + struct bpf_link_info *info) > > +{ > > + u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs); > > + struct bpf_kprobe_multi_link *kmulti_link; > > + u32 ucount = info->kprobe_multi.count; > > + > > + if (!uaddrs ^ !ucount) > > + return -EINVAL; > > + > > + kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link); > > + if (!uaddrs) { > > + info->kprobe_multi.count = kmulti_link->cnt; > > + return 0; > > + } > > + > > + if (ucount < kmulti_link->cnt) > > + return -EINVAL; > > + info->kprobe_multi.flags = kmulti_link->fp.flags; > > + if (!kallsyms_show_value(current_cred())) > > + return 0; > > + if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64))) > > + return -EFAULT; > > + return 0; > > +} > > + > > static const struct bpf_link_ops bpf_kprobe_multi_link_lops = { > > .release = bpf_kprobe_multi_link_release, > > .dealloc = bpf_kprobe_multi_link_dealloc, > > + .fill_link_info = bpf_kprobe_multi_link_fill_link_info, > > }; > > > > static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv) > > @@ -2890,6 +2917,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > > return err; > > } > > > > + link->fp.flags = flags; > > hum this looks wrong, we can't use fprobe flags to store our flags > you should add flags to bpf_kprobe_multi_link Will fix it. Thanks for pointing it out.
On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > With the addition of support for fill_link_info to the kprobe_multi link, > users will gain the ability to inspect it conveniently using the > `bpftool link show`. This enhancement provides valuable information to the > user, including the count of probed functions and their respective > addresses. It's important to note that if the kptr_restrict setting is not > permitted, the probed address will not be exposed, ensuring security. > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > --- > include/uapi/linux/bpf.h | 5 +++++ > kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++ > tools/include/uapi/linux/bpf.h | 5 +++++ > 3 files changed, 38 insertions(+) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index a7b5e91..23691ea 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -6438,6 +6438,11 @@ struct bpf_link_info { > __s32 priority; > __u32 flags; > } netfilter; > + struct { > + __aligned_u64 addrs; /* in/out: addresses buffer ptr */ > + __u32 count; > + __u32 flags; > + } kprobe_multi; > }; > } __attribute__((aligned(8))); > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 2bc41e6..742047c 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -2548,9 +2548,36 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link) > kfree(kmulti_link); > } > > +static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link, > + struct bpf_link_info *info) > +{ > + u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs); > + struct bpf_kprobe_multi_link *kmulti_link; > + u32 ucount = info->kprobe_multi.count; > + > + if (!uaddrs ^ !ucount) > + return -EINVAL; > + > + kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link); > + if (!uaddrs) { > + info->kprobe_multi.count = kmulti_link->cnt; > + return 0; > + } > + > + if (ucount < kmulti_link->cnt) > + return -EINVAL; > + info->kprobe_multi.flags = kmulti_link->fp.flags; besides what Jiri said, flags should always be returned, just like cnt. So structure code instead around uaddrs being optional, that will everything more straightforward (i.e., fill out everything but uaddrs and then at the end fill out addrs if uaddrs is not zero) > + if (!kallsyms_show_value(current_cred())) > + return 0; > + if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64))) > + return -EFAULT; > + return 0; > +} > + > static const struct bpf_link_ops bpf_kprobe_multi_link_lops = { > .release = bpf_kprobe_multi_link_release, > .dealloc = bpf_kprobe_multi_link_dealloc, > + .fill_link_info = bpf_kprobe_multi_link_fill_link_info, > }; > > static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv) > @@ -2890,6 +2917,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > return err; > } > > + link->fp.flags = flags; > return bpf_link_settle(&link_primer); > > error: > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h > index a7b5e91..23691ea 100644 > --- a/tools/include/uapi/linux/bpf.h > +++ b/tools/include/uapi/linux/bpf.h > @@ -6438,6 +6438,11 @@ struct bpf_link_info { > __s32 priority; > __u32 flags; > } netfilter; > + struct { > + __aligned_u64 addrs; /* in/out: addresses buffer ptr */ > + __u32 count; > + __u32 flags; > + } kprobe_multi; > }; > } __attribute__((aligned(8))); > > -- > 1.8.3.1 >
On Sat, Jun 17, 2023 at 1:24 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > With the addition of support for fill_link_info to the kprobe_multi link, > > users will gain the ability to inspect it conveniently using the > > `bpftool link show`. This enhancement provides valuable information to the > > user, including the count of probed functions and their respective > > addresses. It's important to note that if the kptr_restrict setting is not > > permitted, the probed address will not be exposed, ensuring security. > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > --- > > include/uapi/linux/bpf.h | 5 +++++ > > kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++ > > tools/include/uapi/linux/bpf.h | 5 +++++ > > 3 files changed, 38 insertions(+) > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index a7b5e91..23691ea 100644 > > --- a/include/uapi/linux/bpf.h > > +++ b/include/uapi/linux/bpf.h > > @@ -6438,6 +6438,11 @@ struct bpf_link_info { > > __s32 priority; > > __u32 flags; > > } netfilter; > > + struct { > > + __aligned_u64 addrs; /* in/out: addresses buffer ptr */ > > + __u32 count; > > + __u32 flags; > > + } kprobe_multi; > > }; > > } __attribute__((aligned(8))); > > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > index 2bc41e6..742047c 100644 > > --- a/kernel/trace/bpf_trace.c > > +++ b/kernel/trace/bpf_trace.c > > @@ -2548,9 +2548,36 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link) > > kfree(kmulti_link); > > } > > > > +static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link, > > + struct bpf_link_info *info) > > +{ > > + u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs); > > + struct bpf_kprobe_multi_link *kmulti_link; > > + u32 ucount = info->kprobe_multi.count; > > + > > + if (!uaddrs ^ !ucount) > > + return -EINVAL; > > + > > + kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link); > > + if (!uaddrs) { > > + info->kprobe_multi.count = kmulti_link->cnt; > > + return 0; > > + } > > + > > + if (ucount < kmulti_link->cnt) > > + return -EINVAL; > > + info->kprobe_multi.flags = kmulti_link->fp.flags; > > besides what Jiri said, flags should always be returned, just like > cnt. So structure code instead around uaddrs being optional, that will > everything more straightforward (i.e., fill out everything but uaddrs > and then at the end fill out addrs if uaddrs is not zero) Agree. That will be more straightforward. Will change it.
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index a7b5e91..23691ea 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -6438,6 +6438,11 @@ struct bpf_link_info { __s32 priority; __u32 flags; } netfilter; + struct { + __aligned_u64 addrs; /* in/out: addresses buffer ptr */ + __u32 count; + __u32 flags; + } kprobe_multi; }; } __attribute__((aligned(8))); diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 2bc41e6..742047c 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2548,9 +2548,36 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link) kfree(kmulti_link); } +static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link, + struct bpf_link_info *info) +{ + u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs); + struct bpf_kprobe_multi_link *kmulti_link; + u32 ucount = info->kprobe_multi.count; + + if (!uaddrs ^ !ucount) + return -EINVAL; + + kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link); + if (!uaddrs) { + info->kprobe_multi.count = kmulti_link->cnt; + return 0; + } + + if (ucount < kmulti_link->cnt) + return -EINVAL; + info->kprobe_multi.flags = kmulti_link->fp.flags; + if (!kallsyms_show_value(current_cred())) + return 0; + if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64))) + return -EFAULT; + return 0; +} + static const struct bpf_link_ops bpf_kprobe_multi_link_lops = { .release = bpf_kprobe_multi_link_release, .dealloc = bpf_kprobe_multi_link_dealloc, + .fill_link_info = bpf_kprobe_multi_link_fill_link_info, }; static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv) @@ -2890,6 +2917,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr return err; } + link->fp.flags = flags; return bpf_link_settle(&link_primer); error: diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index a7b5e91..23691ea 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -6438,6 +6438,11 @@ struct bpf_link_info { __s32 priority; __u32 flags; } netfilter; + struct { + __aligned_u64 addrs; /* in/out: addresses buffer ptr */ + __u32 count; + __u32 flags; + } kprobe_multi; }; } __attribute__((aligned(8)));
With the addition of support for fill_link_info to the kprobe_multi link, users will gain the ability to inspect it conveniently using the `bpftool link show`. This enhancement provides valuable information to the user, including the count of probed functions and their respective addresses. It's important to note that if the kptr_restrict setting is not permitted, the probed address will not be exposed, ensuring security. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- include/uapi/linux/bpf.h | 5 +++++ kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 5 +++++ 3 files changed, 38 insertions(+)