On 11/23/23 4:20 AM, Jiri Olsa wrote:
On Wed, Nov 22, 2023 at 10:50:06PM +0100, Jiri Olsa wrote:
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
hum, actually.. after checking d_path IIUC it copies into the end of buffer,
so I can't see this code copying more data to user buffer
Double checked as well. Indeed, the path is copied to the end of buffer,
so kmalloc() should be okay. Sorry for noise.
