Re: [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event

[Yafang Shao <laoar.shao@gmail.com>]

On Thu, Jun 15, 2023 at 6:21 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Jun 12, 2023 at 03:16:06PM +0000, Yafang Shao wrote:
>
> SNIP
>
> >
> >  /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 80c9ec0..fe354d5 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3303,9 +3303,133 @@ static void bpf_perf_link_dealloc(struct bpf_link *link)
> >       kfree(perf_link);
> >  }
> >
> > +static int bpf_perf_link_fill_name(const struct perf_event *event,
> > +                                char __user *uname, u32 ulen,
> > +                                u64 *probe_offset, u64 *probe_addr,
> > +                                u32 *fd_type)
> > +{
>
> this function name sounds misleading, it does query all the link data
> plus copying the name.. seems like this should be renamed and separated

Will do it.

>
>
> > +     const char *buf;
> > +     u32 prog_id;
> > +     size_t len;
> > +     int err;
> > +
> > +     if (!ulen ^ !uname)
> > +             return -EINVAL;
> > +     if (!uname)
> > +             return 0;
> > +
> > +     err = bpf_get_perf_event_info(event, &prog_id, fd_type, &buf,
> > +                                   probe_offset, probe_addr);
> > +     if (err)
> > +             return err;
> > +
> > +     len = strlen(buf);
> > +     if (buf) {
> > +             err = bpf_copy_to_user(uname, buf, ulen, len);
> > +             if (err)
> > +                     return err;
> > +     } else {
> > +             char zero = '\0';
> > +
> > +             if (put_user(zero, uname))
> > +                     return -EFAULT;
> > +     }
> > +     return 0;
> > +}
> > +
> > +static int bpf_perf_link_fill_probe(const struct perf_event *event,
> > +                                 struct bpf_link_info *info)
> > +{
> > +     char __user *uname;
> > +     u64 addr, offset;
> > +     u32 ulen, type;
> > +     int err;
> > +
> > +#ifdef CONFIG_KPROBE_EVENTS
>
> this will break compilation when CONFIG_KPROBE_EVENTS or CONFIG_UPROBE_EVENTS
> options are not defined

Indeed. Will improve it.

-- 
Regards
Yafang