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;
Date:   Wed, 15 Apr 2020 09:48:42 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Michael Matz <matz@suse.de>
Cc:     Jakub Jelinek <jakub@redhat.com>,
        Sergei Trofimovich <slyfox@gentoo.org>,
        linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Andy Lutomirski <luto@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>, x86@kernel.org
Subject: Re: [PATCH v2] x86: fix early boot crash on gcc-10
Message-ID: <20200415074842.GA31016@zn.tnic>
References: <20200326223501.GK11398@zn.tnic>
 <20200328084858.421444-1-slyfox@gentoo.org>
 <20200413163540.GD3772@zn.tnic>
 <alpine.LSU.2.21.2004141343370.11688@wotan.suse.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <alpine.LSU.2.21.2004141343370.11688@wotan.suse.de>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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    #
        subl    $28, %esp       #,
# arch/x86/kernel/smpboot.c:226:        cr4_init();
        call    cr4_init        #
	...

Those two pushes on entry are for callee-saved regs, AFAICT, because it
is going to use them later. But no frame setup.

> In that case I would speculate that having a frame pointer
> was (before the change above) only a side-effect of being
> compiled with -fstack-protector, which got now disabled.

Looks like it. And that's 32-bit. 64-bit variant of this looks more like it:

        .text
        .p2align 4
        .type   start_secondary, @function
start_secondary:
        pushq   %rbp    #
        subq    $8, %rsp        #,

although it doesn't save %rsp on the stack. I think it leaves space on
the stack for the canary but I'm not sure.

> But I was under the impression that the upstream kernels build with
> -fno-omit-frame-pointer, so that sounds unexpected.

Yap, see above.

> But I have no better explanation at the moment. If the above
> speculation doesn't make you progress: preprocessed file and a note of
> what the immediate caller of trace_hardirqs_on is in the above case,
> please.

Ok, I'll find you on IRC to talk details.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette