Re: [RFC][PATCH 01/22] x86 user stack frame reads: switch to explicit __get_user()

From: Linus Torvalds
Date: Sun Mar 29 2020 - 13:57:26 EST


On Sun, Mar 29, 2020 at 10:41 AM David Laight <David.Laight@xxxxxxxxxx> wrote:
>
> It may be worth implementing get_user() as an inline
> function that writes the result of access_ok() to a
> 'by reference' parameter and then returns the value
> from an 'real' __get_user() function.

That's how get_user() already works.

It is a polymorphic function (done using macros, sizeof() and ugly
compiler tricks) that generates a call, yes. But it's not a normal C
call. On x86-64, it returns the error code in %rax, and the value in
%rdx

So "get_user()" is already basically optimal. It's likely *faster*
than __get_user(), because it has a smaller I$ footprint if you do
multiple ones.

But, if you have lots of performance-critical get_user() calls, just use

if (user_access_begin(..))
goto efault;

.. multiple "unsafe_get_user(x,ptr,efault);" ..

user_access_end();
...

efault:
user_access_end();
return -EFAULT;

and be done with it.

Yes, the above sequence looks cumbersome, but it's designed for doing
multiple accesses together efficiently. It's basically the "I actually
had a good reason to use __get_user(), but it sucks now, so this is
the new interface"

It's designed for multiple accesses, because as mentioned, if you only
have one, then "get_user()" is already optimal.

And yes, the interface (with that "label for error cases") is
optimized for a (future) world where the compiler can do "asm goto"
together with outputs. Any exception on the access doesn't actually
generate a test at all, the exception will branch directly to the
error label instead.

That already works for "unsafe_put_user()", but for
"unsafe_get_user()" you need a compiler that can do that kind of "asm
goto".

If you use a modern clang version (ie build clang from git), I can
send you a patch for the kernel to try (and a patch for clang to fix a
bug, unless it's been already merged, I didn't check).

The above will generate basically _optimal_ code with my patch and
that modern clang version.

Linus