linux-kernel - Re: [PATCH] x86/uaccess: fix code generation in put

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]

Message-ID: <CAHk-=wjrmKk-94MTNwgf5oGP2VhU3iK5b9YqH-QS2AgvG4mSaw@mail.gmail.com>
Date:   Fri, 23 Oct 2020 13:52:58 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Rasmus Villemoes <linux@...musvillemoes.dk>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        X86 ML <x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Naresh Kamboju <naresh.kamboju@...aro.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/uaccess: fix code generation in put_user()

On Fri, Oct 23, 2020 at 1:42 PM Andy Lutomirski <luto@...nel.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