mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: barebox@lists.infradead.org
Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Subject: [PATCH 3/5] lib: add stackprotector support
Date: Mon, 11 Sep 2023 17:08:58 +0200	[thread overview]
Message-ID: <20230911150900.3584523-4-a.fatoum@pengutronix.de> (raw)
In-Reply-To: <20230911150900.3584523-1-a.fatoum@pengutronix.de>

GCC's "stack-protector" puts, at the beginning of functions, a canary value
on the stack just before the return address, and validates the value just
before actually returning.  Stack based buffer overflows (that need to
overwrite this return address) now also overwrite the canary, which gets
detected and the attack is then neutralized via a barebox panic.

Unlike Linux, we do not add support for the regular stack protector, as
that relies on a heuristic to detect vulnerable functions, which is
greatly improved upon by the later added strong stack protector.

In return, we add a CONFIG_STACKPROTECTOR_ALL option that's missing in
Linux: This turns out to be a nice way to find out, which functions lack
a __prereloc (or __no_stack_protector) annotation as every function will
access the canary and that fails if the function is called prior to
relocation. We don't give it a prompt though, because it's only
interesting for development.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 Makefile                       |  3 --
 include/linux/compiler_types.h | 18 ++++++-
 lib/Kconfig                    |  2 +
 lib/Kconfig.hardening          | 98 ++++++++++++++++++++++++++++++++++
 lib/Makefile                   |  1 +
 lib/stackprot.c                | 32 +++++++++++
 scripts/Makefile.lib           | 10 ++++
 7 files changed, 159 insertions(+), 5 deletions(-)
 create mode 100644 lib/Kconfig.hardening
 create mode 100644 lib/stackprot.c

diff --git a/Makefile b/Makefile
index fb05e5ee7b22..6b3f035f2eb7 100644
--- a/Makefile
+++ b/Makefile
@@ -656,9 +656,6 @@ KBUILD_CFLAGS	+= -fno-omit-frame-pointer -fno-optimize-sibling-calls
 KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
 endif
 
-# Force gcc to behave correct even for buggy distributions
-KBUILD_CFLAGS          += $(call cc-option, -fno-stack-protector)
-
 KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
 
 # This warning generated too much noise in a regular build.
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 9ce272bba5f3..800bc518feea 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -133,6 +133,20 @@ struct ftrace_likely_data {
 # define fallthrough                    do {} while (0)  /* fallthrough */
 #endif
 
+/*
+ * 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__))
+#elif ! defined CONFIG_STACKPROTECTOR
+# define __no_stack_protector		__attribute__((__optimize__("-fno-stack-protector")))
+#else
+# define __no_stack_protector
+#endif
+
 #endif /* __KERNEL__ */
 
 #endif /* __ASSEMBLY__ */
@@ -307,9 +321,9 @@ struct ftrace_likely_data {
 
 /* code that can't be instrumented at all */
 #define noinstr \
-	noinline notrace __no_sanitize_address
+	noinline notrace __no_sanitize_address __no_stack_protector
 
 #define __prereloc \
-	notrace __no_sanitize_address
+	notrace __no_sanitize_address __no_stack_protector
 
 #endif /* __LINUX_COMPILER_TYPES_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index aaede6864533..fbc9fff8654c 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -227,3 +227,5 @@ config GENERIC_ALLOCATOR
 	  Support is curently limited to allocaing a complete mmio-sram at once.
 
 endmenu
+
+source "lib/Kconfig.hardening"
diff --git a/lib/Kconfig.hardening b/lib/Kconfig.hardening
new file mode 100644
index 000000000000..503fdf7c0cc5
--- /dev/null
+++ b/lib/Kconfig.hardening
@@ -0,0 +1,98 @@
+menu "Hardening options"
+
+config STACKPROTECTOR
+	bool
+
+choice
+	prompt "Stack Protector buffer overflow detection"
+
+config STACKPROTECTOR_NONE
+	bool "None"
+
+config STACKPROTECTOR_STRONG
+	bool "Strong"
+	depends on $(cc-option,-fstack-protector-strong)
+	select STACKPROTECTOR
+	help
+	  This option turns on the "stack-protector" GCC feature. This
+	  feature puts, at the beginning of functions, a canary value on
+	  the stack just before the return address, and validates
+	  the value just before actually returning.  Stack based buffer
+	  overflows (that need to overwrite this return address) now also
+	  overwrite the canary, which gets detected and the attack is then
+	  neutralized via a kernel panic.
+
+	  Functions will have the stack-protector canary logic added in any
+	  of the following conditions:
+
+	  - local variable's address used as part of the right hand side of an
+	    assignment or function argument
+	  - local variable is an array (or union containing an array),
+	    regardless of array type or length
+	  - uses register local variables
+
+	  The canary will be a fixed value at first, but will be replaced by
+	  one generated from a hardware random number generator if available
+	  later on.
+
+config STACKPROTECTOR_ALL
+	bool "All"
+	depends on $(cc-option,-fstack-protector-all)
+	depends on COMPILE_TEST
+	select STACKPROTECTOR
+	help
+	  This pushes and verifies stack protector canaries on all functions,
+	  even those that don't need it. As this implies injection of a
+	  global variable dependency on every function, this option is useful
+	  for crashing functions called prior to prerelocation, which lack a
+	  __prereloc attribute. This is likely the only upside compared to
+	  the strong variant, so it's not selectable by default.
+
+endchoice
+
+choice
+	prompt "Stack Protector buffer overflow detection for PBL"
+
+config PBL_STACKPROTECTOR_NONE
+	bool
+
+config PBL_STACKPROTECTOR_STRONG
+	bool "Strong"
+	depends on $(cc-option,-fstack-protector-strong)
+	select STACKPROTECTOR
+	help
+	  For PBL, This option turns on the "stack-protector" GCC feature. This
+	  feature puts, at the beginning of functions, a canary value on
+	  the stack just before the return address, and validates
+	  the value just before actually returning.  Stack based buffer
+	  overflows (that need to overwrite this return address) now also
+	  overwrite the canary, which gets detected and the attack is then
+	  neutralized via a kernel panic.
+
+	  Functions will have the stack-protector canary logic added in any
+	  of the following conditions:
+
+	  - local variable's address used as part of the right hand side of an
+	    assignment or function argument
+	  - local variable is an array (or union containing an array),
+	    regardless of array type or length
+	  - uses register local variables
+
+	  The canary is always a fixed value.
+
+config PBL_STACKPROTECTOR_ALL
+	bool "PBL"
+	depends on $(cc-option,-fstack-protector-strong)
+	depends on COMPILE_TEST
+	select STACKPROTECTOR
+	help
+	  This pushes and verifies stack protector canaries on all functions,
+	  even those that don't need it. As this implies injection of a
+	  global variable dependency on every function, this option is useful
+	  for crashing functions called prior to prerelocation, which lack a
+	  __prereloc attribute. This is likely the only upside compared to
+	  the strong variant.
+
+endchoice
+
+endmenu
diff --git a/lib/Makefile b/lib/Makefile
index 921e5eedf46e..2b577becc444 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -11,6 +11,7 @@ obj-y			+= strtox.o
 obj-y			+= kstrtox.o
 obj-y			+= vsprintf.o
 obj-$(CONFIG_KASAN)	+= kasan/
+obj-pbl-$(CONFIG_STACKPROTECTOR)	+= stackprot.o
 pbl-$(CONFIG_PBL_CONSOLE) += vsprintf.o
 obj-y			+= misc.o
 obj-$(CONFIG_PARAMETER)	+= parameter.o
diff --git a/lib/stackprot.c b/lib/stackprot.c
new file mode 100644
index 000000000000..ca89b37d9042
--- /dev/null
+++ b/lib/stackprot.c
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <printk.h>
+#include <linux/kernel.h>
+#include <linux/export.h>
+#include <init.h>
+#include <stdlib.h>
+
+#ifdef __PBL__
+#define STAGE "PBL"
+#else
+#define STAGE "barebox"
+#endif
+
+void __stack_chk_fail(void);
+
+unsigned long __stack_chk_guard = (unsigned long)(0xfeedf00ddeadbeef & ~0UL);
+
+/*
+ * Called when gcc's -fstack-protector feature is used, and
+ * gcc detects corruption of the on-stack canary value
+ */
+noinstr void __stack_chk_fail(void)
+{
+	panic("stack-protector: " STAGE " stack is corrupted in: %pS\n", _RET_IP_);
+}
+EXPORT_SYMBOL(__stack_chk_fail);
+
+static int stackprot_randomize_guard(void)
+{
+	return get_crypto_bytes(&__stack_chk_guard, sizeof(__stack_chk_guard));
+}
+late_initcall(stackprot_randomize_guard);
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 5f0e666068f3..2af468803d8e 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -160,6 +160,16 @@ ifeq ($(CONFIG_DEBUG_PBL),y)
 PBL_CPPFLAGS   += -DDEBUG
 endif
 
+_stackp_flags-y                                        := -fno-stack-protector
+_stackp_flags-$(CONFIG_STACKPROTECTOR_STRONG)          := -fstack-protector-strong
+_stackp_flags-$(CONFIG_STACKPROTECTOR_ALL)             := -fstack-protector-all
+
+_stackp_flags_pbl-y                                    := -fno-stack-protector
+_stackp_flags_pbl-$(CONFIG_PBL_STACKPROTECTOR_STRONG)  := -fstack-protector-strong
+_stackp_flags_pbl-$(CONFIG_PBL_STACKPROTECTOR_ALL)     := -fstack-protector-all
+
+_c_flags += $(if $(part-of-pbl),$(_stackp_flags_pbl-y),$(_stackp_flags-y))
+
 # If building barebox in a separate objtree expand all occurrences
 # of -Idir to -I$(srctree)/dir except for absolute paths (starting with '/').
 
-- 
2.39.2




  parent reply	other threads:[~2023-09-11 15:10 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-11 15:08 [PATCH 0/5] add stack protector and guard page support Ahmad Fatoum
2023-09-11 15:08 ` [PATCH 1/5] include: move PAGE_ definitions into linux/pagemap.h Ahmad Fatoum
2023-09-11 15:08 ` [PATCH 2/5] ARM: mark early C setup functions as __prereloc Ahmad Fatoum
2023-09-11 15:08 ` Ahmad Fatoum [this message]
2023-09-21  8:52   ` [PATCH] fixup! lib: add stackprotector support Ahmad Fatoum
2023-09-11 15:08 ` [PATCH 4/5] ARM: mmu: catch stack overflowing into TTB with stack guard page Ahmad Fatoum
2023-09-11 15:09 ` [PATCH 5/5] commands: add stacksmash command for causing stack overflows Ahmad Fatoum
2023-09-12  4:48   ` Thorsten Scherer
2023-09-11 15:47 ` [PATCH] fixup! lib: add stackprotector support Ahmad Fatoum
2023-09-14  9:14 ` [PATCH] fixup! commands: add stacksmash command for causing stack overflows Ahmad Fatoum
2023-09-14 10:22   ` Thorsten Scherer
2023-09-14 11:05     ` Ahmad Fatoum
2023-09-21  8:49 ` [PATCH 0/5] add stack protector and guard page support Sascha Hauer

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=20230911150900.3584523-4-a.fatoum@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox