2023-04-17 22:04:15

by Nick Desaulniers

[permalink] [raw]
Subject: [PATCH v2 0/2] start_kernel: omit stack canary

A security research paper was recently published detailing Catch Handler
Oriented Programming (CHOP) attacks.
https://download.vusec.net/papers/chop_ndss23.pdf
The TL;DR being that C++ structured exception handling runtimes are
attractive gadgets for Jump Oriented Programming (JOP) attacks.

In response to this, a mitigation was developed under embargo in
clang-16 to check the stack canary before calling noreturn functions.
https://bugs.chromium.org/p/llvm/issues/detail?id=30

This started causing boot failures in Android kernel trees downstream of
stable linux-4.14.y that had proto-LTO support, as reported by Nathan
Chancellor.
https://github.com/ClangBuiltLinux/linux/issues/1815

Josh Poimboeuf recently sent a series to explicitly annotate more
functions as noreturn. Nathan noticed the series, and tested it finding
that it now caused boot failures with clang-16+ on mainline (raising the
visibility and urgency of the issue).
https://lore.kernel.org/[email protected]/
V2 of this series is rebased on tip/objtool/core @
88b478ee5c7b080b70c68d6e9b3da6c2b518ceb0 now that that series has been
picked up.

Once the embargo was lifted, I asked questions such as "what does C++
structured exception handling have to do with C code" and "surely GCC
didn't ship the same mitigation for C code (narrator: 'They did not:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=a25982ada523689c8745d7fb4b1b93c8f5dab2e7')?"

I now have a patch out for LLVM to undo this mess (or at least limit it
to C++ functions that may throw, similar to GCC's mitigation); it hasn't
landed yet but we're close to consensus and I expect it to land
imminently.
https://reviews.llvm.org/D147975

Remember this thread? (Pepperidge farms remembers...)
https://lore.kernel.org/all/[email protected]/

That reminded me that years ago we discussed a function attribute for
no_stack_protector.
https://lore.kernel.org/all/[email protected]/

GCC didn't have one at the time, it now does. In addition to the LLVM
fix, I'd like to introduce this in the kernel so that we might start
using it in additional places:
* https://lore.kernel.org/linux-pm/[email protected]/
* https://lore.kernel.org/lkml/[email protected]/
And eventually remove the final macro expansion site of
prevent_tail_call_optimization.

With the LLVM fix, this series isn't required, but I'd like to start
paving the way to use these function attributes since I think they are a
sweet spot in terms of granularity (as opposed to trying to move
start_kernel to its own TU compiled with -fno-stack-protector).

Signed-off-by: Nick Desaulniers <[email protected]>
---
Changes in v2:
- Rebase to avoid conflicts with Josh's changes.
- Fix comment style as per Peter.
- Pick up tags.
- Link to v1: https://lore.kernel.org/r/[email protected]
(sorry for the spam with v2, mrincon is helping me get kinks worked out
with b4 and our corporate mailer)

---
Nick Desaulniers (2):
start_kernel: add no_stack_protector fn attr
start_kernel: omit prevent_tail_call_optimization for newer toolchains

arch/powerpc/kernel/smp.c | 1 +
include/linux/compiler_attributes.h | 12 ++++++++++++
init/main.c | 9 ++++++++-
3 files changed, 21 insertions(+), 1 deletion(-)
---
base-commit: 88b478ee5c7b080b70c68d6e9b3da6c2b518ceb0
change-id: 20230412-no_stackp-a98168a2bb0a

Best regards,
--
Nick Desaulniers <[email protected]>


2023-04-17 22:04:16

by Nick Desaulniers

[permalink] [raw]
Subject: [PATCH v2 2/2] start_kernel: omit prevent_tail_call_optimization for newer toolchains

prevent_tail_call_optimization was added in
commit a9a3ed1eff36 ("x86: Fix early boot crash on gcc-10, third try")
to work around stack canaries getting inserted into functions that would
initialize the stack canary in the first place.

Now that we have no_stack_protector function attribute (gcc-11+,
clang-7+) and use it on start_kernel, remove the call to
prevent_tail_call_optimization such that we may one day remove it
outright.

Reviewed-by: Nathan Chancellor <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
init/main.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/init/main.c b/init/main.c
index 1265c8d11052..c6eef497c8c9 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1152,7 +1152,13 @@ void start_kernel(void)
/* Do the rest non-__init'ed, we're now alive */
arch_call_rest_init();

+ /*
+ * Avoid stack canaries in callers of boot_init_stack_canary for gcc-10
+ * and older.
+ */
+#if !__has_attribute(__no_stack_protector__)
prevent_tail_call_optimization();
+#endif
}

/* Call all constructor functions linked into the kernel. */

--
2.40.0.634.g4ca3ef3211-goog

2023-04-18 21:47:34

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] start_kernel: omit stack canary

On Mon, Apr 17, 2023 at 03:00:04PM -0700, [email protected] wrote:
> ---
> Changes in v2:
> - Rebase to avoid conflicts with Josh's changes.
> - Fix comment style as per Peter.
> - Pick up tags.
> - Link to v1: https://lore.kernel.org/r/[email protected]
> (sorry for the spam with v2, mrincon is helping me get kinks worked out
> with b4 and our corporate mailer)

Acked-by: Josh Poimboeuf <[email protected]>

--
Josh

Subject: [tip: objtool/core] start_kernel: Omit prevent_tail_call_optimization() for newer toolchains

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: dc1d05536f44cee16e46e86316e6718b2c0d8872
Gitweb: https://git.kernel.org/tip/dc1d05536f44cee16e46e86316e6718b2c0d8872
Author: [email protected] <[email protected]>
AuthorDate: Mon, 17 Apr 2023 15:00:06 -07:00
Committer: Josh Poimboeuf <[email protected]>
CommitterDate: Tue, 16 May 2023 06:28:24 -07:00

start_kernel: Omit prevent_tail_call_optimization() for newer toolchains

prevent_tail_call_optimization() was added in
commit a9a3ed1eff36 ("x86: Fix early boot crash on gcc-10, third try")
to work around stack canaries getting inserted into functions that would
initialize the stack canary in the first place.

Now that we have no_stack_protector function attribute (gcc-11+,
clang-7+) and use it on start_kernel(), remove the call to
prevent_tail_call_optimization() such that we may one day remove it
outright.

Reviewed-by: Nathan Chancellor <[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]>
---
init/main.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/init/main.c b/init/main.c
index c445c1f..c0b6499 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1088,7 +1088,13 @@ void start_kernel(void)
/* Do the rest non-__init'ed, we're now alive */
arch_call_rest_init();

+ /*
+ * Avoid stack canaries in callers of boot_init_stack_canary for gcc-10
+ * and older.
+ */
+#if !__has_attribute(__no_stack_protector__)
prevent_tail_call_optimization();
+#endif
}

/* Call all constructor functions linked into the kernel. */