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> <CAHk-=whDAxb+83gYCv4=-armoqXQXgzshaVCCe9dNXZb9G_CxQ@mail.gmail.com>
 <9352bc55302d4589aaf2461c7b85fb6b@AcuMS.aculab.com>
In-Reply-To: <9352bc55302d4589aaf2461c7b85fb6b@AcuMS.aculab.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sun, 29 Mar 2020 11:16:19 -0700
Message-ID: <CAHk-=wjEf+0sBkPFKWpYZK_ygS9=ig3KTZkDe5jkDj+v8i7B+w@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 11:03 AM David Laight <David.Laight@aculab.com> wrote:
>
> > 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
>
> I must be mis-remembering the object code from last time
> I looked at it.

On an object code level, the end result actually almost looks like a
normal call, until you start looking at the exact register passing
details.

On a source level, it's anything but.

This is "get_user()" on x86:

  #define get_user(x, ptr)                                              \
  ({                                                                    \
        int __ret_gu;                                                   \
        register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX);            \
        __chk_user_ptr(ptr);                                            \
        might_fault();                                                  \
        asm volatile("call __get_user_%P4"                              \
                     : "=a" (__ret_gu), "=r" (__val_gu),                \
                        ASM_CALL_CONSTRAINT                             \
                     : "0" (ptr), "i" (sizeof(*(ptr))));                \
        (x) = (__force __typeof__(*(ptr))) __val_gu;                    \
        __builtin_expect(__ret_gu, 0);                                  \
  })

and all it actually *generates* is that single "call" instruction, so
the only code you see from the above mess in the object file (assuming
you don't have debugging enabled that expands "might_fault()" into a
lot of checking code) is something like this:

        call __get_user_4

with the input address being in %rax (which is also the returning
error code), and the value of 'x' being in %rdx.

The above macro looks a bit messy, but that's actually hiding some of
the real complexity (that "__inttype()" thing is a mess of compiler
tricks in itself, as is the magical ASM_CALL_CONSTRAINT thing, along
with the magic that goes into a 64-bit return value on x86-32 which is
implicit in the magical register asm() definition).

The uaccess code is a lot of complex macros and inline functions to
make it all work well.

Al's uaccess cleanups (which should be in the next merge window) help
a _lot_. Not with the get_user() above (that is already about as
simple as it can get), but with a lot of the other crazy special cases
that we had grown over the years.

My current "unsafe_get_user() using clang and 'asm goto' with outpus"
patch is on top of Al's patches exactly because it was cleaner to do
with all of his cleanups.

But that magical asm-goto-with-outputs patch probably won't land
upstream for a couple of years.

I had the unsafe_put_user() patches to use 'asm goto' in a local
branch for almost three years before I made them official. See commit
a959dc88f9c8 ("Use __put_user_goto in __put_user_size() and
unsafe_put_user()") and notice how it has an author date of May, 2016,
but was only committed January 2019.

I basically waited until "asm goto" was something we could rely on
(and used for other reasons).

I suspect something very similar will happen with unsafe_get_user(). I
have a patch if people want to play with it, but right now it's a
"this relies on an unreleased version of clang to even work".

                      Linus