On Wed, Oct 25, 2023 at 1:24 PM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> Adding support to get uprobe_link details through bpf_link_info
> interface.
>
> Adding new struct uprobe_multi to struct bpf_link_info to carry
> the uprobe_multi link details.
>
> The uprobe_multi.count is passed from user space to denote size
> of array fields (offsets/ref_ctr_offsets/cookies). The actual
> array size is stored back to uprobe_multi.count (allowing user
> to find out the actual array size) and array fields are populated
> up to the user passed size.
>
> All the non-array fields (path/count/flags/pid) are always set.
>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> include/uapi/linux/bpf.h | 10 +++++
> kernel/trace/bpf_trace.c | 68 ++++++++++++++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 10 +++++
> 3 files changed, 88 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 0f6cdf52b1da..960cf2914d63 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -6556,6 +6556,16 @@ struct bpf_link_info {
> __u32 flags;
> __u64 missed;
> } kprobe_multi;
> + struct {
> + __aligned_u64 path;
> + __aligned_u64 offsets;
> + __aligned_u64 ref_ctr_offsets;
> + __aligned_u64 cookies;
> + __u32 path_max; /* in/out: uprobe_multi path size */
people already called out that path_size makes for a better name, I agree
> + __u32 count; /* in/out: uprobe_multi offsets/ref_ctr_offsets/cookies count */
otherwise we'd have to call this count_max :)
> + __u32 flags;
> + __u32 pid;
> + } uprobe_multi;
> struct {
> __u32 type; /* enum bpf_perf_event_type */
> __u32 :32;
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 843b3846d3f8..9f8ad19a1a93 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -3042,6 +3042,7 @@ struct bpf_uprobe_multi_link {
> u32 cnt;
> struct bpf_uprobe *uprobes;
> struct task_struct *task;
> + u32 flags;
> };
>
> struct bpf_uprobe_multi_run_ctx {
> @@ -3081,9 +3082,75 @@ static void bpf_uprobe_multi_link_dealloc(struct bpf_link *link)
> kfree(umulti_link);
> }
>
> +static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link,
> + struct bpf_link_info *info)
> +{
> + u64 __user *uref_ctr_offsets = u64_to_user_ptr(info->uprobe_multi.ref_ctr_offsets);
> + u64 __user *ucookies = u64_to_user_ptr(info->uprobe_multi.cookies);
> + u64 __user *uoffsets = u64_to_user_ptr(info->uprobe_multi.offsets);
> + u64 __user *upath = u64_to_user_ptr(info->uprobe_multi.path);
> + u32 upath_max = info->uprobe_multi.path_max;
> + struct bpf_uprobe_multi_link *umulti_link;
> + u32 ucount = info->uprobe_multi.count;
> + int err = 0, i;
> + char *p, *buf;
> + long left;
> +
> + if (!upath ^ !upath_max)
> + return -EINVAL;
> +
> + if (!uoffsets ^ !ucount)
> + return -EINVAL;
> +
> + umulti_link = container_of(link, struct bpf_uprobe_multi_link, link);
> + info->uprobe_multi.count = umulti_link->cnt;
> + info->uprobe_multi.flags = umulti_link->flags;
> + info->uprobe_multi.pid = umulti_link->task ?
> + task_pid_nr(umulti_link->task) : (u32) -1;
on attach we do
task = get_pid_task(find_vpid(pid), PIDTYPE_PID);
So on attachment we take pid in user's namespace, is that right? It's
kind of asymmetrical that we return the global PID back? Should we try
to convert PID to user's namespace instead?
> +
> + if (upath) {
> + if (upath_max > PATH_MAX)
> + return -E2BIG;
no need to fail here, as pointed out elsewhere
> + buf = kmalloc(upath_max, GFP_KERNEL);
here we can allocate min(PATH_MAX, upath_max)
> + if (!buf)
> + return -ENOMEM;
> + p = d_path(&umulti_link->path, buf, upath_max);
> + if (IS_ERR(p)) {
> + kfree(buf);
> + return -ENOSPC;
> + }
> + left = copy_to_user(upath, p, buf + upath_max - p);
> + kfree(buf);
> + if (left)
> + return -EFAULT;
> + }
> +
> + if (!uoffsets)
> + return 0;
it would be good to still return actual counts for out parameters, no?
> +
> + if (ucount < umulti_link->cnt)
> + err = -ENOSPC;
> + else
> + ucount = umulti_link->cnt;
> +
> + for (i = 0; i < ucount; i++) {
> + if (put_user(umulti_link->uprobes[i].offset, uoffsets + i))
> + return -EFAULT;
> + if (uref_ctr_offsets &&
> + put_user(umulti_link->uprobes[i].ref_ctr_offset, uref_ctr_offsets + i))
> + return -EFAULT;
> + if (ucookies &&
> + put_user(umulti_link->uprobes[i].cookie, ucookies + i))
> + return -EFAULT;
> + }
> +
> + return err;
> +}
> +
[...]
