linux-kernel - Re: [RFC][PATCH 01/22] x86 user stack frame reads: switch to explicit __get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 29 Mar 2020 10:56:59 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     David Laight <David.Laight@...lab.com>
Cc:     Andy Lutomirski <luto@...capital.net>,
        Ingo Molnar <mingo@...nel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Thomas Gleixner <tglx@...utronix.de>, X86 ML <x86@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Borislav Petkov <bp@...en8.de>
Subject: Re: [RFC][PATCH 01/22] x86 user stack frame reads: switch to explicit __get_user()

On Sun, Mar 29, 2020 at 10:41 AM David Laight <David.Laight@...lab.com> 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