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: <8F6F31A0-0AFC-477D-8B5F-9E8B308CDDAA@zytor.com>
Date:   Fri, 18 Mar 2022 15:33:31 -0700
From:   "H. Peter Anvin" <hpa@...or.com>
To:     Segher Boessenkool <segher@...nel.crashing.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
CC:     Andy Lutomirski <luto@...nel.org>,
        Andrew Cooper <Andrew.Cooper3@...rix.com>,
        Nick Desaulniers <ndesaulniers@...gle.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>,
        "llvm@...ts.linux.dev" <llvm@...ts.linux.dev>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-toolchains <linux-toolchains@...r.kernel.org>
Subject: Re: [PATCH v5] x86: use builtins to read eflags

On March 18, 2022 3:09:01 PM PDT, Segher Boessenkool <segher@...nel.crashing.org> wrote:
>On Fri, Mar 18, 2022 at 11:19:28AM -0700, Linus Torvalds wrote:
>> Or rather, it's not the redzoning itself, but the fact that the
>> compiler might use the word under the stack for random other things,
>> and the pushf will then corrupt some local variable storage.
>> 
>> I think it would be lovely to solve that in inline asm itself some way
>> - by marking the stack pointer clobbered or something.
>
>Inline assembler does not allow you to change the stack pointer, in
>principle.  You have to return everything to its original state before
>you return control from the asm code, and you have to deal with what
>happens if am interrupt comes in halfway through the asm, and all other
>ABI things that may happen on your platform.
>
>> Because you have the same issue if an inline asm might need to do a
>> function call - think magic calling conventions etc, but also possibly
>> slow-path cases.
>
>Yes.  The compiler itself can deal with all the red zone restrictions --
>precisely *because* it is in full control of the stack frame -- but
>those restrictions are very real.  It generally is a very good idea to
>have a redzone though, without it you pay much more than necessary for
>frame setup and teardown in leaf functions (similar to some of what the
>misnamed "shrink-wrapping" optimisation does, but the two are mostly
>independent, the benefits add up).
>
>> As mentioned, it's not an issue for the kernel proper due to
>> -mno-red-zone which we need for entirely unrelated reasons.
>
>It might help to have some special clobber syntax that says "this asm
>clobbers the red zone", so the compiler can arrange for the red zone to
>contain nothing during the asm (it already does the same for function
>calls, for example).
>
>How bad is it to do the fail-safe general solution here though?  I.e.,
>write actual assembler code:
>
># u16 getflags(void);
>getflags:
>	pushf
>	pop %ax
>	ret
>
>(or whatever the syntax is, my x86 is rusty).
>
>> Side note and kind of related: we do have this in the kernel:
>> 
>>   register unsigned long current_stack_pointer asm(_ASM_SP);
>>   #define ASM_CALL_CONSTRAINT "+r" (current_stack_pointer)
>> 
>> which *might* also solve the redzoning issue.
>
>The GCC documentation of inline asm says
>  Another restriction is that the clobber list should not contain the
>  stack pointer register.  This is because the compiler requires the
>  value of the stack pointer to be the same after an 'asm' statement as
>  it was on entry to the statement.  However, previous versions of GCC
>  did not enforce this rule and allowed the stack pointer to appear in
>  the list, with unclear semantics.  This behavior is deprecated and
>  listing the stack pointer may become an error in future versions of
>  GCC.
>
>> In the kernel we need it not because of redzoned stack use, but
>> because we need the stack frame to be set up properly or objtool
>> complains.
>
>If the kernel has special rules for the stack, it had better teach the
>compiler about this special ABI, or there will be tears eventually.  If
>the kernel requires only what the standard ABIs provide, it can trust
>the compiler to do that correctly, this is one of the core jobs of a
>compiler!
>
>
>Segher

It is extremely common for inline assembly to be written using push/pop or call sequences, and not just because of eflags. In the kernel redzone is (currently) not supported (with FRED exception handling it would be possible to support it as a kernel-wide compile-time option), but there needs to be a way to communicate this to the compiler.