From: Michael Ellerman <mpe@ellerman.id.au> To: Ard Biesheuvel <ardb@kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> Cc: "open list\:LINUX FOR POWERPC \(32-BIT AND 64-BIT\)" <linuxppc-dev@lists.ozlabs.org>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Paul Mackerras <paulus@samba.org>, Nick Desaulniers <ndesaulniers@google.com>, Arvind Sankar <nivedita@alum.mit.edu>, Randy Dunlap <rdunlap@infradead.org>, Josh Poimboeuf <jpoimboe@redhat.com>, Thomas Gleixner <tglx@linutronix.de>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Peter Zijlstra <peterz@infradead.org>, Geert Uytterhoeven <geert@linux-m68k.org>, Kees Cook <keescook@chromium.org> Subject: Re: [PATCH] powerpc: avoid broken GCC __attribute__((optimize)) Date: Thu, 29 Oct 2020 22:35:48 +1100 [thread overview] Message-ID: <875z6tw9gr.fsf@mpe.ellerman.id.au> (raw) In-Reply-To: <CAMj1kXHFS7BonvRaSYCn+1rTXKsT8qfQocRaYovj-BTNZw_qng@mail.gmail.com> Ard Biesheuvel <ardb@kernel.org> writes: > On Wed, 28 Oct 2020 at 09:04, Ard Biesheuvel <ardb@kernel.org> wrote: >> >> Commit 7053f80d9696 ("powerpc/64: Prevent stack protection in early boot") >> introduced a couple of uses of __attribute__((optimize)) with function >> scope, to disable the stack protector in some early boot code. >> >> Unfortunately, and this is documented in the GCC man pages [0], overriding >> function attributes for optimization is broken, and is only supported for >> debug scenarios, not for production: the problem appears to be that >> setting GCC -f flags using this method will cause it to forget about some >> or all other optimization settings that have been applied. >> >> So the only safe way to disable the stack protector is to disable it for >> the entire source file. >> >> [0] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html >> >> Cc: Michael Ellerman <mpe@ellerman.id.au> >> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Cc: Paul Mackerras <paulus@samba.org> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> Cc: Arvind Sankar <nivedita@alum.mit.edu> >> Cc: Randy Dunlap <rdunlap@infradead.org> >> Cc: Josh Poimboeuf <jpoimboe@redhat.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Alexei Starovoitov <ast@kernel.org> >> Cc: Daniel Borkmann <daniel@iogearbox.net> >> Cc: Peter Zijlstra (Intel) <peterz@infradead.org> >> Cc: Geert Uytterhoeven <geert@linux-m68k.org> >> Cc: Kees Cook <keescook@chromium.org> >> Fixes: 7053f80d9696 ("powerpc/64: Prevent stack protection in early boot") >> Signed-off-by: Ard Biesheuvel <ardb@kernel.org> >> --- >> Related discussion here: >> https://lore.kernel.org/lkml/CAMuHMdUg0WJHEcq6to0-eODpXPOywLot6UD2=GFHpzoj_hCoBQ@mail.gmail.com/ >> >> TL;DR using __attribute__((optimize("-fno-gcse"))) in the BPF interpreter >> causes the compiler to forget about -fno-asynchronous-unwind-tables passed >> on the command line, resulting in unexpected .eh_frame sections in vmlinux. >> >> arch/powerpc/kernel/Makefile | 3 +++ >> arch/powerpc/kernel/paca.c | 2 +- >> arch/powerpc/kernel/setup.h | 6 ------ >> arch/powerpc/kernel/setup_64.c | 2 +- >> 4 files changed, 5 insertions(+), 8 deletions(-) Thanks for the patch. > FYI i was notified by one of the robots that I missed one occurrence > of __nostackprotector in arch/powerpc/kernel/paca.c > > Let me know if I need to resend. That's fine I'll fix it up when applying. With the existing code, with STACKPROTECTOR_STRONG=y, I see two functions in setup_64.c that are triggering stack protection. One is __init, and the other takes no parameters and is not easily reachable from userspace, so I don't think losing the stack canary on either of those is a concern. I don't see anything in paca.c triggering stack protection. I don't think there's any evidence this is causing a bug for us, so I'll plan to put this in next for v5.11. cheers
WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au> To: Ard Biesheuvel <ardb@kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> Cc: Kees Cook <keescook@chromium.org>, Daniel Borkmann <daniel@iogearbox.net>, Peter Zijlstra <peterz@infradead.org>, Randy Dunlap <rdunlap@infradead.org>, Nick Desaulniers <ndesaulniers@google.com>, Alexei Starovoitov <ast@kernel.org>, Arvind Sankar <nivedita@alum.mit.edu>, Paul Mackerras <paulus@samba.org>, Josh Poimboeuf <jpoimboe@redhat.com>, Geert Uytterhoeven <geert@linux-m68k.org>, Thomas Gleixner <tglx@linutronix.de>, "open list:LINUX FOR POWERPC \(32-BIT AND 64-BIT\)" <linuxppc-dev@lists.ozlabs.org> Subject: Re: [PATCH] powerpc: avoid broken GCC __attribute__((optimize)) Date: Thu, 29 Oct 2020 22:35:48 +1100 [thread overview] Message-ID: <875z6tw9gr.fsf@mpe.ellerman.id.au> (raw) In-Reply-To: <CAMj1kXHFS7BonvRaSYCn+1rTXKsT8qfQocRaYovj-BTNZw_qng@mail.gmail.com> Ard Biesheuvel <ardb@kernel.org> writes: > On Wed, 28 Oct 2020 at 09:04, Ard Biesheuvel <ardb@kernel.org> wrote: >> >> Commit 7053f80d9696 ("powerpc/64: Prevent stack protection in early boot") >> introduced a couple of uses of __attribute__((optimize)) with function >> scope, to disable the stack protector in some early boot code. >> >> Unfortunately, and this is documented in the GCC man pages [0], overriding >> function attributes for optimization is broken, and is only supported for >> debug scenarios, not for production: the problem appears to be that >> setting GCC -f flags using this method will cause it to forget about some >> or all other optimization settings that have been applied. >> >> So the only safe way to disable the stack protector is to disable it for >> the entire source file. >> >> [0] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html >> >> Cc: Michael Ellerman <mpe@ellerman.id.au> >> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Cc: Paul Mackerras <paulus@samba.org> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> Cc: Arvind Sankar <nivedita@alum.mit.edu> >> Cc: Randy Dunlap <rdunlap@infradead.org> >> Cc: Josh Poimboeuf <jpoimboe@redhat.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Alexei Starovoitov <ast@kernel.org> >> Cc: Daniel Borkmann <daniel@iogearbox.net> >> Cc: Peter Zijlstra (Intel) <peterz@infradead.org> >> Cc: Geert Uytterhoeven <geert@linux-m68k.org> >> Cc: Kees Cook <keescook@chromium.org> >> Fixes: 7053f80d9696 ("powerpc/64: Prevent stack protection in early boot") >> Signed-off-by: Ard Biesheuvel <ardb@kernel.org> >> --- >> Related discussion here: >> https://lore.kernel.org/lkml/CAMuHMdUg0WJHEcq6to0-eODpXPOywLot6UD2=GFHpzoj_hCoBQ@mail.gmail.com/ >> >> TL;DR using __attribute__((optimize("-fno-gcse"))) in the BPF interpreter >> causes the compiler to forget about -fno-asynchronous-unwind-tables passed >> on the command line, resulting in unexpected .eh_frame sections in vmlinux. >> >> arch/powerpc/kernel/Makefile | 3 +++ >> arch/powerpc/kernel/paca.c | 2 +- >> arch/powerpc/kernel/setup.h | 6 ------ >> arch/powerpc/kernel/setup_64.c | 2 +- >> 4 files changed, 5 insertions(+), 8 deletions(-) Thanks for the patch. > FYI i was notified by one of the robots that I missed one occurrence > of __nostackprotector in arch/powerpc/kernel/paca.c > > Let me know if I need to resend. That's fine I'll fix it up when applying. With the existing code, with STACKPROTECTOR_STRONG=y, I see two functions in setup_64.c that are triggering stack protection. One is __init, and the other takes no parameters and is not easily reachable from userspace, so I don't think losing the stack canary on either of those is a concern. I don't see anything in paca.c triggering stack protection. I don't think there's any evidence this is causing a bug for us, so I'll plan to put this in next for v5.11. cheers
next prev parent reply other threads:[~2020-10-29 11:37 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-28 8:04 [PATCH] powerpc: avoid broken GCC __attribute__((optimize)) Ard Biesheuvel 2020-10-28 8:04 ` Ard Biesheuvel 2020-10-28 8:19 ` Test Results: " snowpatch 2020-10-28 8:29 ` [PATCH] " Ard Biesheuvel 2020-10-28 8:29 ` Ard Biesheuvel 2020-10-29 11:35 ` Michael Ellerman [this message] 2020-10-29 11:35 ` Michael Ellerman 2020-10-28 10:08 ` kernel test robot 2020-11-25 11:57 ` Michael Ellerman 2020-11-25 11:57 ` Michael Ellerman
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=875z6tw9gr.fsf@mpe.ellerman.id.au \ --to=mpe@ellerman.id.au \ --cc=ardb@kernel.org \ --cc=ast@kernel.org \ --cc=benh@kernel.crashing.org \ --cc=daniel@iogearbox.net \ --cc=geert@linux-m68k.org \ --cc=jpoimboe@redhat.com \ --cc=keescook@chromium.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=ndesaulniers@google.com \ --cc=nivedita@alum.mit.edu \ --cc=paulus@samba.org \ --cc=peterz@infradead.org \ --cc=rdunlap@infradead.org \ --cc=tglx@linutronix.de \ /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.