[PATCH 1/5] lib: ubsan: disable sanitization for UBSAN implementation

[PATCH 1/5] lib: ubsan: disable sanitization for UBSAN implementation

[Ahmad Fatoum]

As done in Linux, the implementation of ubsan itself should not be
instrumented by KASAN and stack protector.

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

diff --git a/lib/Makefile b/lib/Makefile
index 2b577becc444..791080b2d158 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -89,6 +89,8 @@ obj-$(CONFIG_GENERIC_LIB_MULDI3)  += muldi3.o
 pbl-$(CONFIG_GENERIC_LIB_ASHLDI3) += ashldi3.o
 
 UBSAN_SANITIZE_ubsan.o := n
+KASAN_SANITIZE_ubsan.o := n
+CFLAGS_ubsan.o := -fno-stack-protector
 
 libfdt_files = fdt.o fdt_ro.o fdt_wip.o fdt_rw.o fdt_sw.o fdt_strerror.o \
 	                      fdt_empty_tree.o
-- 
2.39.2

[PATCH 2/5] lib: random: don't duplicate error/warning prefix in log message

[Ahmad Fatoum]

pr_err and pr_warn already print an ERROR/WARNING prefix, so no need to
duplicate it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 lib/random.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/random.c b/lib/random.c
index c6532df55249..e83935d0e17c 100644
--- a/lib/random.c
+++ b/lib/random.c
@@ -68,11 +68,11 @@ int get_crypto_bytes(void *buf, int len)
 	}
 
 	if (!IS_ENABLED(CONFIG_ALLOW_PRNG_FALLBACK)) {
-		pr_err("error: no HWRNG available!\n");
+		pr_err("no HWRNG available!\n");
 		return err;
 	}
 
-	pr_warn("warning: falling back to Pseudo RNG source!\n");
+	pr_warn("falling back to Pseudo RNG source!\n");
 
 	get_random_bytes(buf, len);
 
-- 
2.39.2

[PATCH 3/5] lib: stackprot: improve error message on missing HWRNG

[Ahmad Fatoum]

ERROR: no HWRNG available!
WARNING: stackprot: proceeding without randomized stack protector

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 lib/stackprot.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/stackprot.c b/lib/stackprot.c
index aa0e88603aae..c1cc19aadd09 100644
--- a/lib/stackprot.c
+++ b/lib/stackprot.c
@@ -1,5 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
-#include <printk.h>
+
+#define pr_fmt(fmt) "stackprot: " fmt
+
+#include <linux/printk.h>
 #include <linux/kernel.h>
 #include <linux/export.h>
 #include <init.h>
@@ -27,6 +30,11 @@ EXPORT_SYMBOL(__stack_chk_fail);
 
 static __no_stack_protector int stackprot_randomize_guard(void)
 {
-	return get_crypto_bytes(&__stack_chk_guard, sizeof(__stack_chk_guard));
+	int ret;
+
+	ret = get_crypto_bytes(&__stack_chk_guard, sizeof(__stack_chk_guard));
+	if (ret)
+		pr_warn("proceeding without randomized stack protector\n");
+	return 0;
 }
 late_initcall(stackprot_randomize_guard);
-- 
2.39.2

[PATCH 4/5] lib: stackprot: don't directly write stack protector from HWRNG driver

[Ahmad Fatoum]

get_crypto_bytes itself or some function it calls down to the driver may
require a stack protector, so passing the address of the stack protector
value down may end up tripping the stack protector during function
return.

To avoid this, let's write the stack protector in a function
chain that eithr has stack protector disabled or that never returns.

This fixes a crash using the virtio RNG driver to generate the stack
protector.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 lib/stackprot.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/stackprot.c b/lib/stackprot.c
index c1cc19aadd09..7a8d0a4c1064 100644
--- a/lib/stackprot.c
+++ b/lib/stackprot.c
@@ -16,7 +16,7 @@
 
 void __stack_chk_fail(void);
 
-unsigned long __stack_chk_guard = (unsigned long)(0xfeedf00ddeadbeef & ~0UL);
+volatile ulong __stack_chk_guard = (ulong)(0xfeedf00ddeadbeef & ~0UL);
 
 /*
  * Called when gcc's -fstack-protector feature is used, and
@@ -30,11 +30,15 @@ EXPORT_SYMBOL(__stack_chk_fail);
 
 static __no_stack_protector int stackprot_randomize_guard(void)
 {
+	ulong chk_guard;
 	int ret;
 
-	ret = get_crypto_bytes(&__stack_chk_guard, sizeof(__stack_chk_guard));
+	ret = get_crypto_bytes(&chk_guard, sizeof(chk_guard));
 	if (ret)
 		pr_warn("proceeding without randomized stack protector\n");
+	else
+		__stack_chk_guard = chk_guard;
+
 	return 0;
 }
 late_initcall(stackprot_randomize_guard);
-- 
2.39.2

[PATCH 5/5] lib: stackprot: hide symbols when not applicable

[Ahmad Fatoum]

Asking all users about the stacksmash command that's just there to test
stack guard and protector is unnecessary noise. Likewise asking about
PBL stackprotector, when we don't have any.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/Kconfig      | 1 +
 lib/Kconfig.hardening | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/commands/Kconfig b/commands/Kconfig
index c1bba22443e6..a6806f198ec4 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -2403,6 +2403,7 @@ config CMD_UBSAN
 
 config CMD_STACKSMASH
 	tristate "stacksmash"
+	depends on STACKPROTECTOR || STACK_GUARD_PAGE || COMPILE_TEST
 	help
 	  This commands trashes the stack to test stackprotector and
 	  guard page. This command does not return.
diff --git a/lib/Kconfig.hardening b/lib/Kconfig.hardening
index a9d3af110958..f14b256a7d91 100644
--- a/lib/Kconfig.hardening
+++ b/lib/Kconfig.hardening
@@ -61,7 +61,7 @@ config STACKPROTECTOR_ALL
 endchoice
 
 choice
-	prompt "Stack Protector buffer overflow detection for PBL"
+	prompt "Stack Protector buffer overflow detection for PBL" if PBL_IMAGE
 
 config PBL_STACKPROTECTOR_NONE
 	bool "None"
@@ -69,6 +69,7 @@ config PBL_STACKPROTECTOR_NONE
 config PBL_STACKPROTECTOR_STRONG
 	bool "Strong"
 	depends on $(cc-option,-fstack-protector-strong)
+	depends on PBL_IMAGE
 	select STACKPROTECTOR
 	help
 	  For PBL, This option turns on the "stack-protector" GCC feature. This
@@ -93,7 +94,7 @@ config PBL_STACKPROTECTOR_STRONG
 config PBL_STACKPROTECTOR_ALL
 	bool "PBL"
 	depends on $(cc-option,-fstack-protector-strong)
-	depends on COMPILE_TEST
+	depends on PBL_IMAGE && COMPILE_TEST
 	select STACKPROTECTOR
 	help
 	  This pushes and verifies stack protector canaries on all functions,
-- 
2.39.2

Re: [PATCH 1/5] lib: ubsan: disable sanitization for UBSAN implementation

[Sascha Hauer]

On Mon, Oct 09, 2023 at 01:52:35PM +0200, Ahmad Fatoum wrote:
> As done in Linux, the implementation of ubsan itself should not be
> instrumented by KASAN and stack protector.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  lib/Makefile | 2 ++
>  1 file changed, 2 insertions(+)

Applied, thanks

Sascha

> 
> diff --git a/lib/Makefile b/lib/Makefile
> index 2b577becc444..791080b2d158 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -89,6 +89,8 @@ obj-$(CONFIG_GENERIC_LIB_MULDI3)  += muldi3.o
>  pbl-$(CONFIG_GENERIC_LIB_ASHLDI3) += ashldi3.o
>  
>  UBSAN_SANITIZE_ubsan.o := n
> +KASAN_SANITIZE_ubsan.o := n
> +CFLAGS_ubsan.o := -fno-stack-protector
>  
>  libfdt_files = fdt.o fdt_ro.o fdt_wip.o fdt_rw.o fdt_sw.o fdt_strerror.o \
>  	                      fdt_empty_tree.o
> -- 
> 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 |