Re: [PATCH v2 bpf-next 01/11] bpf: Support ->fill_link_info for kprobe_multi

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

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
>