On Mon, Nov 20, 2023 at 10:04:16AM -0800, Yonghong Song wrote:
SNIP
> > +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_size = info->uprobe_multi.path_size;
> > + struct bpf_uprobe_multi_link *umulti_link;
> > + u32 ucount = info->uprobe_multi.count;
> > + int err = 0, i;
> > + long left;
> > +
> > + if (!upath ^ !upath_size)
> > + return -EINVAL;
> > +
> > + if ((uoffsets || uref_ctr_offsets || ucookies) && !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_ns(umulti_link->task, task_active_pid_ns(current)) : 0;
> > +
> > + if (upath) {
> > + char *p, *buf;
> > +
> > + upath_size = min_t(u32, upath_size, PATH_MAX);
> > +
> > + buf = kmalloc(upath_size, GFP_KERNEL);
> > + if (!buf)
> > + return -ENOMEM;
> > + p = d_path(&umulti_link->path, buf, upath_size);
> > + if (IS_ERR(p)) {
> > + kfree(buf);
> > + return -ENOSPC;
>
> Should we just return PTR_ERR(p)? In d_path, it is possible that
> -ENAMETOOLONG is returned. But path->dentry->d_op->d_dname() might
> return a different error reason than -ENAMETOOLONG or -ENOSPC?
true, will change
>
> > + }
> > + upath_size = buf + upath_size - p;
> > + left = copy_to_user(upath, p, upath_size);
>
> Here, the data copied to user may contain more than
> actual path itself. I am okay with this since this
> is not in critical path. But early buf allocation is using
> kmalloc whose content could be arbitrary. Should we
> use kzalloc for the above 'buf' allocation?
good catch, will use kzalloc
thanks,
jirka
