| Message ID | 20230623141546.3751-2-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bpf: Support ->fill_link_info for kprobe_multi and perf_event links | expand |
On Fri, Jun 23, 2023 at 7: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..2123197b 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -2459,6 +2459,7 @@ struct bpf_kprobe_multi_link { > u32 cnt; > u32 mods_cnt; > struct module **mods; > + u32 flags; > }; > > struct bpf_kprobe_multi_run_ctx { > @@ -2548,9 +2549,35 @@ 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); > + info->kprobe_multi.count = kmulti_link->cnt; > + info->kprobe_multi.flags = kmulti_link->flags; > + > + if (!uaddrs) > + return 0; > + if (ucount < kmulti_link->cnt) > + return -EINVAL; it would be probably sane behavior to copy ucount items and return -E2BIG > + if (!kallsyms_show_value(current_cred())) > + return 0; at least we should zero out kmulti_link->cnt elements. Otherwise it's hard for user-space know whether returned data is garbage or not? > + if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64))) s/ucount/kmulti_link->cnt/ ? > + 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) > @@ -2862,6 +2889,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr > link->addrs = addrs; > link->cookies = cookies; > link->cnt = cnt; > + link->flags = flags; > > if (cookies) { > /* > 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 24, 2023 at 5:45 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Fri, Jun 23, 2023 at 7: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..2123197b 100644 > > --- a/kernel/trace/bpf_trace.c > > +++ b/kernel/trace/bpf_trace.c > > @@ -2459,6 +2459,7 @@ struct bpf_kprobe_multi_link { > > u32 cnt; > > u32 mods_cnt; > > struct module **mods; > > + u32 flags; > > }; > > > > struct bpf_kprobe_multi_run_ctx { > > @@ -2548,9 +2549,35 @@ 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); > > + info->kprobe_multi.count = kmulti_link->cnt; > > + info->kprobe_multi.flags = kmulti_link->flags; > > + > > + if (!uaddrs) > > + return 0; > > + if (ucount < kmulti_link->cnt) > > + return -EINVAL; > > it would be probably sane behavior to copy ucount items and return -E2BIG Agree. > > > + if (!kallsyms_show_value(current_cred())) > > + return 0; > > at least we should zero out kmulti_link->cnt elements. Otherwise it's > hard for user-space know whether returned data is garbage or not? Agree. Should clear it. > > > > + if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64))) > > s/ucount/kmulti_link->cnt/ ? Yes. Thanks for pointing it out.
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..2123197b 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2459,6 +2459,7 @@ struct bpf_kprobe_multi_link { u32 cnt; u32 mods_cnt; struct module **mods; + u32 flags; }; struct bpf_kprobe_multi_run_ctx { @@ -2548,9 +2549,35 @@ 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); + info->kprobe_multi.count = kmulti_link->cnt; + info->kprobe_multi.flags = kmulti_link->flags; + + if (!uaddrs) + return 0; + if (ucount < kmulti_link->cnt) + return -EINVAL; + 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) @@ -2862,6 +2889,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr link->addrs = addrs; link->cookies = cookies; link->cnt = cnt; + link->flags = flags; if (cookies) { /* 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(+)