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/[email protected]/
Tested-by: Nathan Chancellor <[email protected]>
Acked-by: Michael Ellerman <[email protected]> (powerpc)
Acked-by: Miguel Ojeda <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
arch/powerpc/kernel/smp.c | 1 +
include/linux/compiler_attributes.h | 12 ++++++++++++
init/main.c | 3 ++-
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index f62e5e651bcd..48acae0da034 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1603,6 +1603,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 e659cb6fded3..84864767a56a 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -255,6 +255,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.
*
diff --git a/init/main.c b/init/main.c
index 5d6365510173..1265c8d11052 100644
--- a/init/main.c
+++ b/init/main.c
@@ -941,7 +941,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;
--
2.40.0.634.g4ca3ef3211-goog
The following commit has been merged into the objtool/core branch of tip:
Commit-ID: 514ca14ed5444b911de59ed3381dfd195d99fe4b
Gitweb: https://git.kernel.org/tip/514ca14ed5444b911de59ed3381dfd195d99fe4b
Author: [email protected] <[email protected]>
AuthorDate: Mon, 17 Apr 2023 15:00:05 -07:00
Committer: Josh Poimboeuf <[email protected]>
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/[email protected]/
Tested-by: Nathan Chancellor <[email protected]>
Acked-by: Michael Ellerman <[email protected]> (powerpc)
Acked-by: Miguel Ojeda <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Josh Poimboeuf <[email protected]>
Signed-off-by: [email protected] <[email protected]>
---
arch/powerpc/kernel/smp.c | 1 +
include/linux/compiler_attributes.h | 12 ++++++++++++
init/main.c | 3 ++-
3 files changed, 15 insertions(+), 1 deletion(-)
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;
On Thu, May 18, 2023 at 11:08:03AM -0000, tip-bot2 for [email protected] 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: [email protected] <[email protected]>
> AuthorDate: Mon, 17 Apr 2023 15:00:05 -07:00
> Committer: Josh Poimboeuf <[email protected]>
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
On Fri, May 19, 2023 at 10:11 AM David Vernet <[email protected]> wrote:
>
> On Thu, May 18, 2023 at 11:08:03AM -0000, tip-bot2 for [email protected] 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: [email protected] <[email protected]>
> > AuthorDate: Mon, 17 Apr 2023 15:00:05 -07:00
> > Committer: Josh Poimboeuf <[email protected]>
>
> 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
--
Thanks,
~Nick Desaulniers
On Fri, May 19, 2023 at 10:18:40AM -0700, Nick Desaulniers wrote:
> On Fri, May 19, 2023 at 10:11 AM David Vernet <[email protected]> wrote:
> >
> > On Thu, May 18, 2023 at 11:08:03AM -0000, tip-bot2 for [email protected] 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: [email protected] <[email protected]>
> > > AuthorDate: Mon, 17 Apr 2023 15:00:05 -07:00
> > > Committer: Josh Poimboeuf <[email protected]>
> >
> > 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/[email protected]/
> 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