On Fri, Oct 27, 2023 at 7:30 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Thu, Oct 26, 2023 at 10:55:35AM -0700, Song Liu wrote:
> > On Wed, Oct 25, 2023 at 1:24 PM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> > [...]
[...]
> >
> > > + if (upath_max > PATH_MAX)
> > > + return -E2BIG;
> >
> > I don't think we need to fail here. How about we simply do
> >
> > upath_max = min_ut(u32, upath_max, PATH_MAX);
>
> ok
+1, was going to say the same
>
> >
> > > + buf = kmalloc(upath_max, GFP_KERNEL);
> > > + 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;
> > > +
> > > + 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;
> >
> > It feels expensive to put_user() 3x in a loop. Maybe we need a new struct
> > with offset, ref_ctr_offset, and cookie?
>
> good idea, I think we could store offsets/uref_ctr_offsets/cookies
> together both in kernel and user sapce and use just single put_user
> call... will check
>
hm... only offset is mandatory, and then we can have either cookie or
ref_ctr_offset or both, so co-locating them in the same struct seems
inconvenient and unnecessary
> thanks,
> jirka
>
> >
> > Thanks,
> > Song
> >
> > > + }
> > > +
> > > + return err;
> > > +}
> > > +
>
