LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Arvind Sankar <nivedita@alum.mit.edu>
To: Arvind Sankar <nivedita@alum.mit.edu>
Cc: Borislav Petkov <bp@alien8.de>,
	Peter Zijlstra <peterz@infradead.org>,
	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>,
	x86@kernel.org
Subject: Re: [PATCH] x86: fix early boot crash on gcc-10
Date: Mon, 16 Mar 2020 14:54:21 -0400
Message-ID: <20200316185418.GA372474@rani.riverdale.lan> (raw)
In-Reply-To: <20200316181957.GA348193@rani.riverdale.lan>

On Mon, Mar 16, 2020 at 02:20:00PM -0400, Arvind Sankar wrote:
> On Mon, Mar 16, 2020 at 06:54:50PM +0100, Borislav Petkov wrote:
> > On Mon, Mar 16, 2020 at 02:42:34PM +0100, Peter Zijlstra wrote:
> > > Right I know, I looked for it recently :/ But since this is new in 10
> > > and 10 isn't released yet, I figured someone can add the attribute
> > > before it does get released.
> > 
> > Yes, that would be a good solution.
> > 
> > I looked at what happens briefly after building gcc10 from git and IINM,
> > the function in question - start_secondary() - already gets the stack
> > canary asm glue added so it checks for a stack canary.
> > 
> > However, the stack canary value itself gets set later in that same
> > function:
> > 
> >         /* to prevent fake stack check failure in clock setup */
> >         boot_init_stack_canary();
> > 
> > so the asm glue which checks for it would need to reload the newly
> > computed canary value (it is 0 before we compute it and thus the
> > mismatch).
> > 
> > So having a way to state "do not add stack canary checking to this
> > particular function" would be optimal. And since you already have the
> > "stack_protect" function attribute I figure adding a "no_stack_protect"
> > one should be easy...
> > 
> > > > Or of course you could add noinline attribute to whatever got inlined
> > > > and contains some array or addressable variable that whatever
> > > > -fstack-protector* mode kernel uses triggers it.  With -fstack-protector-all
> > > > it would never work even in the past I believe.
> > > 
> > > I don't think the kernel supports -fstack-protector-all, but I could be
> > > mistaken.
> > 
> > The other thing I was thinking was to carve out only that function into
> > a separate compilation unit and disable stack protector only for it.
> > 
> > All IMHO of course.
> > 
> > Thx.
> > 
> > -- 
> > Regards/Gruss,
> >     Boris.
> > 
> > https://people.kernel.org/tglx/notes-about-netiquette
> 
> With STACKPROTECTOR_STRONG, gcc9 (at least gentoo's version, not sure if
> they have some patches that affect it) already adds stack canary into
> start_secondary. Not sure why it doesn't panic already with gcc9?
> 
> 00000000000008f0 <start_secondary>:
>      8f0:       53                      push   %rbx
>      8f1:       48 83 ec 10             sub    $0x10,%rsp
>      8f5:       65 48 8b 04 25 28 00    mov    %gs:0x28,%rax
>      8fc:       00 00
>      8fe:       48 89 44 24 08          mov    %rax,0x8(%rsp)
>      903:       31 c0                   xor    %eax,%eax
> ...
>      a2e:       e8 00 00 00 00          callq  a33 <start_secondary+0x143>
>                         a2f: R_X86_64_PLT32     cpu_startup_entry-0x4
>      a33:       48 8b 44 24 08          mov    0x8(%rsp),%rax
>      a38:       65 48 33 04 25 28 00    xor    %gs:0x28,%rax
>      a3f:       00 00 
>      a41:       75 12                   jne    a55 <start_secondary+0x165>
>      a43:       48 83 c4 10             add    $0x10,%rsp
>      a47:       5b                      pop    %rbx
>      a48:       c3                      retq   
>      a49:       0f 01 1d 00 00 00 00    lidt   0x0(%rip)        # a50 <start_secondary+0x160>
>                         a4c: R_X86_64_PC32      debug_idt_descr-0x4
>      a50:       e9 cb fe ff ff          jmpq   920 <start_secondary+0x30>
>      a55:       e8 00 00 00 00          callq  a5a <start_secondary+0x16a>
>                         a56: R_X86_64_PLT32     __stack_chk_fail-0x4

Wait a sec, this function calls cpu_startup_entry as the last thing it
does, which should never return, and hence the stack canary value should
never get checked.

How does the canary get checked with the gcc10 code?

boot_init_stack_canary depends on working if called from functions that
don't return. If that doesn't work with gcc10, we need to disable stack
protector for the other callpoints too -- start_kernel in init/main.c
and cpu_bringup_and_idle in arch/x86/xen/smp_pv.c.

/*
 * Initialize the stackprotector canary value.
 *
 * NOTE: this must only be called from functions that never return,
 * and it must always be inlined.
 */
static __always_inline void boot_init_stack_canary(void)

  reply index

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-14 16:44 Sergei Trofimovich
2020-03-16 13:04 ` Peter Zijlstra
2020-03-16 13:26   ` Jakub Jelinek
2020-03-16 13:42     ` Peter Zijlstra
2020-03-16 17:54       ` Borislav Petkov
2020-03-16 18:03         ` Jakub Jelinek
2020-03-17 14:36           ` Borislav Petkov
2020-03-17 14:39             ` Jakub Jelinek
2020-03-17 14:49               ` Borislav Petkov
2020-03-17 16:35                 ` David Laight
2020-03-25 13:31                 ` Borislav Petkov
2020-03-26 21:54                   ` Sergei Trofimovich
2020-03-26 22:35                     ` Borislav Petkov
2020-03-28  8:48                       ` [PATCH v2] " Sergei Trofimovich
2020-04-13 14:15                         ` [tip: x86/urgent] x86: Fix " tip-bot2 for Sergei Trofimovich
2020-04-13 16:35                         ` [PATCH v2] x86: fix " Borislav Petkov
2020-04-14 13:50                           ` Michael Matz
2020-04-15  7:48                             ` Borislav Petkov
2020-04-15 14:53                               ` Michael Matz
2020-04-15 22:19                                 ` Sergei Trofimovich
2020-04-17  7:57                                   ` Borislav Petkov
2020-04-17  8:07                                     ` Jakub Jelinek
2020-04-17  8:42                                       ` Borislav Petkov
2020-04-17  8:58                                         ` Jakub Jelinek
2020-04-17  9:09                                           ` Borislav Petkov
2020-04-17 18:15                                             ` Nick Desaulniers
2020-04-17 18:22                                               ` Nick Desaulniers
2020-04-17 19:06                                                 ` Jakub Jelinek
2020-04-17 19:49                                                   ` Nick Desaulniers
2020-04-17 19:53                                                     ` Nick Desaulniers
2020-04-20 14:04                                                     ` Michael Matz
2020-04-22 10:23                                                       ` Borislav Petkov
2020-04-22 11:40                                                         ` Peter Zijlstra
2020-04-22 13:49                                                           ` Borislav Petkov
2020-04-22 13:55                                                             ` Jakub Jelinek
2020-04-22 14:16                                                               ` Martin Liška
2020-04-22 15:06                                                                 ` Michael Matz
2020-04-22 16:53                                                                 ` Borislav Petkov
2020-04-22 17:02                                                                   ` Jakub Jelinek
2020-04-22 18:47                                                                   ` Nick Desaulniers
2020-04-22 18:55                                                         ` Nick Desaulniers
2020-04-22 19:21                                                           ` Borislav Petkov
2020-04-22 21:05                                                             ` Nick Desaulniers
2020-04-22 21:26                                                               ` Borislav Petkov
2020-04-22 22:57                                                                 ` Nick Desaulniers
2020-04-23 12:53                                                                   ` Borislav Petkov
2020-04-23 16:12                                                                     ` [PATCH] x86: Fix early boot crash on gcc-10, next try Borislav Petkov
2020-04-23 17:30                                                                       ` Borislav Petkov
2020-04-23 18:02                                                                         ` Nick Desaulniers
2020-04-23 18:27                                                                           ` Borislav Petkov
2020-04-27 11:37                                                                         ` [tip: x86/build] x86/build: Check whether the compiler is sane tip-bot2 for Borislav Petkov
2020-04-23 19:40                                                                       ` [PATCH] x86: Fix early boot crash on gcc-10, next try Kees Cook
2020-04-25  1:46                                                                       ` Arvind Sankar
2020-04-25  8:57                                                                         ` Borislav Petkov
2020-04-25 11:09                                                                           ` Jürgen Groß
2020-04-25 15:04                                                                           ` Arvind Sankar
2020-04-25 17:31                                                                             ` Borislav Petkov
2020-04-25 17:52                                                                               ` Borislav Petkov
2020-04-27 17:07                                                                                 ` David Laight
2020-04-25 18:37                                                                               ` Segher Boessenkool
2020-04-25 18:53                                                                                 ` Borislav Petkov
2020-04-25 19:15                                                                                   ` Segher Boessenkool
2020-04-25 22:17                                                                                     ` Borislav Petkov
2020-04-25 22:25                                                                                     ` Arvind Sankar
2020-04-17 10:38                                           ` [PATCH v2] x86: fix early boot crash on gcc-10 Peter Zijlstra
2020-04-18 13:12                                             ` David Laight
2020-04-17 10:41                                     ` Peter Zijlstra
2020-03-16 18:20         ` [PATCH] " Arvind Sankar
2020-03-16 18:54           ` Arvind Sankar [this message]
2020-03-16 19:53             ` Arvind Sankar
2020-03-16 20:08               ` Jakub Jelinek
2020-03-16 20:40                 ` Arvind Sankar
2020-03-16 22:12     ` Sergei Trofimovich
2020-03-17 11:46       ` Jakub Jelinek
2020-03-17 18:10         ` Sergei Trofimovich
2020-03-16 18:22 ` Arvind Sankar
2020-03-26 23:16 ` [PATCH v2] " Sergei Trofimovich
2020-04-27 11:37 ` [tip: x86/build] x86: Fix early boot crash on gcc-10, next try tip-bot2 for Borislav Petkov
2020-05-15 11:20 ` [tip: x86/urgent] x86: Fix early boot crash on gcc-10, third try tip-bot2 for Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200316185418.GA372474@rani.riverdale.lan \
    --to=nivedita@alum.mit.edu \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jakub@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=slyfox@gentoo.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git