diff mbox series

[tip:,objtool/core] start_kernel: Add __no_stack_protector function attribute

Message ID 168440808395.404.16801982965854981978.tip-bot2@tip-bot2
State New
Headers
Series [tip:,objtool/core] start_kernel: Add __no_stack_protector function attribute |

Commit Message

tip-bot2 for Thomas Gleixner May 18, 2023, 11:08 a.m. UTC 
  The following commit has been merged into the objtool/core branch of tip:

Commit-ID:     514ca14ed5444b911de59ed3381dfd195d99fe4b
Gitweb:        https://git.kernel.org/tip/514ca14ed5444b911de59ed3381dfd195d99fe4b
Author:        ndesaulniers@google.com <ndesaulniers@google.com>
AuthorDate:    Mon, 17 Apr 2023 15:00:05 -07:00
Committer:     Josh Poimboeuf <jpoimboe@kernel.org>
CommitterDate: Tue, 16 May 2023 06:28:15 -07:00

start_kernel: Add __no_stack_protector function attribute

Back during the discussion of
commit a9a3ed1eff36 ("x86: Fix early boot crash on gcc-10, third try")
we discussed the need for a function attribute to control the omission
of stack protectors on a per-function basis; at the time Clang had
support for no_stack_protector but GCC did not. This was fixed in
gcc-11. Now that the function attribute is available, let's start using
it.

Callers of boot_init_stack_canary need to use this function attribute
unless they're compiled with -fno-stack-protector, otherwise the canary
stored in the stack slot of the caller will differ upon the call to
boot_init_stack_canary. This will lead to a call to __stack_chk_fail()
then panic.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94722
Link: https://lore.kernel.org/all/20200316130414.GC12561@hirez.programming.kicks-ass.net/
Tested-by: Nathan Chancellor <nathan@kernel.org>
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
Acked-by: Miguel Ojeda <ojeda@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Link: https://lore.kernel.org/r/20230412-no_stackp-v2-1-116f9fe4bbe7@google.com
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Signed-off-by: ndesaulniers@google.com <ndesaulniers@google.com>
---
 arch/powerpc/kernel/smp.c           |  1 +
 include/linux/compiler_attributes.h | 12 ++++++++++++
 init/main.c                         |  3 ++-
 3 files changed, 15 insertions(+), 1 deletion(-)

Comments

David Vernet May 19, 2023, 5:11 p.m. UTC | #1 
On Thu, May 18, 2023 at 11:08:03AM -0000, tip-bot2 for ndesaulniers@google.com wrote:
> The following commit has been merged into the objtool/core branch of tip:
> 
> Commit-ID:     514ca14ed5444b911de59ed3381dfd195d99fe4b
> Gitweb:        https://git.kernel.org/tip/514ca14ed5444b911de59ed3381dfd195d99fe4b
> Author:        ndesaulniers@google.com <ndesaulniers@google.com>
> AuthorDate:    Mon, 17 Apr 2023 15:00:05 -07:00
> Committer:     Josh Poimboeuf <jpoimboe@kernel.org>

Hi Nick, Josh, Peter,

Do you have an ETA for when this will make its way to Linus' tree?
clang-17 built kernels have failed to boot since [0], so it would be
nice to get this in sooner rather than later if possible.

[0]: https://lore.kernel.org/all/7194ed8a989a85b98d92e62df660f4a90435a723.1681342859.git.jpoimboe@kernel.org/

Thanks,
David
Nick Desaulniers May 19, 2023, 5:18 p.m. UTC | #2 
On Fri, May 19, 2023 at 10:11 AM David Vernet <void@manifault.com> wrote:
>
> On Thu, May 18, 2023 at 11:08:03AM -0000, tip-bot2 for ndesaulniers@google.com wrote:
> > The following commit has been merged into the objtool/core branch of tip:
> >
> > Commit-ID:     514ca14ed5444b911de59ed3381dfd195d99fe4b
> > Gitweb:        https://git.kernel.org/tip/514ca14ed5444b911de59ed3381dfd195d99fe4b
> > Author:        ndesaulniers@google.com <ndesaulniers@google.com>
> > AuthorDate:    Mon, 17 Apr 2023 15:00:05 -07:00
> > Committer:     Josh Poimboeuf <jpoimboe@kernel.org>
>
> Hi Nick, Josh, Peter,
>
> Do you have an ETA for when this will make its way to Linus' tree?
> clang-17 built kernels have failed to boot since [0], so it would be
> nice to get this in sooner rather than later if possible.

David,
Can you confirm that your version of clang-17 is updated? clang-17 is
unreleased; ToT will become clang-17.

https://reviews.llvm.org/rGfc4494dffa5422b2be5442c235554e76bed79c8a
should have fixed any boot failures related to stack protectors.  That
is to say that Josh's series is irrelevant to anyone using either an
existing release of clang, or something closer to ToT than April 13.

LLVM commit fc4494dffa54 ("[StackProtector] don't check stack
protector before calling nounwind functions")
landed April 13, so please check that your build of clang-17 is after that date.

Either way, thanks for testing with clang, and the report. You can
always file a bug at our issue tracker:
https://github.com/ClangBuiltLinux/linux/issues or see our page for
more ways to get in touch:
https://clangbuiltlinux.github.io/
We're very active on our mailing list, and on IRC.

>
> [0]: https://lore.kernel.org/all/7194ed8a989a85b98d92e62df660f4a90435a723.1681342859.git.jpoimboe@kernel.org/
>
> Thanks,
> David
David Vernet May 19, 2023, 5:42 p.m. UTC | #3 
On Fri, May 19, 2023 at 10:18:40AM -0700, Nick Desaulniers wrote:
> On Fri, May 19, 2023 at 10:11 AM David Vernet <void@manifault.com> wrote:
> >
> > On Thu, May 18, 2023 at 11:08:03AM -0000, tip-bot2 for ndesaulniers@google.com wrote:
> > > The following commit has been merged into the objtool/core branch of tip:
> > >
> > > Commit-ID:     514ca14ed5444b911de59ed3381dfd195d99fe4b
> > > Gitweb:        https://git.kernel.org/tip/514ca14ed5444b911de59ed3381dfd195d99fe4b
> > > Author:        ndesaulniers@google.com <ndesaulniers@google.com>
> > > AuthorDate:    Mon, 17 Apr 2023 15:00:05 -07:00
> > > Committer:     Josh Poimboeuf <jpoimboe@kernel.org>
> >
> > Hi Nick, Josh, Peter,
> >
> > Do you have an ETA for when this will make its way to Linus' tree?
> > clang-17 built kernels have failed to boot since [0], so it would be
> > nice to get this in sooner rather than later if possible.
> 
> David,
> Can you confirm that your version of clang-17 is updated? clang-17 is
> unreleased; ToT will become clang-17.
>
> https://reviews.llvm.org/rGfc4494dffa5422b2be5442c235554e76bed79c8a
> should have fixed any boot failures related to stack protectors.  That
> is to say that Josh's series is irrelevant to anyone using either an
> existing release of clang, or something closer to ToT than April 13.

Thanks for the quick reply, Nick. The latest clang-17 does indeed fix
the issue. Apologies for not trying that first -- I was using the only
tagged verson of clang-17 (which admittedly is not a released version),
and figured it wasn't a compiler bug given that the assembly looked
sane, compilers are allowed to do all sorts of interesting things with
__noreturn, and that [1] removes -fstack-protector from start_kernel()
altogether.

[1]: https://lore.kernel.org/lkml/20230412-no_stackp-v1-1-46a69b507a4b@google.com/

> LLVM commit fc4494dffa54 ("[StackProtector] don't check stack
> protector before calling nounwind functions")
> landed April 13, so please check that your build of clang-17 is after that date.
> 
> Either way, thanks for testing with clang, and the report. You can
> always file a bug at our issue tracker:
> https://github.com/ClangBuiltLinux/linux/issues or see our page for
> more ways to get in touch:
> https://clangbuiltlinux.github.io/
> We're very active on our mailing list, and on IRC.

Ack, thanks for letting me know for next time.

- David
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 265801a..6903a72 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1605,6 +1605,7 @@  static void add_cpu_to_masks(int cpu)
 }
 
 /* Activate a secondary processor. */
+__no_stack_protector
 void start_secondary(void *unused)
 {
 	unsigned int cpu = raw_smp_processor_id();
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index e659cb6..8486476 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -256,6 +256,18 @@ 
 #define __noreturn                      __attribute__((__noreturn__))
 
 /*
+ * Optional: only supported since GCC >= 11.1, clang >= 7.0.
+ *
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-no_005fstack_005fprotector-function-attribute
+ *   clang: https://clang.llvm.org/docs/AttributeReference.html#no-stack-protector-safebuffers
+ */
+#if __has_attribute(__no_stack_protector__)
+# define __no_stack_protector		__attribute__((__no_stack_protector__))
+#else
+# define __no_stack_protector
+#endif
+
+/*
  * Optional: not supported by gcc.
  *
  * clang: https://clang.llvm.org/docs/AttributeReference.html#overloadable
diff --git a/init/main.c b/init/main.c
index af50044..c445c1f 100644
--- a/init/main.c
+++ b/init/main.c
@@ -877,7 +877,8 @@  static void __init print_unknown_bootoptions(void)
 	memblock_free(unknown_options, len);
 }
 
-asmlinkage __visible void __init __no_sanitize_address __noreturn start_kernel(void)
+asmlinkage __visible __init __no_sanitize_address __noreturn __no_stack_protector
+void start_kernel(void)
 {
 	char *command_line;
 	char *after_dashes;