linux-kernel - Re: [PATCH v2] x86: fix early boot crash on gcc-10

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]

Date:   Wed, 15 Apr 2020 14:53:45 +0000 (UTC)
From:   Michael Matz <matz@...e.de>
To:     Borislav Petkov <bp@...en8.de>
cc:     Jakub Jelinek <jakub@...hat.com>,
        Sergei Trofimovich <slyfox@...too.org>,
        linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>, x86@...nel.org
Subject: Re: [PATCH v2] x86: fix early boot crash on gcc-10

Hello,

On Wed, 15 Apr 2020, Borislav Petkov wrote:

> On Tue, Apr 14, 2020 at 01:50:29PM +0000, Michael Matz wrote:
> > So this part expects that the caller (!) of trace_hardirqs_on was compiled
> > with a frame pointer (in %ebp).
> 
> /me looks at the .s file...
> 
> options passed comment at the top has -fno-omit-frame-pointer
> 
> > Obviously that's not the case as you traced above. Is start_secondary
> > the immediate caller in the above case?
> 
> Yes, start_secondary() is the function which is marked as
> __attribute__((optimize("-fno-stack-protector"))) and it does:
> 
> # arch/x86/kernel/smpboot.c:264:        local_irq_enable();
>         call    trace_hardirqs_on       #
> 
> (the local_irq_enable() is a macro which has the call to
> trace_hardirqs_on().
> 
> > Look at it's disassembly.  If it doesn't have the usual push
> > %ebp/mov%esp,%ebp prologue it probably doesn't use a frame pointer.
> 
> Here's the preamble:
> 
>         .text
>         .p2align 4
>         .type   start_secondary, @function
> start_secondary:
>         pushl   %esi    #
>         pushl   %ebx    #

Right.  So meanwhile it became clear: the optimize function attribute 
doesn't work cumulative but rather replaces all cmdline args (the 
optimization ones, but that roughly translates to -fxxx options).  In this 
case an 'optimize("-fno-stack-protector")' also disables the crucial 
-fno-omit-frame-pointer, reverting to the compilers default, which, 
depending on version, is also to omit the frame pointer on 32bit.  You 
could fix that by adding ',-fno-omit-frame-pointer' to the attribute 
string.  But that quickly gets out of hand, considering all the options 
you carefully need to set in Makefiles to get the right behaviour.  (Note 
that e.g. the optimization level is reset to -O0 as well!).

(I'll admit that I was somewhat surprised by this behaviour, even though 
it makes sense in the abstract; resetting to a clean slate and 
everything).

I think in its current form the optimize attribute is not useful for the 
purposes you need, and you're better off to disable the stack protector 
for the whole compilation unit from the Makefile.

(That attribute is also documented as "not suitable in production code", 
so go figure ;-) )

I think it will be possible to make that attribute a bit more useful 
in the future, but for the time being I think you'll just want to live 
without it.


Ciao,
Michael.