On Fri, Dec 10, 2021 at 11:51:10PM +0900, Masami Hiramatsu wrote:
> Hi Beau,
>
> On Thu, 9 Dec 2021 14:32:06 -0800
> Beau Belgrave <beaub@xxxxxxxxxxxxxxxxxxx> wrote:
>
> > Pass iterator through to probes to allow copying data directly to the
> > probe buffers instead of taking multiple copies. Enables eBPF user and
> > raw iterator types out to programs for no-copy scenarios.
> >
> > Signed-off-by: Beau Belgrave <beaub@xxxxxxxxxxxxxxxxxxx>
> > ---
[..]
> > @@ -1009,32 +1059,28 @@ static ssize_t user_events_write_core(struct file *file, struct iov_iter *i)
> > if (likely(atomic_read(&tp->key.enabled) > 0)) {
> > struct tracepoint_func *probe_func_ptr;
> > user_event_func_t probe_func;
> > + struct iov_iter copy;
> > void *tpdata;
> > - void *kdata;
> > - u32 datalen;
> >
> > - kdata = kmalloc(i->count, GFP_KERNEL);
> > -
> > - if (unlikely(!kdata))
> > - return -ENOMEM;
> > -
> > - datalen = copy_from_iter(kdata, i->count, i);
> > + if (unlikely(iov_iter_fault_in_readable(i, i->count)))
> > + return -EFAULT;
> >
> > rcu_read_lock_sched();
> > + pagefault_disable();
>
> Since the pagefault_disable() may have unexpected side effect,
> I think it should be used really limited area, e.g. around
> actual memory access function.
> Can we move this around the copy_nofault()?
>
Sure thing.
> Thank you,
> >
> > probe_func_ptr = rcu_dereference_sched(tp->funcs);
> >
> > if (probe_func_ptr) {
> > do {
> > + copy = *i;
> > probe_func = probe_func_ptr->func;
> > tpdata = probe_func_ptr->data;
> > - probe_func(user, kdata, datalen, tpdata);
> > + probe_func(user, ©, tpdata);
> > } while ((++probe_func_ptr)->func);
> > }
> >
> > + pagefault_enable();
> > rcu_read_unlock_sched();
> > -
> > - kfree(kdata);
> > }
> >
> > return ret;
> > --
> > 2.17.1
> >
>
>
> --
> Masami Hiramatsu <mhiramat@xxxxxxxxxx>
Thanks,
-Beau
|