Re: [PATCH] Convert filldir[64]() from __put_user() to unsafe_put_user()

From: Al Viro
Date: Mon Oct 07 2019 - 13:34:35 EST


On Sun, Oct 06, 2019 at 08:11:42PM -0700, Linus Torvalds wrote:

> > So do we want to bother with separation between raw_copy_to_user() and
> > unsafe_copy_to_user()? After all, __copy_to_user() also has only few
> > callers, most of them in arch/*
>
> No, you're right. Just switch over.
>
> > I'll take a look into that tomorrow - half-asleep right now...
>
> Thanks. No huge hurry.

Tangentially related: copy_regster_to_user() and copy_regset_from_user().
That's where we do access_ok(), followed by calls of ->get() and
->set() resp. Those tend to either use user_regset_copy{out,in}(),
or open-code those. The former variant tends to lead to few calls
of __copy_{to,from}_user(); the latter... On x86 it ends up doing
this:
static int genregs_get(struct task_struct *target,
const struct user_regset *regset,
unsigned int pos, unsigned int count,
void *kbuf, void __user *ubuf)
{
if (kbuf) {
unsigned long *k = kbuf;
while (count >= sizeof(*k)) {
*k++ = getreg(target, pos);
count -= sizeof(*k);
pos += sizeof(*k);
}
} else {
unsigned long __user *u = ubuf;
while (count >= sizeof(*u)) {
if (__put_user(getreg(target, pos), u++))
return -EFAULT;
count -= sizeof(*u);
pos += sizeof(*u);
}
}

return 0;
}

Potentially doing arseloads of stac/clac as it goes. OTOH, getreg()
(and setreg()) in there are not entirely trivial, so blanket
user_access_begin()/user_access_end() over the entire loop might be
a bad idea...

How hot is that codepath? I know that arch/um used to rely on it
(== PTRACE_[GS]ETREGS) quite a bit...