Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201023203154.27335-1-linux@rasmusvillemoes.dk> <CALCETrV2rzowdWqhEG7v-hmbUOpNHsBz-hR1RdxBFKuwU6rDTA@mail.gmail.com>
In-Reply-To: <CALCETrV2rzowdWqhEG7v-hmbUOpNHsBz-hR1RdxBFKuwU6rDTA@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Fri, 23 Oct 2020 13:52:58 -0700
Message-ID: <CAHk-=wjrmKk-94MTNwgf5oGP2VhU3iK5b9YqH-QS2AgvG4mSaw@mail.gmail.com>
Subject: Re: [PATCH] x86/uaccess: fix code generation in put_user()
To:     Andy Lutomirski <luto@kernel.org>
Cc:     Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        X86 ML <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
        Sean Christopherson <sean.j.christopherson@intel.com>,
        Naresh Kamboju <naresh.kamboju@linaro.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Oct 23, 2020 at 1:42 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> This looks like the patch is an improvement, but this is still IMO
> likely to be very fragile.  Can we just do the size-dependent "a" vs
> "A" selection method instead?  Sure, it's a little more code, but it
> will be Obviously Correct.  As it stands, I can easily see our code
> failing on some gcc or clang version and the compiler authors telling
> us that we're relying on unsupportable behavior.

Well, the "register asm()" thing actually _is_ documented, and it's
something we've used before too (and still use in other places).

We actually have quite a bit of them - just do a

    git grep 'register.*asm('

to see literally hundreds of them. So gcc (and clang) actually has a
lot of test-cases for it.

In many ways, x86 actually tends to need _fewer_ of these than most
other architectures, since on x86, we almost always have specific
register naming in ways that other architectures often don't (because
x86 has all those specific register rules).

So the 64-bit issue for x86-32 is a bit of a special case for x86, but
this is in no way a special case in general, and is quite the common
pattern on other architectures.

The fact that KASAN could generate code in between the register asm
assignment was something I just overlooked (and embarrassingly similar
to the issues we had with objdump checking STAC/CLAC back when we
inlined that all).

It's a bit sad that gcc doesn't _warn_ about this kind of issue, since
the compiler certainly should see "oh, we just had a register clash".
But it is what it is..

                Linus