Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200323183620.GD23230@ZenIV.linux.org.uk> <20200323183819.250124-1-viro@ZenIV.linux.org.uk>
 <20200328104857.GA93574@gmail.com> <20200328115936.GA23230@ZenIV.linux.org.uk>
 <20200329092602.GB93574@gmail.com> <CALCETrX=nXN14fqu-yEMGwwN-vdSz=-0C3gcOMucmxrCUpevdA@mail.gmail.com>
 <489c9af889954649b3453e350bab6464@AcuMS.aculab.com>
In-Reply-To: <489c9af889954649b3453e350bab6464@AcuMS.aculab.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sun, 29 Mar 2020 10:56:59 -0700
Message-ID: <CAHk-=whDAxb+83gYCv4=-armoqXQXgzshaVCCe9dNXZb9G_CxQ@mail.gmail.com>
Subject: Re: [RFC][PATCH 01/22] x86 user stack frame reads: switch to explicit __get_user()
To:     David Laight <David.Laight@aculab.com>
Cc:     Andy Lutomirski <luto@amacapital.net>,
        Ingo Molnar <mingo@kernel.org>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Thomas Gleixner <tglx@linutronix.de>, X86 ML <x86@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Borislav Petkov <bp@alien8.de>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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