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@xxxxxxxxxxxxxx>
---
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
