linux-kernel - Re: [PATCH v5] x86: use builtins to read eflags

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: <fd89333f-e470-a295-baf6-a736c55caeb5@citrix.com>
Date:   Thu, 17 Mar 2022 21:05:52 +0000
From:   Andrew Cooper <Andrew.Cooper3@...rix.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>
CC:     "H. Peter Anvin" <hpa@...or.com>, Bill Wendling <morbo@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        "Nathan Chancellor" <nathan@...nel.org>,
        Juergen Gross <jgross@...e.com>,
        "Peter Zijlstra" <peterz@...radead.org>,
        Andy Lutomirski <luto@...nel.org>,
        "llvm@...ts.linux.dev" <llvm@...ts.linux.dev>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-toolchains <linux-toolchains@...r.kernel.org>,
        Andrew Cooper <Andrew.Cooper3@...rix.com>
Subject: Re: [PATCH v5] x86: use builtins to read eflags

On 17/03/2022 18:52, Linus Torvalds wrote:
> On Thu, Mar 17, 2022 at 11:00 AM Nick Desaulniers
> <ndesaulniers@...gle.com> wrote:
>>>> One change you may see due to this patch is the compiler moving the
>>>> "pop %..." instruction away from the "pushf" instruction. This could
>>>> happen if the compiler determines that it could produce better code by
>>>> doing so---e.g. to reduce register pressure. The "gcc -O2" example
>>>> below shows this code movement.
> Honestly, that part worries me a _lot_.
>
> Why?
>
> Because clang in particular has already messed up eflags handling
> once, by spilling condition codes (in the form of eflags) onto the
> stack, and then loading them later with a "popf".
>
> And it did so across a function call THAT MODIFIED 'IF'. This was a
> major bug in clang back in 2015 or so, and made it unusable for the
> kernel.
>
> See for example
>
>     https://lore.kernel.org/all/CA+icZUU7y5ATSLV_0TGzi5m5deWADLmAMBkAT32FKGyUWNSJSA@mail.gmail.com/
>
> for some kernel discussion, and separately
>
>     https://lists.llvm.org/pipermail/llvm-dev/2015-July/088774.html
>
> for just llvm discussions.
>
> It is perhaps telling that the LLVM discussion I found seems to talk
> more about the performance impact, not about the fact that THE
> GENERATED CODE WAS WRONG.
>
> That compiler bug would basically enable or disable interrupts in
> random places - because clang developers thought that 'eflags' is only
> about the condition codes.

I was idly playing with this builtin in response to the thread, and
found that GCC has a far more serious bug than Clang had.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104971

If the optimiser concluded that the result of the readflags builtin is
unused, it generates:

broken:
    pushf
    ret

~Andrew