On Fri, Jun 9, 2023 at 2:14 AM Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
>
> On Fri, Jun 9, 2023 at 7:05 AM Andrii Nakryiko
> <andrii.nakryiko@xxxxxxxxx> wrote:
> >
> > On Thu, Jun 8, 2023 at 3:35 AM Yafang Shao <laoar.shao@xxxxxxxxx> 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@xxxxxxxxx>
> > > ---
> > > 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
