[PATCH 0/5] add stack protector and guard page support

[PATCH 0/5] add stack protector and guard page support

[Ahmad Fatoum]

GCC's strong stack protector feature is increasingly used as default in
many distros, because of comparatively low overhead. This series adds
support in barebox to catch stack frame overflow as well as a guard
page feature to catch stack region overflow.

Ahmad Fatoum (5):
  include: move PAGE_ definitions into linux/pagemap.h
  ARM: mark early C setup functions as __prereloc
  lib: add stackprotector support
  ARM: mmu: catch stack overflowing into TTB with stack guard page
  commands: add stacksmash command for causing stack overflows

 Makefile                           |   3 -
 arch/arm/cpu/common.c              |   2 +-
 arch/arm/cpu/interrupts_32.c       |  21 +++++-
 arch/arm/cpu/interrupts_64.c       |  38 +++++++---
 arch/arm/cpu/mmu_32.c              |  16 +++++
 arch/arm/cpu/mmu_64.c              |  15 ++++
 arch/arm/cpu/start.c               |   4 +-
 arch/arm/include/asm/barebox-arm.h |  18 ++++-
 arch/arm/include/asm/reloc.h       |   2 +-
 arch/arm/lib64/string.c            |   2 +-
 commands/Kconfig                   |   6 ++
 commands/Makefile                  |   1 +
 commands/stacksmash.c              |  58 ++++++++++++++++
 include/common.h                   |   6 +-
 include/linux/compiler_types.h     |  21 ++++++
 include/linux/pagemap.h            |   8 ++-
 lib/Kconfig                        |   2 +
 lib/Kconfig.hardening              | 108 +++++++++++++++++++++++++++++
 lib/Makefile                       |   1 +
 lib/stackprot.c                    |  32 +++++++++
 lib/string.c                       |   2 +-
 scripts/Makefile.lib               |  10 +++
 22 files changed, 350 insertions(+), 26 deletions(-)
 create mode 100644 commands/stacksmash.c
 create mode 100644 lib/Kconfig.hardening
 create mode 100644 lib/stackprot.c

-- 
2.39.2

[PATCH 1/5] include: move PAGE_ definitions into linux/pagemap.h

[Ahmad Fatoum]

<common.h> is a fat header and we shouldn't need to include it just to
get a definition for PAGE_SIZE. In Linux PAGE_SIZE is defined per
architecture, but in barebox we only have 4K pages so far, so let's move
it into <linux/pagemap.h>.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/common.h        | 6 +-----
 include/linux/pagemap.h | 8 +++++++-
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/common.h b/include/common.h
index cf3e0447a09a..b7b4d9e35094 100644
--- a/include/common.h
+++ b/include/common.h
@@ -16,6 +16,7 @@
 #include <linux/string.h>
 #include <linux/kernel.h>
 #include <linux/stddef.h>
+#include <linux/pagemap.h>
 #include <asm/common.h>
 #include <asm/io.h>
 #include <linux/printk.h>
@@ -105,11 +106,6 @@ void shutdown_barebox(void);
 	char __##name[sizeof(type) * (size) + (align) - 1];	\
 	type *name = (type *)ALIGN((uintptr_t)__##name, align)
 
-#define PAGE_SIZE	4096
-#define PAGE_SHIFT	12
-#define PAGE_ALIGN(s)	ALIGN(s, PAGE_SIZE)
-#define PAGE_ALIGN_DOWN(x) ALIGN_DOWN(x, PAGE_SIZE)
-
 int mem_parse_options(int argc, char *argv[], char *optstr, int *mode,
 		char **sourcefile, char **destfile, int *swab);
 int memcpy_parse_options(int argc, char *argv[], int *sourcefd,
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 20c38a0b8912..01cbfc17c57b 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -3,11 +3,17 @@
 #ifndef _LINUX_PAGEMAP_H
 #define _LINUX_PAGEMAP_H
 
+#include <linux/kernel.h>
+
 /*
  * Copyright 1995 Linus Torvalds
  */
 
-#include <common.h>
+
+#define PAGE_SIZE	4096
+#define PAGE_SHIFT	12
+#define PAGE_ALIGN(s)	ALIGN(s, PAGE_SIZE)
+#define PAGE_ALIGN_DOWN(x) ALIGN_DOWN(x, PAGE_SIZE)
 
 #define PAGE_CACHE_SHIFT        PAGE_SHIFT
 #define PAGE_CACHE_SIZE         PAGE_SIZE
-- 
2.39.2

[PATCH 2/5] ARM: mark early C setup functions as __prereloc

[Ahmad Fatoum]

In preparation for adding stack protector support, we need to start
marking functions run before the C environment is completely set up.

Introduce a __prereloc attribute for this use case and an even stronger
no noinstr (no instrumentation) attribute and start adding it at enough
places for bareboxproper to start up with -fstack-protector-all.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/cpu/common.c          | 2 +-
 arch/arm/cpu/start.c           | 4 ++--
 arch/arm/include/asm/reloc.h   | 2 +-
 arch/arm/lib64/string.c        | 2 +-
 include/linux/compiler_types.h | 7 +++++++
 lib/string.c                   | 2 +-
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/common.c b/arch/arm/cpu/common.c
index 47da9fbe494f..e9118b450d3f 100644
--- a/arch/arm/cpu/common.c
+++ b/arch/arm/cpu/common.c
@@ -59,7 +59,7 @@ void pbl_barebox_break(void)
 /*
  * relocate binary to the currently running address
  */
-void relocate_to_current_adr(void)
+void __prereloc relocate_to_current_adr(void)
 {
 	unsigned long offset;
 	unsigned long __maybe_unused *dynsym, *dynend;
diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
index 2e987ec41d1e..15f5b2937227 100644
--- a/arch/arm/cpu/start.c
+++ b/arch/arm/cpu/start.c
@@ -137,7 +137,7 @@ static int barebox_memory_areas_init(void)
 }
 device_initcall(barebox_memory_areas_init);
 
-__noreturn __no_sanitize_address void barebox_non_pbl_start(unsigned long membase,
+__noreturn __prereloc void barebox_non_pbl_start(unsigned long membase,
 		unsigned long memsize, void *boarddata)
 {
 	unsigned long endmem = membase + memsize;
@@ -245,7 +245,7 @@ void start(unsigned long membase, unsigned long memsize, void *boarddata);
  * First function in the uncompressed image. We get here from
  * the pbl. The stack already has been set up by the pbl.
  */
-void NAKED __no_sanitize_address __section(.text_entry) start(unsigned long membase,
+void NAKED __prereloc __section(.text_entry) start(unsigned long membase,
 		unsigned long memsize, void *boarddata)
 {
 	barebox_non_pbl_start(membase, memsize, boarddata);
diff --git a/arch/arm/include/asm/reloc.h b/arch/arm/include/asm/reloc.h
index 0002c96c014c..95b4ef0af88b 100644
--- a/arch/arm/include/asm/reloc.h
+++ b/arch/arm/include/asm/reloc.h
@@ -12,7 +12,7 @@ unsigned long get_runtime_offset(void);
  * Get the offset of global variables when not running at the address we are
  * linked at.
  */
-static inline unsigned long global_variable_offset(void)
+static inline __prereloc unsigned long global_variable_offset(void)
 {
 #ifdef CONFIG_CPU_V8
 	unsigned long text;
diff --git a/arch/arm/lib64/string.c b/arch/arm/lib64/string.c
index 26a284be5a77..938790e1a9b2 100644
--- a/arch/arm/lib64/string.c
+++ b/arch/arm/lib64/string.c
@@ -7,7 +7,7 @@
 void *__arch_memset(void *dst, int c, __kernel_size_t size);
 void *__arch_memcpy(void * dest, const void *src, size_t count);
 
-static void *_memset(void *dst, int c, __kernel_size_t size)
+static __prereloc void *_memset(void *dst, int c, __kernel_size_t size)
 {
 	if (likely(get_cr() & CR_M))
 		return __arch_memset(dst, c, size);
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index bc1b43aab0dc..9ce272bba5f3 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -305,4 +305,11 @@ struct ftrace_likely_data {
  */
 #define noinline_for_stack noinline
 
+/* code that can't be instrumented at all */
+#define noinstr \
+	noinline notrace __no_sanitize_address
+
+#define __prereloc \
+	notrace __no_sanitize_address
+
 #endif /* __LINUX_COMPILER_TYPES_H */
diff --git a/lib/string.c b/lib/string.c
index 8ea68044cc0a..166ef190d6aa 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -534,7 +534,7 @@ void *__default_memset(void * s, int c, size_t count)
 }
 EXPORT_SYMBOL(__default_memset);
 
-void __no_sanitize_address *__nokasan_default_memset(void * s, int c, size_t count)
+void __prereloc __no_sanitize_address *__nokasan_default_memset(void * s, int c, size_t count)
 {
 	char *xs = (char *) s;
 
-- 
2.39.2

[PATCH 3/5] lib: add stackprotector support

[Ahmad Fatoum]

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

[PATCH 4/5] ARM: mmu: catch stack overflowing into TTB with stack guard page

[Ahmad Fatoum]

While barebox stack is often quite generous, due to its default of 32K,
bugs can make it overflow and on ARM, this clobbers the page tables
leading to even harder to debug problems than usual.

Let's add a 4K buffer zone between the page tables and the stack and
configure the MMU to trap all accesses into it.

Note that hitting the stack guard page can be silent if the exception
handler places it's frame there. Still a hanging barebox may be better
than an erratically behaving one.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/cpu/interrupts_32.c       | 21 +++++++++++++++--
 arch/arm/cpu/interrupts_64.c       | 38 +++++++++++++++++++++++-------
 arch/arm/cpu/mmu_32.c              | 16 +++++++++++++
 arch/arm/cpu/mmu_64.c              | 15 ++++++++++++
 arch/arm/include/asm/barebox-arm.h | 18 +++++++++++++-
 lib/Kconfig.hardening              | 10 ++++++++
 6 files changed, 107 insertions(+), 11 deletions(-)

diff --git a/arch/arm/cpu/interrupts_32.c b/arch/arm/cpu/interrupts_32.c
index 5bc790a796fb..468dcdd30e93 100644
--- a/arch/arm/cpu/interrupts_32.c
+++ b/arch/arm/cpu/interrupts_32.c
@@ -8,7 +8,9 @@
 
 #include <common.h>
 #include <abort.h>
+#include <linux/sizes.h>
 #include <asm/ptrace.h>
+#include <asm/barebox-arm.h>
 #include <asm/unwind.h>
 #include <init.h>
 
@@ -106,6 +108,22 @@ void do_prefetch_abort (struct pt_regs *pt_regs)
 	do_exception(pt_regs);
 }
 
+static const char *data_abort_reason(ulong far)
+{
+	ulong guard_page;
+
+	if (far < PAGE_SIZE)
+		return "NULL pointer dereference";
+
+	if (IS_ENABLED(CONFIG_STACK_GUARD_PAGE)) {
+		guard_page = arm_mem_guard_page_get();
+		if (guard_page <= far && far < guard_page + PAGE_SIZE)
+			return "stack overflow";
+	}
+
+	return "paging request";
+}
+
 /**
  * The CPU catches a data abort. That really should not happen!
  * @param[in] pt_regs Register set content when the accident happens
@@ -119,8 +137,7 @@ void do_data_abort (struct pt_regs *pt_regs)
 	asm volatile ("mrc     p15, 0, %0, c6, c0, 0" : "=r" (far) : : "cc");
 
 	printf("unable to handle %s at address 0x%08x\n",
-			far < PAGE_SIZE ? "NULL pointer dereference" :
-			"paging request", far);
+	       data_abort_reason(far), far);
 
 	do_exception(pt_regs);
 }
diff --git a/arch/arm/cpu/interrupts_64.c b/arch/arm/cpu/interrupts_64.c
index d844915fee24..b3e7da179756 100644
--- a/arch/arm/cpu/interrupts_64.c
+++ b/arch/arm/cpu/interrupts_64.c
@@ -6,6 +6,7 @@
 #include <common.h>
 #include <abort.h>
 #include <asm/ptrace.h>
+#include <asm/barebox-arm.h>
 #include <asm/unwind.h>
 #include <init.h>
 #include <asm/system.h>
@@ -142,17 +143,38 @@ void do_bad_error(struct pt_regs *pt_regs)
 extern volatile int arm_ignore_data_abort;
 extern volatile int arm_data_abort_occurred;
 
-void do_sync(struct pt_regs *pt_regs, unsigned int esr, unsigned long far)
+static const char *data_abort_reason(ulong far)
 {
-	if ((esr >> ESR_ELx_EC_SHIFT) == ESR_ELx_EC_DABT_CUR &&
-			arm_ignore_data_abort) {
-		arm_data_abort_occurred = 1;
-		pt_regs->elr += 4;
-		return;
+	ulong guard_page;
+
+	if (far < PAGE_SIZE)
+		return "NULL pointer dereference: ";
+
+	if (IS_ENABLED(CONFIG_STACK_GUARD_PAGE)) {
+		guard_page = arm_mem_guard_page_get();
+		if (guard_page <= far && far < guard_page + PAGE_SIZE)
+			return "Stack overflow: ";
 	}
 
-	printf("%s exception (ESR 0x%08x) at 0x%016lx\n", esr_get_class_string(esr),
-	       esr, far);
+	return NULL;
+}
+
+void do_sync(struct pt_regs *pt_regs, unsigned int esr, unsigned long far)
+{
+	const char *extra = NULL;
+
+	if ((esr >> ESR_ELx_EC_SHIFT) == ESR_ELx_EC_DABT_CUR) {
+		if (arm_ignore_data_abort) {
+			arm_data_abort_occurred = 1;
+			pt_regs->elr += 4;
+			return;
+		}
+
+		extra = data_abort_reason(far);
+	}
+
+	printf("%s%s exception (ESR 0x%08x) at 0x%016lx\n", extra ?: "",
+	       esr_get_class_string(esr), esr, far);
 	do_exception(pt_regs);
 }
 
diff --git a/arch/arm/cpu/mmu_32.c b/arch/arm/cpu/mmu_32.c
index c5d64aa88bac..07b225067796 100644
--- a/arch/arm/cpu/mmu_32.c
+++ b/arch/arm/cpu/mmu_32.c
@@ -475,11 +475,27 @@ static void create_zero_page(void)
 	pr_debug("Created zero page\n");
 }
 
+static void create_guard_page(void)
+{
+	ulong guard_page;
+
+	if (!IS_ENABLED(CONFIG_STACK_GUARD_PAGE))
+		return;
+
+	guard_page = arm_mem_guard_page_get();
+	request_sdram_region("guard page", guard_page, PAGE_SIZE);
+	remap_range((void *)guard_page, PAGE_SIZE, MAP_FAULT);
+
+	pr_debug("Created guard page\n");
+}
+
 /*
  * Map vectors and zero page
  */
 static void vectors_init(void)
 {
+	create_guard_page();
+
 	/*
 	 * First try to use the vectors where they actually are, works
 	 * on ARMv7 and later.
diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index 3124f8f3a987..fb57260c90ae 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -204,6 +204,20 @@ static void mmu_enable(void)
 	set_cr(get_cr() | CR_M | CR_C | CR_I);
 }
 
+static void create_guard_page(void)
+{
+	ulong guard_page;
+
+	if (!IS_ENABLED(CONFIG_STACK_GUARD_PAGE))
+		return;
+
+	guard_page = arm_mem_guard_page_get();
+	request_sdram_region("guard page", guard_page, PAGE_SIZE);
+	remap_range((void *)guard_page, PAGE_SIZE, MAP_FAULT);
+
+	pr_debug("Created guard page\n");
+}
+
 /*
  * Prepare MMU for usage enable it.
  */
@@ -241,6 +255,7 @@ void __mmu_init(bool mmu_on)
 
 	/* Make zero page faulting to catch NULL pointer derefs */
 	zero_page_faulting();
+	create_guard_page();
 }
 
 void mmu_disable(void)
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index aceb7fdf74f8..382fa8505a66 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -15,6 +15,7 @@
 #include <linux/sizes.h>
 #include <asm-generic/memory_layout.h>
 #include <linux/kernel.h>
+#include <linux/pagemap.h>
 #include <linux/types.h>
 #include <linux/compiler.h>
 #include <asm/barebox-arm-head.h>
@@ -82,9 +83,19 @@ static inline unsigned long arm_mem_stack(unsigned long endmem)
 	return arm_mem_scratch(endmem) - STACK_SIZE;
 }
 
-static inline unsigned long arm_mem_ttb(unsigned long endmem)
+static inline unsigned long arm_mem_guard_page(unsigned long endmem)
 {
 	endmem = arm_mem_stack(endmem);
+
+	if (!IS_ENABLED(CONFIG_STACK_GUARD_PAGE))
+		return endmem;
+
+	return ALIGN_DOWN(endmem, PAGE_SIZE) - PAGE_SIZE;
+}
+
+static inline unsigned long arm_mem_ttb(unsigned long endmem)
+{
+	endmem = arm_mem_guard_page(endmem);
 	endmem = ALIGN_DOWN(endmem, ARM_EARLY_PAGETABLE_SIZE) - ARM_EARLY_PAGETABLE_SIZE;
 
 	return endmem;
@@ -121,6 +132,11 @@ static inline const void *arm_mem_scratch_get(void)
 	return (const void *)arm_mem_scratch(arm_mem_endmem_get());
 }
 
+static inline unsigned long arm_mem_guard_page_get(void)
+{
+	return arm_mem_guard_page(arm_mem_endmem_get());
+}
+
 static inline unsigned long arm_mem_barebox_image(unsigned long membase,
 						  unsigned long endmem,
 						  unsigned long size)
diff --git a/lib/Kconfig.hardening b/lib/Kconfig.hardening
index 503fdf7c0cc5..aad0d8b97024 100644
--- a/lib/Kconfig.hardening
+++ b/lib/Kconfig.hardening
@@ -1,5 +1,15 @@
 menu "Hardening options"
 
+config STACK_GUARD_PAGE
+	bool "Place guard page to catch stack overflows"
+	depends on ARM && MMU
+	help
+	  When enabled, barebox places a faulting guard page to catch total
+	  stack usage exceeding CONFIG_STACK_SIZE. On overflows, that hit
+	  the reserved 4KiB, barebox will panic and report a stack overflow.
+	  The report may not always succeed if the stack overflow impacts
+	  operation of the exception handler.
+
 config STACKPROTECTOR
 	bool
 
-- 
2.39.2

[PATCH 5/5] commands: add stacksmash command for causing stack overflows

[Ahmad Fatoum]

Now that we have two mechanisms for detecting stack overflows, add a
command to intentionally trigger stack frame and stack region overflow
to verify their correct operation.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/Kconfig      |  6 +++++
 commands/Makefile     |  1 +
 commands/stacksmash.c | 58 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+)
 create mode 100644 commands/stacksmash.c

diff --git a/commands/Kconfig b/commands/Kconfig
index eb95b2a5fbcc..c1bba22443e6 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -2401,6 +2401,12 @@ config CMD_UBSAN
 	  This is a test command for the undefined behavior sanitizer.
 	  It triggers various undefined behavior, and detect it.
 
+config CMD_STACKSMASH
+	tristate "stacksmash"
+	help
+	  This commands trashes the stack to test stackprotector and
+	  guard page. This command does not return.
+
 # end Miscellaneous commands
 endmenu
 
diff --git a/commands/Makefile b/commands/Makefile
index 4b083a852d83..4924755500e3 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -145,5 +145,6 @@ obj-$(CONFIG_CMD_BTHREAD)	+= bthread.o
 obj-$(CONFIG_CMD_UBSAN)		+= ubsan.o
 obj-$(CONFIG_CMD_SELFTEST)	+= selftest.o
 obj-$(CONFIG_CMD_TUTORIAL)	+= tutorial.o
+obj-$(CONFIG_CMD_STACKSMASH)	+= stacksmash.o
 
 UBSAN_SANITIZE_ubsan.o := y
diff --git a/commands/stacksmash.c b/commands/stacksmash.c
new file mode 100644
index 000000000000..1e9be0d40e15
--- /dev/null
+++ b/commands/stacksmash.c
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <common.h>
+#include <command.h>
+#include <complete.h>
+#include <linux/compiler.h>
+#include <string.h>
+
+static noinline void stack_overflow_frame(void)
+{
+	volatile int length = 512;
+	char a[128] = {};
+
+	/*
+	 * In order to avoid having the compiler optimize away the stack smashing
+	 * we need to do a little something here.
+	 */
+	OPTIMIZER_HIDE_VAR(length);
+
+	memset(a, 0xa5, length);
+
+	printf("We have smashed our stack as this should not exceed 128: sizeof(a) = %zu\n",
+	       strlen(a));
+}
+
+static noinline void stack_overflow_region(u64 i)
+{
+	volatile char a[1024] = {};
+
+	if (ctrlc())
+		return;
+
+	RELOC_HIDE(&a, 0);
+
+	stack_overflow_region(0);
+
+	printf("%*ph", 1024, a);
+}
+
+static int do_stacksmash(int argc, char *argv[])
+{
+	if (argc != 2)
+		return COMMAND_ERROR_USAGE;
+
+	if (!strcmp(argv[1], "frame"))
+		stack_overflow_frame();
+	else if (!strcmp(argv[1], "region"))
+		stack_overflow_region(0);
+
+	panic("Stack smashing of %s not caught\n", argv[1]);
+}
+BAREBOX_CMD_START(stacksmash)
+        .cmd            = do_stacksmash,
+        BAREBOX_CMD_DESC("Run stack smashing tests")
+	BAREBOX_CMD_OPTS("[frame | region]")
+        BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+        BAREBOX_CMD_COMPLETE(empty_complete)
+BAREBOX_CMD_END
-- 
2.39.2

[PATCH] fixup! lib: add stackprotector support

[Ahmad Fatoum]

lib: stackprotector: add prompt text to option

Kconfig rightfully complains about it:

  lib/Kconfig.hardening:66:warning: choice value must have a prompt

So add it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 lib/Kconfig.hardening | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig.hardening b/lib/Kconfig.hardening
index aad0d8b97024..a9d3af110958 100644
--- a/lib/Kconfig.hardening
+++ b/lib/Kconfig.hardening
@@ -64,7 +64,7 @@ choice
 	prompt "Stack Protector buffer overflow detection for PBL"
 
 config PBL_STACKPROTECTOR_NONE
-	bool
+	bool "None"
 
 config PBL_STACKPROTECTOR_STRONG
 	bool "Strong"
-- 
2.39.2

Re: [PATCH 5/5] commands: add stacksmash command for causing stack overflows

[Thorsten Scherer]

Hi Ahmad,

On Mon, Sep 11, 2023 at 05:09:00PM +0200, Ahmad Fatoum wrote:
> Now that we have two mechanisms for detecting stack overflows, add a
> command to intentionally trigger stack frame and stack region overflow
> to verify their correct operation.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  commands/Kconfig      |  6 +++++
>  commands/Makefile     |  1 +
>  commands/stacksmash.c | 58 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 65 insertions(+)
>  create mode 100644 commands/stacksmash.c
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index eb95b2a5fbcc..c1bba22443e6 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -2401,6 +2401,12 @@ config CMD_UBSAN
>  	  This is a test command for the undefined behavior sanitizer.
>  	  It triggers various undefined behavior, and detect it.
>  
> +config CMD_STACKSMASH
> +	tristate "stacksmash"
> +	help
> +	  This commands trashes the stack to test stackprotector and
> +	  guard page. This command does not return.
> +
>  # end Miscellaneous commands
>  endmenu
>  
> diff --git a/commands/Makefile b/commands/Makefile
> index 4b083a852d83..4924755500e3 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -145,5 +145,6 @@ obj-$(CONFIG_CMD_BTHREAD)	+= bthread.o
>  obj-$(CONFIG_CMD_UBSAN)		+= ubsan.o
>  obj-$(CONFIG_CMD_SELFTEST)	+= selftest.o
>  obj-$(CONFIG_CMD_TUTORIAL)	+= tutorial.o
> +obj-$(CONFIG_CMD_STACKSMASH)	+= stacksmash.o
>  
>  UBSAN_SANITIZE_ubsan.o := y
> diff --git a/commands/stacksmash.c b/commands/stacksmash.c
> new file mode 100644
> index 000000000000..1e9be0d40e15
> --- /dev/null
> +++ b/commands/stacksmash.c
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <complete.h>
> +#include <linux/compiler.h>
> +#include <string.h>
> +
> +static noinline void stack_overflow_frame(void)
> +{
> +	volatile int length = 512;
> +	char a[128] = {};
> +
> +	/*
> +	 * In order to avoid having the compiler optimize away the stack smashing
> +	 * we need to do a little something here.
> +	 */
> +	OPTIMIZER_HIDE_VAR(length);
> +
> +	memset(a, 0xa5, length);
> +
> +	printf("We have smashed our stack as this should not exceed 128: sizeof(a) = %zu\n",
> +	       strlen(a));
> +}
> +
> +static noinline void stack_overflow_region(u64 i)
> +{
> +	volatile char a[1024] = {};
> +
> +	if (ctrlc())
> +		return;
> +
> +	RELOC_HIDE(&a, 0);
> +
> +	stack_overflow_region(0);
> +
> +	printf("%*ph", 1024, a);
> +}
> +
> +static int do_stacksmash(int argc, char *argv[])
> +{
> +	if (argc != 2)
> +		return COMMAND_ERROR_USAGE;
> +
> +	if (!strcmp(argv[1], "frame"))
> +		stack_overflow_frame();
> +	else if (!strcmp(argv[1], "region"))
> +		stack_overflow_region(0);
> +
> +	panic("Stack smashing of %s not caught\n", argv[1]);
> +}
> +BAREBOX_CMD_START(stacksmash)
> +        .cmd            = do_stacksmash,
> +        BAREBOX_CMD_DESC("Run stack smashing tests")
> +	BAREBOX_CMD_OPTS("[frame | region]")

Indentation is a bit broken here.

> +        BAREBOX_CMD_GROUP(CMD_GRP_MISC)
> +        BAREBOX_CMD_COMPLETE(empty_complete)
> +BAREBOX_CMD_END
> -- 
> 2.39.2
> 
> 

Best regards
Thorsten

[PATCH] fixup! commands: add stacksmash command for causing stack overflows

[Ahmad Fatoum]

commands: stacksmash: fix whitespace in command description

Command description mixed spaces and tabs. Fix it to have tabs only.

Cc: Thorsten Scherer <T.Scherer@eckelmann.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Thanks, Thorsten!
---
 commands/stacksmash.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/commands/stacksmash.c b/commands/stacksmash.c
index 1e9be0d40e15..b812d945e215 100644
--- a/commands/stacksmash.c
+++ b/commands/stacksmash.c
@@ -50,9 +50,9 @@ static int do_stacksmash(int argc, char *argv[])
 	panic("Stack smashing of %s not caught\n", argv[1]);
 }
 BAREBOX_CMD_START(stacksmash)
-        .cmd            = do_stacksmash,
-        BAREBOX_CMD_DESC("Run stack smashing tests")
+	.cmd            = do_stacksmash,
+	BAREBOX_CMD_DESC("Run stack smashing tests")
 	BAREBOX_CMD_OPTS("[frame | region]")
-        BAREBOX_CMD_GROUP(CMD_GRP_MISC)
-        BAREBOX_CMD_COMPLETE(empty_complete)
+	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+	BAREBOX_CMD_COMPLETE(empty_complete)
 BAREBOX_CMD_END
-- 
2.39.2

Re: [PATCH] fixup! commands: add stacksmash command for causing stack overflows

[Thorsten Scherer]

Hi Ahmad,

sha just applied four of the five patches and stacksmash isn't in next
yet.

Refer,

    Message-ID: <20230912093638.GZ637806@pengutronix.de>

Best regards,
Thorsten

On Thu, Sep 14, 2023 at 11:14:38AM +0200, Ahmad Fatoum wrote:
> commands: stacksmash: fix whitespace in command description
> 
> Command description mixed spaces and tabs. Fix it to have tabs only.
> 
> Cc: Thorsten Scherer <T.Scherer@eckelmann.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> Thanks, Thorsten!
> ---
>  commands/stacksmash.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/commands/stacksmash.c b/commands/stacksmash.c
> index 1e9be0d40e15..b812d945e215 100644
> --- a/commands/stacksmash.c
> +++ b/commands/stacksmash.c
> @@ -50,9 +50,9 @@ static int do_stacksmash(int argc, char *argv[])
>  	panic("Stack smashing of %s not caught\n", argv[1]);
>  }
>  BAREBOX_CMD_START(stacksmash)
> -        .cmd            = do_stacksmash,
> -        BAREBOX_CMD_DESC("Run stack smashing tests")
> +	.cmd            = do_stacksmash,
> +	BAREBOX_CMD_DESC("Run stack smashing tests")
>  	BAREBOX_CMD_OPTS("[frame | region]")
> -        BAREBOX_CMD_GROUP(CMD_GRP_MISC)
> -        BAREBOX_CMD_COMPLETE(empty_complete)
> +	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
> +	BAREBOX_CMD_COMPLETE(empty_complete)
>  BAREBOX_CMD_END
> -- 
> 2.39.2
> 

Re: [PATCH] fixup! commands: add stacksmash command for causing stack overflows

[Ahmad Fatoum]

On 14.09.23 12:22, Thorsten Scherer wrote:
> Hi Ahmad,
> 
> sha just applied four of the five patches and stacksmash isn't in next
> yet.
> 
> Refer,
> 
>     Message-ID: <20230912093638.GZ637806@pengutronix.de>

He did? 20230912093638.GZ637806@pengutronix.de is an unrelated series:

https://lore.barebox.org/barebox/20230912093638.GZ637806@pengutronix.de/

Cheers,
Ahmad

> 
> Best regards,
> Thorsten
> 
> On Thu, Sep 14, 2023 at 11:14:38AM +0200, Ahmad Fatoum wrote:
>> commands: stacksmash: fix whitespace in command description
>>
>> Command description mixed spaces and tabs. Fix it to have tabs only.
>>
>> Cc: Thorsten Scherer <T.Scherer@eckelmann.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> Thanks, Thorsten!
>> ---
>>  commands/stacksmash.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/commands/stacksmash.c b/commands/stacksmash.c
>> index 1e9be0d40e15..b812d945e215 100644
>> --- a/commands/stacksmash.c
>> +++ b/commands/stacksmash.c
>> @@ -50,9 +50,9 @@ static int do_stacksmash(int argc, char *argv[])
>>  	panic("Stack smashing of %s not caught\n", argv[1]);
>>  }
>>  BAREBOX_CMD_START(stacksmash)
>> -        .cmd            = do_stacksmash,
>> -        BAREBOX_CMD_DESC("Run stack smashing tests")
>> +	.cmd            = do_stacksmash,
>> +	BAREBOX_CMD_DESC("Run stack smashing tests")
>>  	BAREBOX_CMD_OPTS("[frame | region]")
>> -        BAREBOX_CMD_GROUP(CMD_GRP_MISC)
>> -        BAREBOX_CMD_COMPLETE(empty_complete)
>> +	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
>> +	BAREBOX_CMD_COMPLETE(empty_complete)
>>  BAREBOX_CMD_END
>> -- 
>> 2.39.2
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/5] add stack protector and guard page support

[Sascha Hauer]

On Mon, Sep 11, 2023 at 05:08:55PM +0200, Ahmad Fatoum wrote:
> GCC's strong stack protector feature is increasingly used as default in
> many distros, because of comparatively low overhead. This series adds
> support in barebox to catch stack frame overflow as well as a guard
> page feature to catch stack region overflow.
> 
> Ahmad Fatoum (5):
>   include: move PAGE_ definitions into linux/pagemap.h
>   ARM: mark early C setup functions as __prereloc
>   lib: add stackprotector support
>   ARM: mmu: catch stack overflowing into TTB with stack guard page
>   commands: add stacksmash command for causing stack overflows

Applied, thanks

Sascha

> 
>  Makefile                           |   3 -
>  arch/arm/cpu/common.c              |   2 +-
>  arch/arm/cpu/interrupts_32.c       |  21 +++++-
>  arch/arm/cpu/interrupts_64.c       |  38 +++++++---
>  arch/arm/cpu/mmu_32.c              |  16 +++++
>  arch/arm/cpu/mmu_64.c              |  15 ++++
>  arch/arm/cpu/start.c               |   4 +-
>  arch/arm/include/asm/barebox-arm.h |  18 ++++-
>  arch/arm/include/asm/reloc.h       |   2 +-
>  arch/arm/lib64/string.c            |   2 +-
>  commands/Kconfig                   |   6 ++
>  commands/Makefile                  |   1 +
>  commands/stacksmash.c              |  58 ++++++++++++++++
>  include/common.h                   |   6 +-
>  include/linux/compiler_types.h     |  21 ++++++
>  include/linux/pagemap.h            |   8 ++-
>  lib/Kconfig                        |   2 +
>  lib/Kconfig.hardening              | 108 +++++++++++++++++++++++++++++
>  lib/Makefile                       |   1 +
>  lib/stackprot.c                    |  32 +++++++++
>  lib/string.c                       |   2 +-
>  scripts/Makefile.lib               |  10 +++
>  22 files changed, 350 insertions(+), 26 deletions(-)
>  create mode 100644 commands/stacksmash.c
>  create mode 100644 lib/Kconfig.hardening
>  create mode 100644 lib/stackprot.c
> 
> -- 
> 2.39.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH] fixup! lib: add stackprotector support

[Ahmad Fatoum]

lib: stackprot: annotate stackprot_randomize_guard with __no_stack_protector

stackprot_randomize_guard() changes the stack protector, so it's
important that it can't make use of the stackprotector itself.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Not a problem in practice unless we use stackprotector-all, which we
don't.
---
 lib/stackprot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/stackprot.c b/lib/stackprot.c
index ca89b37d9042..aa0e88603aae 100644
--- a/lib/stackprot.c
+++ b/lib/stackprot.c
@@ -25,7 +25,7 @@ noinstr void __stack_chk_fail(void)
 }
 EXPORT_SYMBOL(__stack_chk_fail);
 
-static int stackprot_randomize_guard(void)
+static __no_stack_protector int stackprot_randomize_guard(void)
 {
 	return get_crypto_bytes(&__stack_chk_guard, sizeof(__stack_chk_guard));
 }
-- 
2.39.2