| Message ID | 20230608103523.102267-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 Thu, Jun 8, 2023 at 3:35 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` command. 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 set > to 2, the probed addresses 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 | 30 ++++++++++++++++++++++++++++++ > tools/include/uapi/linux/bpf.h | 5 +++++ > 3 files changed, 40 insertions(+) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index a7b5e91..d99cc16 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; > + __u8 retprobe; from kernel API side it's probably better to just expose flags? retprobe is determined by BPF_F_KPROBE_MULTI_RETURN flag > + } kprobe_multi; > }; > } __attribute__((aligned(8))); > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 2bc41e6..738efcf 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -2548,9 +2548,39 @@ 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) > + return 0; > + if (ucount != kmulti_link->cnt) > + return -EINVAL; should this just check that kmulti_link->cnt is <= ucount?... > + info->kprobe_multi.retprobe = kmulti_link->fp.exit_handler ? > + true : false; > + if (kptr_restrict == 2) > + return 0; use kallsyms_show_value() instead of hard-coding this? > + 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) > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h > index a7b5e91..d99cc16 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; > + __u8 retprobe; > + } kprobe_multi; > }; > } __attribute__((aligned(8))); > > -- > 1.8.3.1 >
On Fri, Jun 9, 2023 at 7:05 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, Jun 8, 2023 at 3:35 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` command. 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 set > > to 2, the probed addresses 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 | 30 ++++++++++++++++++++++++++++++ > > tools/include/uapi/linux/bpf.h | 5 +++++ > > 3 files changed, 40 insertions(+) > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index a7b5e91..d99cc16 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; > > + __u8 retprobe; > > from kernel API side it's probably better to just expose flags? Agreed. The flags will be extensible. > retprobe is determined by BPF_F_KPROBE_MULTI_RETURN flag Should we print 'flags' in `bpftool link show` directly? As we print it in `bpftool map show`. > > > + } kprobe_multi; > > }; > > } __attribute__((aligned(8))); > > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > index 2bc41e6..738efcf 100644 > > --- a/kernel/trace/bpf_trace.c > > +++ b/kernel/trace/bpf_trace.c > > @@ -2548,9 +2548,39 @@ 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) > > + return 0; > > + if (ucount != kmulti_link->cnt) > > + return -EINVAL; > > should this just check that kmulti_link->cnt is <= ucount?... Agreed. > > > + info->kprobe_multi.retprobe = kmulti_link->fp.exit_handler ? > > + true : false; > > + if (kptr_restrict == 2) > > + return 0; > > use kallsyms_show_value() instead of hard-coding this? Good point. Will use it.
On Fri, Jun 9, 2023 at 2:14 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > On Fri, Jun 9, 2023 at 7:05 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Thu, Jun 8, 2023 at 3:35 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` command. 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 set > > > to 2, the probed addresses 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 | 30 ++++++++++++++++++++++++++++++ > > > tools/include/uapi/linux/bpf.h | 5 +++++ > > > 3 files changed, 40 insertions(+) > > > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > > index a7b5e91..d99cc16 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; > > > + __u8 retprobe; > > > > from kernel API side it's probably better to just expose flags? > > Agreed. The flags will be extensible. > > > retprobe is determined by BPF_F_KPROBE_MULTI_RETURN flag > > Should we print 'flags' in `bpftool link show` directly? As we print > it in `bpftool map show`. specifically for kprobe vs kretprobe (and similarly uprobe vs uretprobe), if bpftool can make it human-readable it would be best. We can also additionally print flags, but I don't know how useful it would be. > > > > > > + } kprobe_multi; > > > }; > > > } __attribute__((aligned(8))); > > > > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > > index 2bc41e6..738efcf 100644 > > > --- a/kernel/trace/bpf_trace.c > > > +++ b/kernel/trace/bpf_trace.c > > > @@ -2548,9 +2548,39 @@ 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) > > > + return 0; > > > + if (ucount != kmulti_link->cnt) > > > + return -EINVAL; > > > > should this just check that kmulti_link->cnt is <= ucount?... > > Agreed. > > > > > > + info->kprobe_multi.retprobe = kmulti_link->fp.exit_handler ? > > > + true : false; > > > + if (kptr_restrict == 2) > > > + return 0; > > > > use kallsyms_show_value() instead of hard-coding this? > > Good point. Will use it. > > -- > Regards > Yafang
On Sat, Jun 10, 2023 at 2:25 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Fri, Jun 9, 2023 at 2:14 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > On Fri, Jun 9, 2023 at 7:05 AM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Thu, Jun 8, 2023 at 3:35 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` command. 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 set > > > > to 2, the probed addresses 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 | 30 ++++++++++++++++++++++++++++++ > > > > tools/include/uapi/linux/bpf.h | 5 +++++ > > > > 3 files changed, 40 insertions(+) > > > > > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > > > index a7b5e91..d99cc16 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; > > > > + __u8 retprobe; > > > > > > from kernel API side it's probably better to just expose flags? > > > > Agreed. The flags will be extensible. > > > > > retprobe is determined by BPF_F_KPROBE_MULTI_RETURN flag > > > > Should we print 'flags' in `bpftool link show` directly? As we print > > it in `bpftool map show`. > > specifically for kprobe vs kretprobe (and similarly uprobe vs > uretprobe), if bpftool can make it human-readable it would be best. We > can also additionally print flags, but I don't know how useful it > would be. Got it. Thanks. Will print 'retprobe' only.
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index a7b5e91..d99cc16 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; + __u8 retprobe; + } kprobe_multi; }; } __attribute__((aligned(8))); diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 2bc41e6..738efcf 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2548,9 +2548,39 @@ 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) + return 0; + if (ucount != kmulti_link->cnt) + return -EINVAL; + info->kprobe_multi.retprobe = kmulti_link->fp.exit_handler ? + true : false; + if (kptr_restrict == 2) + 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) diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index a7b5e91..d99cc16 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; + __u8 retprobe; + } 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` command. 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 set to 2, the probed addresses 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 | 30 ++++++++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 5 +++++ 3 files changed, 40 insertions(+)