| Message ID | 20230628115329.248450-2-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: Support ->fill_link_info for kprobe_multi and perf_event links | expand |
On Wed, Jun 28, 2023 at 4:53 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> > --- documentation nit, but otherwise LGTM Also, looking at other patch where you introduce bpf_copy_user(), seems like we return -ENOSPC when user-provided memory is not big enough. So let's change E2BIG in this patch to ENOSPC? Acked-by: Andrii Nakryiko <andrii@kernel.org> > include/uapi/linux/bpf.h | 5 +++++ > kernel/trace/bpf_trace.c | 37 ++++++++++++++++++++++++++++++++++ > tools/include/uapi/linux/bpf.h | 5 +++++ > 3 files changed, 47 insertions(+) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 60a9d59beeab..512ba3ba2ed3 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -6439,6 +6439,11 @@ struct bpf_link_info { > __s32 priority; > __u32 flags; > } netfilter; > + struct { > + __aligned_u64 addrs; /* in/out: addresses buffer ptr */ addrs field itself is not modified, the memory it points to is modified, so it's not really in/out per se, it is just a pointer to memory where kernel stores output data > + __u32 count; but count field itself is in/out, so please add a comment explicitly stating this > + __u32 flags; > + } kprobe_multi; > }; > } __attribute__((aligned(8))); > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 03b7f6b8e4f0..1f9f78e1992f 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -2469,6 +2469,7 @@ struct bpf_kprobe_multi_link { > u32 cnt; > u32 mods_cnt; > struct module **mods; > + u32 flags; > }; > > struct bpf_kprobe_multi_run_ctx { > @@ -2558,9 +2559,44 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link) > kfree(kmulti_link); > } > [...]
On Fri, Jul 7, 2023 at 6:00 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Wed, Jun 28, 2023 at 4:53 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> > > --- > > documentation nit, but otherwise LGTM > > Also, looking at other patch where you introduce bpf_copy_user(), > seems like we return -ENOSPC when user-provided memory is not big > enough. So let's change E2BIG in this patch to ENOSPC? Sure. Will change it. > > > Acked-by: Andrii Nakryiko <andrii@kernel.org> Thanks for your review. > > > include/uapi/linux/bpf.h | 5 +++++ > > kernel/trace/bpf_trace.c | 37 ++++++++++++++++++++++++++++++++++ > > tools/include/uapi/linux/bpf.h | 5 +++++ > > 3 files changed, 47 insertions(+) > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index 60a9d59beeab..512ba3ba2ed3 100644 > > --- a/include/uapi/linux/bpf.h > > +++ b/include/uapi/linux/bpf.h > > @@ -6439,6 +6439,11 @@ struct bpf_link_info { > > __s32 priority; > > __u32 flags; > > } netfilter; > > + struct { > > + __aligned_u64 addrs; /* in/out: addresses buffer ptr */ > > addrs field itself is not modified, the memory it points to is > modified, so it's not really in/out per se, it is just a pointer to > memory where kernel stores output data Thanks for the explanation. Will change it. > > > + __u32 count; > > but count field itself is in/out, so please add a comment explicitly > stating this Will do it.
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 60a9d59beeab..512ba3ba2ed3 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -6439,6 +6439,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 03b7f6b8e4f0..1f9f78e1992f 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2469,6 +2469,7 @@ struct bpf_kprobe_multi_link { u32 cnt; u32 mods_cnt; struct module **mods; + u32 flags; }; struct bpf_kprobe_multi_run_ctx { @@ -2558,9 +2559,44 @@ 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; + int err = 0, i; + + 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) + err = -E2BIG; + else + ucount = kmulti_link->cnt; + + if (kallsyms_show_value(current_cred())) { + if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64))) + return -EFAULT; + } else { + for (i = 0; i < ucount; i++) { + if (put_user(0, uaddrs + i)) + return -EFAULT; + } + } + return err; +} + 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) @@ -2872,6 +2908,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 60a9d59beeab..512ba3ba2ed3 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -6439,6 +6439,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 | 37 ++++++++++++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 5 +++++ 3 files changed, 47 insertions(+)