From: Arvind Sankar <nivedita@alum.mit.edu> To: Borislav Petkov <bp@alien8.de> Cc: "Arvind Sankar" <nivedita@alum.mit.edu>, "Nick Desaulniers" <ndesaulniers@google.com>, "Michael Matz" <matz@suse.de>, "Jakub Jelinek" <jakub@redhat.com>, "Sergei Trofimovich" <slyfox@gentoo.org>, LKML <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, clang-built-linux <clang-built-linux@googlegroups.com>, "Kees Cook" <keescook@chromium.org>, "Martin Liška" <mliska@suse.cz>, "Frédéric Pierret (fepitre)" <frederic.pierret@qubes-os.org>, boris.ostrovsky@oracle.com, jgross@suse.com, linuxppc-dev@lists.ozlabs.org, "Michael Ellerman" <mpe@ellerman.id.au>, "Benjamin Herrenschmidt" <benh@kernel.crashing.org>, "Paul Mackerras" <paulus@samba.org> Subject: Re: [PATCH] x86: Fix early boot crash on gcc-10, next try Date: Sat, 25 Apr 2020 11:04:40 -0400 [thread overview] Message-ID: <20200425150440.GA470719@rani.riverdale.lan> (raw) In-Reply-To: <20200425085759.GA24294@zn.tnic> On Sat, Apr 25, 2020 at 10:57:59AM +0200, Borislav Petkov wrote: > On Fri, Apr 24, 2020 at 09:46:57PM -0400, Arvind Sankar wrote: > > The comment above boot_init_stack_canary's definition should be updated > > to note that it needs to be called from a function that, in addition to > > not returning, either has stackprotector disabled or avoids ending in a > > tail call. > > How's that? > > diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h > index 91e29b6a86a5..237a54f60d6b 100644 > --- a/arch/x86/include/asm/stackprotector.h > +++ b/arch/x86/include/asm/stackprotector.h > @@ -55,8 +55,12 @@ > /* > * Initialize the stackprotector canary value. > * > - * NOTE: this must only be called from functions that never return, > - * and it must always be inlined. > + * NOTE: this must only be called from functions that never return, it must > + * always be inlined and it should be called from a compilation unit for > + * which stack protector is disabled. > + * > + * Alternatively, the caller should not end with a function call which gets > + * tail-call optimized as that would lead to checking a modified canary value. > */ > static __always_inline void boot_init_stack_canary(void) > { I'd put the clause about stack protector being disabled and the tail-call one together, to make clear that you still need the never return and always inline bits. Also, this function is implemented by multiple arch's and they all have similar comments -- might be better to consolidate the comment in the generic (dummy) one in include/linux/stackprotector.h laying out the restrictions that arch implementations should follow? > > > There are also other calls that likely need to be fixed as well -- in > > init/main.c, arch/x86/xen/smp_pv.c, and there is a powerpc version of > > start_secondary in arch/powerpc/kernel/smp.c which may also be affected. > > Yes, there was an attempt to fix former: > > https://lkml.kernel.org/r/20200413123535.10884-1-frederic.pierret@qubes-os.org There's also the one in init/main.c which is used by multiple architectures. On x86 at least, the call to arch_call_rest_init at the end of start_kernel does not get tail-call optimized by gcc-10, but I don't see anything that actually prevents that from happening. We should add the asm("") there as well I think, unless the compiler guys see something about this function that will always prevent the optimization? Cc'ing PPC list for powerpc start_secondary.
WARNING: multiple messages have this Message-ID (diff)
From: Arvind Sankar <nivedita@alum.mit.edu> To: Borislav Petkov <bp@alien8.de> Cc: "Peter Zijlstra" <peterz@infradead.org>, "Arvind Sankar" <nivedita@alum.mit.edu>, "Paul Mackerras" <paulus@samba.org>, "H. Peter Anvin" <hpa@zytor.com>, boris.ostrovsky@oracle.com, x86@kernel.org, clang-built-linux <clang-built-linux@googlegroups.com>, "Ingo Molnar" <mingo@redhat.com>, "Frédéric Pierret (fepitre)" <frederic.pierret@qubes-os.org>, "Jakub Jelinek" <jakub@redhat.com>, "Kees Cook" <keescook@chromium.org>, "Michael Matz" <matz@suse.de>, "Sergei Trofimovich" <slyfox@gentoo.org>, "Andy Lutomirski" <luto@kernel.org>, "Thomas Gleixner" <tglx@linutronix.de>, jgross@suse.com, "Martin Liška" <mliska@suse.cz>, "Nick Desaulniers" <ndesaulniers@google.com>, LKML <linux-kernel@vger.kernel.org>, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH] x86: Fix early boot crash on gcc-10, next try Date: Sat, 25 Apr 2020 11:04:40 -0400 [thread overview] Message-ID: <20200425150440.GA470719@rani.riverdale.lan> (raw) In-Reply-To: <20200425085759.GA24294@zn.tnic> On Sat, Apr 25, 2020 at 10:57:59AM +0200, Borislav Petkov wrote: > On Fri, Apr 24, 2020 at 09:46:57PM -0400, Arvind Sankar wrote: > > The comment above boot_init_stack_canary's definition should be updated > > to note that it needs to be called from a function that, in addition to > > not returning, either has stackprotector disabled or avoids ending in a > > tail call. > > How's that? > > diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h > index 91e29b6a86a5..237a54f60d6b 100644 > --- a/arch/x86/include/asm/stackprotector.h > +++ b/arch/x86/include/asm/stackprotector.h > @@ -55,8 +55,12 @@ > /* > * Initialize the stackprotector canary value. > * > - * NOTE: this must only be called from functions that never return, > - * and it must always be inlined. > + * NOTE: this must only be called from functions that never return, it must > + * always be inlined and it should be called from a compilation unit for > + * which stack protector is disabled. > + * > + * Alternatively, the caller should not end with a function call which gets > + * tail-call optimized as that would lead to checking a modified canary value. > */ > static __always_inline void boot_init_stack_canary(void) > { I'd put the clause about stack protector being disabled and the tail-call one together, to make clear that you still need the never return and always inline bits. Also, this function is implemented by multiple arch's and they all have similar comments -- might be better to consolidate the comment in the generic (dummy) one in include/linux/stackprotector.h laying out the restrictions that arch implementations should follow? > > > There are also other calls that likely need to be fixed as well -- in > > init/main.c, arch/x86/xen/smp_pv.c, and there is a powerpc version of > > start_secondary in arch/powerpc/kernel/smp.c which may also be affected. > > Yes, there was an attempt to fix former: > > https://lkml.kernel.org/r/20200413123535.10884-1-frederic.pierret@qubes-os.org There's also the one in init/main.c which is used by multiple architectures. On x86 at least, the call to arch_call_rest_init at the end of start_kernel does not get tail-call optimized by gcc-10, but I don't see anything that actually prevents that from happening. We should add the asm("") there as well I think, unless the compiler guys see something about this function that will always prevent the optimization? Cc'ing PPC list for powerpc start_secondary.
next prev parent reply other threads:[~2020-04-25 15:04 UTC|newest] Thread overview: 89+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-14 16:44 [PATCH] x86: fix early boot crash on gcc-10 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 [this message] 2020-04-25 15:04 ` Arvind Sankar 2020-04-25 17:31 ` Borislav Petkov 2020-04-25 17:31 ` Borislav Petkov 2020-04-25 17:52 ` Borislav Petkov 2020-04-25 17:52 ` Borislav Petkov 2020-04-27 17:07 ` David Laight 2020-04-27 17:07 ` David Laight 2020-04-25 18:37 ` Segher Boessenkool 2020-04-25 18:37 ` Segher Boessenkool 2020-04-25 18:53 ` Borislav Petkov 2020-04-25 18:53 ` Borislav Petkov 2020-04-25 19:15 ` Segher Boessenkool 2020-04-25 19:15 ` Segher Boessenkool 2020-04-25 22:17 ` Borislav Petkov 2020-04-25 22:17 ` Borislav Petkov 2020-04-25 22:25 ` Arvind Sankar 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 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 2020-05-19 11:49 ` Sasha Levin
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=20200425150440.GA470719@rani.riverdale.lan \ --to=nivedita@alum.mit.edu \ --cc=benh@kernel.crashing.org \ --cc=boris.ostrovsky@oracle.com \ --cc=bp@alien8.de \ --cc=clang-built-linux@googlegroups.com \ --cc=frederic.pierret@qubes-os.org \ --cc=hpa@zytor.com \ --cc=jakub@redhat.com \ --cc=jgross@suse.com \ --cc=keescook@chromium.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=luto@kernel.org \ --cc=matz@suse.de \ --cc=mingo@redhat.com \ --cc=mliska@suse.cz \ --cc=mpe@ellerman.id.au \ --cc=ndesaulniers@google.com \ --cc=paulus@samba.org \ --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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.