All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add support for stack-protector
@ 2021-01-10 15:39 Joel Peshkin
  2021-01-10 16:18 ` Heinrich Schuchardt
                   ` (9 more replies)
  0 siblings, 10 replies; 29+ messages in thread
From: Joel Peshkin @ 2021-01-10 15:39 UTC (permalink / raw)
  To: u-boot

Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Usama Arif <usama.arif@arm.com>
Cc: Sam Protsenko <joe.skb7@gmail.com>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Philippe Reynes <philippe.reynes@softathome.com>
Cc: Eugeniu Rosca <roscaeugeniu@gmail.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>

Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>

---

 Makefile             |  4 ++++
 common/Kconfig       | 15 +++++++++++++++
 common/Makefile      |  2 ++
 common/stackprot.c   | 17 +++++++++++++++++
 scripts/Makefile.spl |  6 ++++++
 5 files changed, 44 insertions(+)
 create mode 100644 common/stackprot.c

diff --git a/Makefile b/Makefile
index 3ee4cc00dd..6e7a81ec7d 100644
--- a/Makefile
+++ b/Makefile
@@ -677,7 +677,11 @@ else
 KBUILD_CFLAGS	+= -O2
 endif
 
+ifeq ($(CONFIG_STACKPROTECTOR),y)
+KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
+else
 KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
+endif
 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
 
 # disable stringop warnings in gcc 8+
diff --git a/common/Kconfig b/common/Kconfig
index 2bce8c9ba1..e30c3c4ab8 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -595,6 +595,21 @@ config TPL_HASH
 	  and the algorithms it supports are defined in common/hash.c. See
 	  also CMD_HASH for command-line access.
 
+config STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection"
+	default n
+	help
+	  Enable stack smash detection through gcc built-in stack-protector
+	  canary logic
+
+config SPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for SPL"
+	default n
+
+config TPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for SPL"
+	default n
+
 endmenu
 
 menu "Update support"
diff --git a/common/Makefile b/common/Makefile
index bcf352d016..fe71e18317 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
 obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
 
 obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
+obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
+
diff --git a/common/stackprot.c b/common/stackprot.c
new file mode 100644
index 0000000000..7c95b8544f
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard = 0xfeedf00ddeadbeef;
+
+void __stack_chk_fail(void)
+{
+	panic("Stack smashing detected in function: %p relocated from %p",
+	      __builtin_return_address(0),
+	      __builtin_return_address(0) - gd->reloc_off);
+}
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 9f1f7445d7..1505e4e851 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -63,6 +63,12 @@ include $(srctree)/scripts/Makefile.lib
 KBUILD_CFLAGS += -ffunction-sections -fdata-sections
 LDFLAGS_FINAL += --gc-sections
 
+ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
+KBUILD_CFLAGS += -fstack-protector-strong
+else
+KBUILD_CFLAGS += -fno-stack-protector
+endif
+
 # FIX ME
 cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
 							$(NOSTDINC_FLAGS)
-- 
2.27.0

^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH] Add support for stack-protector
  2021-01-10 15:39 [PATCH] Add support for stack-protector Joel Peshkin
@ 2021-01-10 16:18 ` Heinrich Schuchardt
  2021-01-10 19:44   ` Joel Peshkin
  2021-01-10 22:40 ` Alex Sadovsky
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-01-10 16:18 UTC (permalink / raw)
  To: u-boot

On 1/10/21 4:39 PM, Joel Peshkin wrote:
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Usama Arif <usama.arif@arm.com>
> Cc: Sam Protsenko <joe.skb7@gmail.com>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Philippe Reynes <philippe.reynes@softathome.com>
> Cc: Eugeniu Rosca <roscaeugeniu@gmail.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
>
> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
>
> ---
>
>   Makefile             |  4 ++++
>   common/Kconfig       | 15 +++++++++++++++
>   common/Makefile      |  2 ++
>   common/stackprot.c   | 17 +++++++++++++++++
>   scripts/Makefile.spl |  6 ++++++
>   5 files changed, 44 insertions(+)
>   create mode 100644 common/stackprot.c
>
> diff --git a/Makefile b/Makefile
> index 3ee4cc00dd..6e7a81ec7d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -677,7 +677,11 @@ else
>   KBUILD_CFLAGS	+= -O2
>   endif
>
> +ifeq ($(CONFIG_STACKPROTECTOR),y)
> +KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
> +else
>   KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
> +endif
>   KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
>
>   # disable stringop warnings in gcc 8+
> diff --git a/common/Kconfig b/common/Kconfig
> index 2bce8c9ba1..e30c3c4ab8 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -595,6 +595,21 @@ config TPL_HASH
>   	  and the algorithms it supports are defined in common/hash.c. See
>   	  also CMD_HASH for command-line access.
>
> +config STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection"
> +	default n
> +	help
> +	  Enable stack smash detection through gcc built-in stack-protector
> +	  canary logic
> +
> +config SPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for SPL"
> +	default n
> +
> +config TPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for SPL"

%s/SPL/TPL/

> +	default n
> +
>   endmenu
>
>   menu "Update support"
> diff --git a/common/Makefile b/common/Makefile
> index bcf352d016..fe71e18317 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
>   obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
>
>   obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
> +obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
> +
> diff --git a/common/stackprot.c b/common/stackprot.c
> new file mode 100644
> index 0000000000..7c95b8544f
> --- /dev/null
> +++ b/common/stackprot.c
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright 2021 Broadcom
> + */
> +
> +#include <common.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +unsigned long __stack_chk_guard = 0xfeedf00ddeadbeef;
> +
> +void __stack_chk_fail(void)

The standalone EFI binaries are compiled with -fstack-protector-strong
when selecting CONFIG_STACKPROTECTOR.

Do we need a function __stack_chk_fail) in
lib/efi_selftest/efi_freestanding.c and
lib/efi_loader/efi_freestanding.c too?

Could you, please, provide unit tests demonstrating that the stack
protection is actually working SPL, main U-Boot, and the EFI binaries.

Best regards

Heinrich

> +{
> +	panic("Stack smashing detected in function: %p relocated from %p",
> +	      __builtin_return_address(0),
> +	      __builtin_return_address(0) - gd->reloc_off);
> +}
> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index 9f1f7445d7..1505e4e851 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -63,6 +63,12 @@ include $(srctree)/scripts/Makefile.lib
>   KBUILD_CFLAGS += -ffunction-sections -fdata-sections
>   LDFLAGS_FINAL += --gc-sections
>
> +ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
> +KBUILD_CFLAGS += -fstack-protector-strong
> +else
> +KBUILD_CFLAGS += -fno-stack-protector
> +endif
> +
>   # FIX ME
>   cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
>   							$(NOSTDINC_FLAGS)
>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH] Add support for stack-protector
  2021-01-10 16:18 ` Heinrich Schuchardt
@ 2021-01-10 19:44   ` Joel Peshkin
  2021-01-10 22:20     ` Heinrich Schuchardt
  0 siblings, 1 reply; 29+ messages in thread
From: Joel Peshkin @ 2021-01-10 19:44 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

  Thank you for your comments.  I have 2 questions about how to proceed....

1) Unit test
  I can add a function that can be used to trigger an overrun, but I am not
sure where to include it as the stack protector prints the error message
and then resets uboot so it wouldn't fir in a unit test suite.

  I could add a CONFIG_STACKPROTECTOR_TEST_FAIL to add a
"test_stackprotector fail" command to the CLI and you could call the
underlying stackprot_test_fail() from code hacked into SPL and TPL

2) Standalone/EFI
    What we did for our own standalone code was to add the KBUILD_CFLAGS +=
-fno-stack-protector   to the Makefile for our specific standalone.   The
problem is there is no generic place from which all standalone/EFI is
called, so I could just leave this for maintainers of specific
standalone/EPI programs to add IF they are enabling STACKPROTECTOR (If they
don't enable it, they don't need to do anything) or I could add
KBUILD_CFLAGS += -fno-stack-protector  to  both lib/efi_setlftest/Makefile
and lib/efi_loader/Makefile

What would you suggest?

Regards,

Joel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4166 bytes
Desc: S/MIME Cryptographic Signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210110/ade01a9b/attachment.bin>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH] Add support for stack-protector
  2021-01-10 19:44   ` Joel Peshkin
@ 2021-01-10 22:20     ` Heinrich Schuchardt
  0 siblings, 0 replies; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-01-10 22:20 UTC (permalink / raw)
  To: u-boot

Am 10. Januar 2021 20:44:15 MEZ schrieb Joel Peshkin <joel.peshkin@broadcom.com>:
>Hi Heinrich,
>
>Thank you for your comments.  I have 2 questions about how to
>proceed....
>
>1) Unit test
>I can add a function that can be used to trigger an overrun, but I am
>not
>sure where to include it as the stack protector prints the error
>message
>and then resets uboot so it wouldn't fir in a unit test suite.
>
>  I could add a CONFIG_STACKPROTECTOR_TEST_FAIL to add a
>"test_stackprotector fail" command to the CLI and you could call the
>underlying stackprot_test_fail() from code hacked into SPL and TPL

Additonally to the test command you will nedd a Python test (in /test/py/tests/) to excercise it.


>
>2) Standalone/EFI
>What we did for our own standalone code was to add the KBUILD_CFLAGS +=
>-fno-stack-protector   to the Makefile for our specific standalone.  
>The
>problem is there is no generic place from which all standalone/EFI is
>called, so I could just leave this for maintainers of specific
>standalone/EPI programs to add IF they are enabling STACKPROTECTOR (If
>they
>don't enable it, they don't need to do anything) or I could add
>KBUILD_CFLAGS += -fno-stack-protector  to  both

This would lead to contradictory arguments on the GCC command line.

>lib/efi_setlftest/Makefile
>and lib/efi_loader/Makefile

Have a look at CFLAGS_REMOVE in aforementioned Makefiles. 

Best regards

Heinrich

>
>What would you suggest?
>
>Regards,
>
>Joel

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH] Add support for stack-protector
  2021-01-10 15:39 [PATCH] Add support for stack-protector Joel Peshkin
  2021-01-10 16:18 ` Heinrich Schuchardt
@ 2021-01-10 22:40 ` Alex Sadovsky
  2021-01-11  0:23   ` Joel Peshkin
  2021-01-11  3:10 ` [PATCH v2] " Joel Peshkin
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Alex Sadovsky @ 2021-01-10 22:40 UTC (permalink / raw)
  To: u-boot

Hi,
> +
> +unsigned long __stack_chk_guard = 0xfeedf00ddeadbeef;

sizeof(unsigned long) isn't always 8, even gcc issues a warning when it's invoked with proper options (e.g. 32-bit build):

> warning: conversion from ?long long unsigned int? to ?long unsigned int? changes value from ?18369602397475290863? to ?3735928559? [-Woverflow]

Maybe there's some better way to initialize this variable. E.g. with #if ? #else ? #endif or using some initialization function that is invoked early.
I should also mention that a fixed canary value doesn't actually bring proper protection against exploits, thus run-time initialization with a random value is usually preferred.

I'm not sure whether it's important at all in bootloader code, I just wanted to be sure that it isn't unnoticed.

Cheers, Alex.

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH] Add support for stack-protector
  2021-01-10 22:40 ` Alex Sadovsky
@ 2021-01-11  0:23   ` Joel Peshkin
  0 siblings, 0 replies; 29+ messages in thread
From: Joel Peshkin @ 2021-01-11  0:23 UTC (permalink / raw)
  To: u-boot

Hi Alex,

   Yeah, I think I'll wind up with some ifdef code for the static init.
 In the case of arm (32-bit), there is actually a GCC bug that causes it to
use the address of the canary value instead of the canary value itself.
GCC upstream just fixed that a few days ago, but it may be a year or so
before the appropriate GCC version is widely available.

    I may eventually add an optional mechanism to allow the value to be
changed (very carefully) at runtime.  This has to be done early enough that
we cannot wait for an RNG to be identified via DTB, but there are a few
ways to keep arm and aarch64 from being too predictable and some boards may
have peripherals that can provide a sufficiently variable value.

-Joel




On Sun, Jan 10, 2021 at 2:40 PM Alex Sadovsky <
nable.maininbox@googlemail.com> wrote:

> Hi,
> > +
> > +unsigned long __stack_chk_guard = 0xfeedf00ddeadbeef;
>
> sizeof(unsigned long) isn't always 8, even gcc issues a warning when it's
> invoked with proper options (e.g. 32-bit build):
>
> > warning: conversion from ?long long unsigned int? to ?long unsigned int?
> changes value from ?18369602397475290863? to ?3735928559? [-Woverflow]
>
> Maybe there's some better way to initialize this variable. E.g. with #if ?
> #else ? #endif or using some initialization function that is invoked early.
> I should also mention that a fixed canary value doesn't actually bring
> proper protection against exploits, thus run-time initialization with a
> random value is usually preferred.
>
> I'm not sure whether it's important at all in bootloader code, I just
> wanted to be sure that it isn't unnoticed.
>
> Cheers, Alex.
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4166 bytes
Desc: S/MIME Cryptographic Signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210110/47ae31cf/attachment.bin>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v2] Add support for stack-protector
  2021-01-10 15:39 [PATCH] Add support for stack-protector Joel Peshkin
  2021-01-10 16:18 ` Heinrich Schuchardt
  2021-01-10 22:40 ` Alex Sadovsky
@ 2021-01-11  3:10 ` Joel Peshkin
  2021-01-11  9:59   ` Heinrich Schuchardt
  2021-01-11 16:20 ` [PATCH v3] " Joel Peshkin
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Joel Peshkin @ 2021-01-11  3:10 UTC (permalink / raw)
  To: u-boot

Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Usama Arif <usama.arif@arm.com>
Cc: Sam Protsenko <joe.skb7@gmail.com>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Philippe Reynes <philippe.reynes@softathome.com>
Cc: Eugeniu Rosca <roscaeugeniu@gmail.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>

Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>

Changes for v2:
   - Add test command and corresponding config
   - Fixed incorrect description in Kconfig
   - Add unit test
---
 Makefile                             |  4 +++
 common/Kconfig                       | 22 ++++++++++++++
 common/Makefile                      |  2 ++
 common/stackprot.c                   | 44 ++++++++++++++++++++++++++++
 configs/sandbox_defconfig            |  2 ++
 scripts/Makefile.spl                 |  6 ++++
 test/py/tests/test_stackprotector.py | 15 ++++++++++
 7 files changed, 95 insertions(+)
 create mode 100644 common/stackprot.c
 create mode 100644 test/py/tests/test_stackprotector.py

diff --git a/Makefile b/Makefile
index 3ee4cc00dd..6e7a81ec7d 100644
--- a/Makefile
+++ b/Makefile
@@ -677,7 +677,11 @@ else
 KBUILD_CFLAGS	+= -O2
 endif
 
+ifeq ($(CONFIG_STACKPROTECTOR),y)
+KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
+else
 KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
+endif
 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
 
 # disable stringop warnings in gcc 8+
diff --git a/common/Kconfig b/common/Kconfig
index 2bce8c9ba1..f773babd3a 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -595,6 +595,28 @@ config TPL_HASH
 	  and the algorithms it supports are defined in common/hash.c. See
 	  also CMD_HASH for command-line access.
 
+config STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection"
+	default n
+	help
+	  Enable stack smash detection through gcc built-in stack-protector
+	  canary logic
+
+config STACKPROTECTOR_TEST_COMMAND
+	bool "Test command for stack protector"
+	depends on STACKPROTECTOR
+	default n
+	help
+	  Enable stackprot_test command
+
+config SPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for SPL"
+	default n
+
+config TPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for TPL"
+	default n
+
 endmenu
 
 menu "Update support"
diff --git a/common/Makefile b/common/Makefile
index bcf352d016..fe71e18317 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
 obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
 
 obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
+obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
+
diff --git a/common/stackprot.c b/common/stackprot.c
new file mode 100644
index 0000000000..d602dc7de1
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+#include <cli.h>
+#include <command.h>
+#include <console.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard =
+    (unsigned long)((unsigned long long)0xfeedf00ddeadbeef &
+		    ~((unsigned long)0));
+
+void __stack_chk_fail(void)
+{
+	panic("Stack smashing detected in function: %p relocated from %p",
+	      __builtin_return_address(0),
+	      __builtin_return_address(0) - gd->reloc_off);
+}
+
+#ifdef CONFIG_STACKPROTECTOR_TEST_COMMAND
+static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+				  char *const argv[]);
+
+int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+				  char *const argv[])
+{
+	char a[128];
+	char b[256];
+	int i;
+	for (i = 0; i < 256; i++) {
+		b[i] = i;
+	}
+	memcpy(a, b, 512);
+	return (0);
+}
+
+U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
+	   "test stack protector fail", "");
+
+#endif
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index f1ec701a9f..da3aca2551 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -274,3 +274,5 @@ CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
+CONFIG_STACKPROTECTOR=y
+CONFIG_STACKPROTECTOR_TEST_COMMAND=y
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 9f1f7445d7..1505e4e851 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -63,6 +63,12 @@ include $(srctree)/scripts/Makefile.lib
 KBUILD_CFLAGS += -ffunction-sections -fdata-sections
 LDFLAGS_FINAL += --gc-sections
 
+ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
+KBUILD_CFLAGS += -fstack-protector-strong
+else
+KBUILD_CFLAGS += -fno-stack-protector
+endif
+
 # FIX ME
 cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
 							$(NOSTDINC_FLAGS)
diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
new file mode 100644
index 0000000000..49df8eff8c
--- /dev/null
+++ b/test/py/tests/test_stackprotector.py
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Broadcom 
+
+import pytest
+import signal
+
+@pytest.mark.buildconfigspec('stackprotector_test_command')
+def test_stackprotector(u_boot_console):
+    """Test that the stackprotector function works."""
+
+    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
+    expected_response = 'Stack smashing detected'
+    u_boot_console.wait_for(expected_response)
+    assert(True)
+
-- 
2.27.0

^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH v2] Add support for stack-protector
  2021-01-11  3:10 ` [PATCH v2] " Joel Peshkin
@ 2021-01-11  9:59   ` Heinrich Schuchardt
  0 siblings, 0 replies; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-01-11  9:59 UTC (permalink / raw)
  To: u-boot

On 11.01.21 04:10, Joel Peshkin wrote:
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Usama Arif <usama.arif@arm.com>
> Cc: Sam Protsenko <joe.skb7@gmail.com>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Philippe Reynes <philippe.reynes@softathome.com>
> Cc: Eugeniu Rosca <roscaeugeniu@gmail.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
>
> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
>
> Changes for v2:
>    - Add test command and corresponding config
>    - Fixed incorrect description in Kconfig
>    - Add unit test
> ---
>  Makefile                             |  4 +++
>  common/Kconfig                       | 22 ++++++++++++++
>  common/Makefile                      |  2 ++
>  common/stackprot.c                   | 44 ++++++++++++++++++++++++++++
>  configs/sandbox_defconfig            |  2 ++
>  scripts/Makefile.spl                 |  6 ++++
>  test/py/tests/test_stackprotector.py | 15 ++++++++++
>  7 files changed, 95 insertions(+)
>  create mode 100644 common/stackprot.c
>  create mode 100644 test/py/tests/test_stackprotector.py
>
> diff --git a/Makefile b/Makefile
> index 3ee4cc00dd..6e7a81ec7d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -677,7 +677,11 @@ else
>  KBUILD_CFLAGS	+= -O2
>  endif
>
> +ifeq ($(CONFIG_STACKPROTECTOR),y)
> +KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
> +else
>  KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
> +endif
>  KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
>
>  # disable stringop warnings in gcc 8+
> diff --git a/common/Kconfig b/common/Kconfig
> index 2bce8c9ba1..f773babd3a 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -595,6 +595,28 @@ config TPL_HASH
>  	  and the algorithms it supports are defined in common/hash.c. See
>  	  also CMD_HASH for command-line access.
>
> +config STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection"
> +	default n
> +	help
> +	  Enable stack smash detection through gcc built-in stack-protector
> +	  canary logic
> +
> +config STACKPROTECTOR_TEST_COMMAND

This should be called CMD_STACKPROTECTOR_TEST cmd/Kconfig.

> +	bool "Test command for stack protector"
> +	depends on STACKPROTECTOR
> +	default n
> +	help
> +	  Enable stackprot_test command
> +
> +config SPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for SPL"

depends on SPL && STACKPROTECTOR

> +	default n
> +
> +config TPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for TPL"

depends on TPL && STACKPROTECTOR

> +	default n
> +
>  endmenu
>
>  menu "Update support"
> diff --git a/common/Makefile b/common/Makefile
> index bcf352d016..fe71e18317 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
>  obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
>
>  obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
> +obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
> +
> diff --git a/common/stackprot.c b/common/stackprot.c
> new file mode 100644
> index 0000000000..d602dc7de1
> --- /dev/null
> +++ b/common/stackprot.c
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright 2021 Broadcom
> + */
> +
> +#include <common.h>
> +#include <cli.h>
> +#include <command.h>
> +#include <console.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +unsigned long __stack_chk_guard =
> +    (unsigned long)((unsigned long long)0xfeedf00ddeadbeef &
> +		    ~((unsigned long)0));

Please, use scripts/checkpatch.pl and correct the reported problems, e.g.

WARNING: please, no spaces at the start of a line
#115: FILE: common/stackprot.c:14:
+    (unsigned long)((unsigned long long)0xfeedf00ddeadbeef &$

WARNING: Unnecessary typecast of c90 int constant
#115: FILE: common/stackprot.c:14:
+    (unsigned long)((unsigned long long)0xfeedf00ddeadbeef &

WARNING: Unnecessary typecast of c90 int constant
#116: FILE: common/stackprot.c:15:
+                   ~((unsigned long)0));

> +
> +void __stack_chk_fail(void)
> +{
> +	panic("Stack smashing detected in function: %p relocated from %p",
> +	      __builtin_return_address(0),
> +	      __builtin_return_address(0) - gd->reloc_off);
> +}
> +
> +#ifdef CONFIG_STACKPROTECTOR_TEST_COMMAND

The command should be in directory /cmd/. There you can use the Makefile
to decide if it is compiled.

Please, add a short man-page to /doc/usage/. See doc/usage/button.rst
for a template.

> +static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
> +				  char *const argv[]);
> +
> +int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
> +				  char *const argv[])
> +{
> +	char a[128];
> +	char b[256];
> +	int i;

Missing empty line.

> +	for (i = 0; i < 256; i++) {
> +		b[i] = i;
> +	}
> +	memcpy(a, b, 512);

What is b[] good for? The following seems to be enough to do the test:

{
	char a[128];

	memset(a, 0xff, 512);
	return;
}

Best regards

Heinrich

> +	return (0);
> +}
> +
> +U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
> +	   "test stack protector fail", "");
> +
> +#endif
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index f1ec701a9f..da3aca2551 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -274,3 +274,5 @@ CONFIG_TEST_FDTDEC=y
>  CONFIG_UNIT_TEST=y
>  CONFIG_UT_TIME=y
>  CONFIG_UT_DM=y
> +CONFIG_STACKPROTECTOR=y
> +CONFIG_STACKPROTECTOR_TEST_COMMAND=y
> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index 9f1f7445d7..1505e4e851 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -63,6 +63,12 @@ include $(srctree)/scripts/Makefile.lib
>  KBUILD_CFLAGS += -ffunction-sections -fdata-sections
>  LDFLAGS_FINAL += --gc-sections
>
> +ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
> +KBUILD_CFLAGS += -fstack-protector-strong
> +else
> +KBUILD_CFLAGS += -fno-stack-protector
> +endif
> +
>  # FIX ME
>  cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
>  							$(NOSTDINC_FLAGS)
> diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
> new file mode 100644
> index 0000000000..49df8eff8c
> --- /dev/null
> +++ b/test/py/tests/test_stackprotector.py
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Broadcom
> +
> +import pytest
> +import signal
> +
> + at pytest.mark.buildconfigspec('stackprotector_test_command')
> +def test_stackprotector(u_boot_console):
> +    """Test that the stackprotector function works."""
> +
> +    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
> +    expected_response = 'Stack smashing detected'
> +    u_boot_console.wait_for(expected_response)
> +    assert(True)
> +
>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v3] Add support for stack-protector
  2021-01-10 15:39 [PATCH] Add support for stack-protector Joel Peshkin
                   ` (2 preceding siblings ...)
  2021-01-11  3:10 ` [PATCH v2] " Joel Peshkin
@ 2021-01-11 16:20 ` Joel Peshkin
  2021-01-11 18:12   ` Heinrich Schuchardt
  2021-01-11 22:49 ` [PATCH v4] " Joel Peshkin
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Joel Peshkin @ 2021-01-11 16:20 UTC (permalink / raw)
  To: u-boot

Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Usama Arif <usama.arif@arm.com>
Cc: Sam Protsenko <joe.skb7@gmail.com>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Philippe Reynes <philippe.reynes@softathome.com>
Cc: Eugeniu Rosca <roscaeugeniu@gmail.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>

Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>

Changes for v3:
   - Move test command to cmd/
   - Update Kconfig names and depends
   - clean up default canary initialization

Changes for v2:
   - Add test command and corresponding config
   - Fixed incorrect description in Kconfig
   - Add unit test
---
 MAINTAINERS                          |  7 +++++++
 Makefile                             |  4 ++++
 cmd/Kconfig                          | 10 ++++++++++
 cmd/Makefile                         |  1 +
 cmd/stackprot_test.c                 | 26 ++++++++++++++++++++++++++
 common/Kconfig                       | 17 +++++++++++++++++
 common/Makefile                      |  2 ++
 common/stackprot.c                   | 18 ++++++++++++++++++
 configs/sandbox_defconfig            |  2 ++
 scripts/Makefile.spl                 |  6 ++++++
 test/py/tests/test_stackprotector.py | 15 +++++++++++++++
 11 files changed, 108 insertions(+)
 create mode 100644 cmd/stackprot_test.c
 create mode 100644 common/stackprot.c
 create mode 100644 test/py/tests/test_stackprotector.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 52d7307525..2982ffcc07 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1022,6 +1022,13 @@ F:	include/sqfs.h
 F:	cmd/sqfs.c
 F:	test/py/tests/test_fs/test_squashfs/
 
+STACKPROTECTOR
+M:	Joel Peshkin <joel.peshkin@broadcom.com>
+S:	Maintained
+F:	common/stackprot.c
+F:	cmd/stackprot_test.c
+F:	test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M:	Bharat Gooty <bharat.gooty@broadcom.com>
 M:	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
diff --git a/Makefile b/Makefile
index 3ee4cc00dd..6e7a81ec7d 100644
--- a/Makefile
+++ b/Makefile
@@ -677,7 +677,11 @@ else
 KBUILD_CFLAGS	+= -O2
 endif
 
+ifeq ($(CONFIG_STACKPROTECTOR),y)
+KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
+else
 KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
+endif
 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
 
 # disable stringop warnings in gcc 8+
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 1595de999b..04700e4bb4 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2271,6 +2271,16 @@ config CMD_AVB
 	    avb read_part_hex - read data from partition and output to stdout
 	    avb write_part - write data to partition
 	    avb verify - run full verification chain
+
+config CMD_STACKPROTECTOR_TEST
+	bool "Test command for stack protector"
+	depends on STACKPROTECTOR
+	default n
+	help
+	  Enable stackprot_test command
+	  The stackprot_test command will force a stack overrun to test
+	  the stack smashing detection mechanisms.
+
 endmenu
 
 config CMD_UBI
diff --git a/cmd/Makefile b/cmd/Makefile
index dd86675bf2..8e8fcea19e 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
 obj-$(CONFIG_CMD_STRINGS) += strings.o
 obj-$(CONFIG_CMD_SMC) += smccc.o
 obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
+obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
 obj-$(CONFIG_CMD_TERMINAL) += terminal.o
 obj-$(CONFIG_CMD_TIME) += time.o
 obj-$(CONFIG_CMD_TIMER) += timer.o
diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
new file mode 100644
index 0000000000..ea952c075e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+#include <cli.h>
+#include <command.h>
+#include <console.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+				  char *const argv[]);
+
+int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+			   char *const argv[])
+{
+	char a[128];
+
+	memset(a, 0xa5, 512);
+	return 0;
+}
+
+U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
+	   "test stack protector fail", "");
diff --git a/common/Kconfig b/common/Kconfig
index 2bce8c9ba1..5a053c7336 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -595,6 +595,23 @@ config TPL_HASH
 	  and the algorithms it supports are defined in common/hash.c. See
 	  also CMD_HASH for command-line access.
 
+config STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection"
+	default n
+	help
+	  Enable stack smash detection through gcc built-in stack-protector
+	  canary logic
+
+config SPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for SPL"
+	depends on STACKPROTECTOR && SPL
+	default n
+
+config TPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for TPL"
+	depends on STACKPROTECTOR && TPL
+	default n
+
 endmenu
 
 menu "Update support"
diff --git a/common/Makefile b/common/Makefile
index bcf352d016..fe71e18317 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
 obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
 
 obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
+obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
+
diff --git a/common/stackprot.c b/common/stackprot.c
new file mode 100644
index 0000000000..014257b3e7
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+#include <console.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard = (long)(0xfeedf00ddeadbeef & ~0L);
+
+void __stack_chk_fail(void)
+{
+	panic("Stack smashing detected in function: %p relocated from %p",
+	      __builtin_return_address(0),
+	      __builtin_return_address(0) - gd->reloc_off);
+}
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index f1ec701a9f..3e92a58bdb 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -274,3 +274,5 @@ CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
+CONFIG_STACKPROTECTOR=y
+CONFIG_CMD_STACKPROTECTOR_TEST=y
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 9f1f7445d7..1505e4e851 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -63,6 +63,12 @@ include $(srctree)/scripts/Makefile.lib
 KBUILD_CFLAGS += -ffunction-sections -fdata-sections
 LDFLAGS_FINAL += --gc-sections
 
+ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
+KBUILD_CFLAGS += -fstack-protector-strong
+else
+KBUILD_CFLAGS += -fno-stack-protector
+endif
+
 # FIX ME
 cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
 							$(NOSTDINC_FLAGS)
diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
new file mode 100644
index 0000000000..957a2a6cf7
--- /dev/null
+++ b/test/py/tests/test_stackprotector.py
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Broadcom
+
+import pytest
+import signal
+
+ at pytest.mark.buildconfigspec('cmd_stackprotector_test')
+def test_stackprotector(u_boot_console):
+    """Test that the stackprotector function works."""
+
+    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
+    expected_response = 'Stack smashing detected'
+    u_boot_console.wait_for(expected_response)
+    assert(True)
+
-- 
2.27.0

^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH v3] Add support for stack-protector
  2021-01-11 16:20 ` [PATCH v3] " Joel Peshkin
@ 2021-01-11 18:12   ` Heinrich Schuchardt
  0 siblings, 0 replies; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-01-11 18:12 UTC (permalink / raw)
  To: u-boot

On 11.01.21 17:20, Joel Peshkin wrote:
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Usama Arif <usama.arif@arm.com>
> Cc: Sam Protsenko <joe.skb7@gmail.com>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Philippe Reynes <philippe.reynes@softathome.com>
> Cc: Eugeniu Rosca <roscaeugeniu@gmail.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
>
> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
>
> Changes for v3:
>    - Move test command to cmd/
>    - Update Kconfig names and depends
>    - clean up default canary initialization
>
> Changes for v2:
>    - Add test command and corresponding config
>    - Fixed incorrect description in Kconfig
>    - Add unit test
> ---
>  MAINTAINERS                          |  7 +++++++
>  Makefile                             |  4 ++++
>  cmd/Kconfig                          | 10 ++++++++++
>  cmd/Makefile                         |  1 +
>  cmd/stackprot_test.c                 | 26 ++++++++++++++++++++++++++
>  common/Kconfig                       | 17 +++++++++++++++++
>  common/Makefile                      |  2 ++
>  common/stackprot.c                   | 18 ++++++++++++++++++
>  configs/sandbox_defconfig            |  2 ++
>  scripts/Makefile.spl                 |  6 ++++++

Thanks for resubmitting.

I am missing the changes for lib/efi_loader/Makefile and
lib/efi_selftest/Makefile to keep the stack protector out of the EFI
binaries.

>  test/py/tests/test_stackprotector.py | 15 +++++++++++++++
>  11 files changed, 108 insertions(+)
>  create mode 100644 cmd/stackprot_test.c
>  create mode 100644 common/stackprot.c
>  create mode 100644 test/py/tests/test_stackprotector.py
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 52d7307525..2982ffcc07 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1022,6 +1022,13 @@ F:	include/sqfs.h
>  F:	cmd/sqfs.c
>  F:	test/py/tests/test_fs/test_squashfs/
>
> +STACKPROTECTOR
> +M:	Joel Peshkin <joel.peshkin@broadcom.com>
> +S:	Maintained
> +F:	common/stackprot.c
> +F:	cmd/stackprot_test.c
> +F:	test/py/tests/test_stackprotector.py
> +
>  TARGET_BCMNS3
>  M:	Bharat Gooty <bharat.gooty@broadcom.com>
>  M:	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> diff --git a/Makefile b/Makefile
> index 3ee4cc00dd..6e7a81ec7d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -677,7 +677,11 @@ else
>  KBUILD_CFLAGS	+= -O2
>  endif
>
> +ifeq ($(CONFIG_STACKPROTECTOR),y)
> +KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
> +else
>  KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
> +endif
>  KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
>
>  # disable stringop warnings in gcc 8+
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 1595de999b..04700e4bb4 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2271,6 +2271,16 @@ config CMD_AVB
>  	    avb read_part_hex - read data from partition and output to stdout
>  	    avb write_part - write data to partition
>  	    avb verify - run full verification chain
> +
> +config CMD_STACKPROTECTOR_TEST
> +	bool "Test command for stack protector"
> +	depends on STACKPROTECTOR
> +	default n
> +	help
> +	  Enable stackprot_test command
> +	  The stackprot_test command will force a stack overrun to test
> +	  the stack smashing detection mechanisms.
> +
>  endmenu
>
>  config CMD_UBI
> diff --git a/cmd/Makefile b/cmd/Makefile
> index dd86675bf2..8e8fcea19e 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
>  obj-$(CONFIG_CMD_STRINGS) += strings.o
>  obj-$(CONFIG_CMD_SMC) += smccc.o
>  obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
> +obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
>  obj-$(CONFIG_CMD_TERMINAL) += terminal.o
>  obj-$(CONFIG_CMD_TIME) += time.o
>  obj-$(CONFIG_CMD_TIMER) += timer.o
> diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
> new file mode 100644
> index 0000000000..ea952c075e
> --- /dev/null
> +++ b/cmd/stackprot_test.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright 2021 Broadcom
> + */
> +
> +#include <common.h>
> +#include <cli.h>

The include is not needed and should be removed.

> +#include <command.h>
> +#include <console.h>

superfluous include

> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
> +				  char *const argv[]);

superfluous forward declaration

> +
> +int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,

The static keyword can be added here.

Static is preferable. The only good thing about making it non-static
would be easy checking if the address output works. Looks good at first
sight:

Stack smashing detected for function:
00005591a897ea34 relocated from 000000000004ba34

u-boot.map:
0x000000000004b9ee do_test_stackprot_fail
0x000000000004befc print_byte_string

> +			   char *const argv[])
> +{
> +	char a[128];
> +
> +	memset(a, 0xa5, 512);
> +	return 0;
> +}
> +
> +U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
> +	   "test stack protector fail", "");
> diff --git a/common/Kconfig b/common/Kconfig
> index 2bce8c9ba1..5a053c7336 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -595,6 +595,23 @@ config TPL_HASH
>  	  and the algorithms it supports are defined in common/hash.c. See
>  	  also CMD_HASH for command-line access.
>
> +config STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection"
> +	default n
> +	help
> +	  Enable stack smash detection through gcc built-in stack-protector

We build U-Boot with Clang too. So I would prefer not to reference any
individual compiler here.

GCC would have to be capitalized.

> +	  canary logic
> +
> +config SPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for SPL"
> +	depends on STACKPROTECTOR && SPL
> +	default n
> +
> +config TPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for TPL"
> +	depends on STACKPROTECTOR && TPL
> +	default n
> +
>  endmenu
>
>  menu "Update support"
> diff --git a/common/Makefile b/common/Makefile
> index bcf352d016..fe71e18317 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
>  obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
>
>  obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
> +obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
> +
> diff --git a/common/stackprot.c b/common/stackprot.c
> new file mode 100644
> index 0000000000..014257b3e7
> --- /dev/null
> +++ b/common/stackprot.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright 2021 Broadcom
> + */
> +
> +#include <common.h>
> +#include <console.h>

superfluous include

> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +unsigned long __stack_chk_guard = (long)(0xfeedf00ddeadbeef & ~0L);
> +
> +void __stack_chk_fail(void)
> +{
> +	panic("Stack smashing detected in function: %p relocated from %p",

On a 64-bit system the current output does not fit into 80 characters.
Please add a newline after the colon.

> +	      __builtin_return_address(0),

Shouldn't you use

    __builtin_extract_return_addr(__builtin_return_address(0));

For instance on ARM6 condition codes may have to be masked out of the
return address.

Cf.
https://gcc.gnu.org/onlinedocs/gcc/Return-Address.html
https://github.com/gcc-mirror/gcc/blob/master/gcc/config/arm/arm.h#L2316

Best regards

Heinrich

> +	      __builtin_return_address(0) - gd->reloc_off);
> +}
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index f1ec701a9f..3e92a58bdb 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -274,3 +274,5 @@ CONFIG_TEST_FDTDEC=y
>  CONFIG_UNIT_TEST=y
>  CONFIG_UT_TIME=y
>  CONFIG_UT_DM=y
> +CONFIG_STACKPROTECTOR=y
> +CONFIG_CMD_STACKPROTECTOR_TEST=y
> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index 9f1f7445d7..1505e4e851 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -63,6 +63,12 @@ include $(srctree)/scripts/Makefile.lib
>  KBUILD_CFLAGS += -ffunction-sections -fdata-sections
>  LDFLAGS_FINAL += --gc-sections
>
> +ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
> +KBUILD_CFLAGS += -fstack-protector-strong
> +else
> +KBUILD_CFLAGS += -fno-stack-protector
> +endif
> +
>  # FIX ME
>  cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
>  							$(NOSTDINC_FLAGS)
> diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
> new file mode 100644
> index 0000000000..957a2a6cf7
> --- /dev/null
> +++ b/test/py/tests/test_stackprotector.py
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Broadcom
> +
> +import pytest
> +import signal
> +
> + at pytest.mark.buildconfigspec('cmd_stackprotector_test')
> +def test_stackprotector(u_boot_console):
> +    """Test that the stackprotector function works."""
> +
> +    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
> +    expected_response = 'Stack smashing detected'
> +    u_boot_console.wait_for(expected_response)
> +    assert(True)
> +
>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v4] Add support for stack-protector
  2021-01-10 15:39 [PATCH] Add support for stack-protector Joel Peshkin
                   ` (3 preceding siblings ...)
  2021-01-11 16:20 ` [PATCH v3] " Joel Peshkin
@ 2021-01-11 22:49 ` Joel Peshkin
  2021-01-12 15:48   ` Heinrich Schuchardt
  2021-01-11 23:55 ` [PATCH v5] " Joel Peshkin
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Joel Peshkin @ 2021-01-11 22:49 UTC (permalink / raw)
  To: u-boot

Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Usama Arif <usama.arif@arm.com>
Cc: Sam Protsenko <joe.skb7@gmail.com>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Philippe Reynes <philippe.reynes@softathome.com>
Cc: Eugeniu Rosca <roscaeugeniu@gmail.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>

Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>

Changes for v4:
   - Exclude EFI from stackprotector
   - Cleanups of extra includes and declaration

Changes for v3:
   - Move test command to cmd/
   - Update Kconfig names and depends
   - clean up default canary initialization

Changes for v2:
   - Add test command and corresponding config
   - Fixed incorrect description in Kconfig
   - Add unit test
---
 MAINTAINERS                          |  7 +++++++
 Makefile                             |  5 +++++
 cmd/Kconfig                          | 10 ++++++++++
 cmd/Makefile                         |  1 +
 cmd/stackprot_test.c                 | 21 +++++++++++++++++++++
 common/Kconfig                       | 17 +++++++++++++++++
 common/Makefile                      |  2 ++
 common/stackprot.c                   | 17 +++++++++++++++++
 configs/sandbox_defconfig            |  2 ++
 scripts/Makefile.spl                 |  6 ++++++
 test/py/tests/test_stackprotector.py | 15 +++++++++++++++
 11 files changed, 103 insertions(+)
 create mode 100644 cmd/stackprot_test.c
 create mode 100644 common/stackprot.c
 create mode 100644 test/py/tests/test_stackprotector.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 52d7307525..2982ffcc07 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1022,6 +1022,13 @@ F:	include/sqfs.h
 F:	cmd/sqfs.c
 F:	test/py/tests/test_fs/test_squashfs/
 
+STACKPROTECTOR
+M:	Joel Peshkin <joel.peshkin@broadcom.com>
+S:	Maintained
+F:	common/stackprot.c
+F:	cmd/stackprot_test.c
+F:	test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M:	Bharat Gooty <bharat.gooty@broadcom.com>
 M:	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
diff --git a/Makefile b/Makefile
index 3ee4cc00dd..8c04fcf96c 100644
--- a/Makefile
+++ b/Makefile
@@ -677,7 +677,12 @@ else
 KBUILD_CFLAGS	+= -O2
 endif
 
+ifeq ($(CONFIG_STACKPROTECTOR),y)
+KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
+CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
+else
 KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
+endif
 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
 
 # disable stringop warnings in gcc 8+
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 1595de999b..04700e4bb4 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2271,6 +2271,16 @@ config CMD_AVB
 	    avb read_part_hex - read data from partition and output to stdout
 	    avb write_part - write data to partition
 	    avb verify - run full verification chain
+
+config CMD_STACKPROTECTOR_TEST
+	bool "Test command for stack protector"
+	depends on STACKPROTECTOR
+	default n
+	help
+	  Enable stackprot_test command
+	  The stackprot_test command will force a stack overrun to test
+	  the stack smashing detection mechanisms.
+
 endmenu
 
 config CMD_UBI
diff --git a/cmd/Makefile b/cmd/Makefile
index dd86675bf2..8e8fcea19e 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
 obj-$(CONFIG_CMD_STRINGS) += strings.o
 obj-$(CONFIG_CMD_SMC) += smccc.o
 obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
+obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
 obj-$(CONFIG_CMD_TERMINAL) += terminal.o
 obj-$(CONFIG_CMD_TIME) += time.o
 obj-$(CONFIG_CMD_TIMER) += timer.o
diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
new file mode 100644
index 0000000000..6ad287e55f
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+#include <command.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+				  char *const argv[])
+{
+	char a[128];
+
+	memset(a, 0xa5, 512);
+	return 0;
+}
+
+U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
+	   "test stack protector fail", "");
diff --git a/common/Kconfig b/common/Kconfig
index 2bce8c9ba1..6a94045948 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -595,6 +595,23 @@ config TPL_HASH
 	  and the algorithms it supports are defined in common/hash.c. See
 	  also CMD_HASH for command-line access.
 
+config STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection"
+	default n
+	help
+	  Enable stack smash detection through compiler's stack-protector
+	  canary logic
+
+config SPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for SPL"
+	depends on STACKPROTECTOR && SPL
+	default n
+
+config TPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for TPL"
+	depends on STACKPROTECTOR && TPL
+	default n
+
 endmenu
 
 menu "Update support"
diff --git a/common/Makefile b/common/Makefile
index bcf352d016..fe71e18317 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
 obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
 
 obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
+obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
+
diff --git a/common/stackprot.c b/common/stackprot.c
new file mode 100644
index 0000000000..1f1c3b4273
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard = (long)(0xfeedf00ddeadbeef & ~0L);
+
+void __stack_chk_fail(void)
+{
+	panic("Stack smashing detected in function:\n%p relocated from %p",
+	      __builtin_extract_return_addr(__builtin_return_address(0)),
+	      __builtin_return_address(0) - gd->reloc_off);
+}
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index f1ec701a9f..3e92a58bdb 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -274,3 +274,5 @@ CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
+CONFIG_STACKPROTECTOR=y
+CONFIG_CMD_STACKPROTECTOR_TEST=y
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 9f1f7445d7..1505e4e851 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -63,6 +63,12 @@ include $(srctree)/scripts/Makefile.lib
 KBUILD_CFLAGS += -ffunction-sections -fdata-sections
 LDFLAGS_FINAL += --gc-sections
 
+ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
+KBUILD_CFLAGS += -fstack-protector-strong
+else
+KBUILD_CFLAGS += -fno-stack-protector
+endif
+
 # FIX ME
 cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
 							$(NOSTDINC_FLAGS)
diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
new file mode 100644
index 0000000000..957a2a6cf7
--- /dev/null
+++ b/test/py/tests/test_stackprotector.py
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Broadcom
+
+import pytest
+import signal
+
+ at pytest.mark.buildconfigspec('cmd_stackprotector_test')
+def test_stackprotector(u_boot_console):
+    """Test that the stackprotector function works."""
+
+    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
+    expected_response = 'Stack smashing detected'
+    u_boot_console.wait_for(expected_response)
+    assert(True)
+
-- 
2.27.0

^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH v5] Add support for stack-protector
  2021-01-10 15:39 [PATCH] Add support for stack-protector Joel Peshkin
                   ` (4 preceding siblings ...)
  2021-01-11 22:49 ` [PATCH v4] " Joel Peshkin
@ 2021-01-11 23:55 ` Joel Peshkin
  2021-01-12 16:51 ` [PATCH v6] " Joel Peshkin
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Joel Peshkin @ 2021-01-11 23:55 UTC (permalink / raw)
  To: u-boot

Cc: Joel Peshkin <joel.peshkin@broadcom.com>,
	Simon Glass <sjg@chromium.org>,
	Bin Meng <bmeng.cn@gmail.com>,
	Jagan Teki <jagan@amarulasolutions.com>,
	Kever Yang <kever.yang@rock-chips.com>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	AKASHI Takahiro <takahiro.akashi@linaro.org>,
	Usama Arif <usama.arif@arm.com>,
	Sam Protsenko <joe.skb7@gmail.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Philippe Reynes <philippe.reynes@softathome.com>,
	Eugeniu Rosca <roscaeugeniu@gmail.com>,
	Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>

Changes for v5:
   - Rebase
Changes for v4:
   - Exclude EFI from stackprotector
   - Cleanups of extra includes and declaration

Changes for v3:
   - Move test command to cmd/
   - Update Kconfig names and depends
   - clean up default canary initialization

Changes for v2:
   - Add test command and corresponding config
   - Fixed incorrect description in Kconfig
   - Add unit test
---
---
 MAINTAINERS                          |  7 +++++++
 Makefile                             |  5 +++++
 cmd/Kconfig                          | 10 ++++++++++
 cmd/Makefile                         |  1 +
 cmd/stackprot_test.c                 | 21 +++++++++++++++++++++
 common/Kconfig                       | 17 +++++++++++++++++
 common/Makefile                      |  2 ++
 common/stackprot.c                   | 17 +++++++++++++++++
 configs/sandbox_defconfig            | 14 +++++++-------
 scripts/Makefile.spl                 |  6 ++++++
 test/py/tests/test_stackprotector.py | 15 +++++++++++++++
 11 files changed, 108 insertions(+), 7 deletions(-)
 create mode 100644 cmd/stackprot_test.c
 create mode 100644 common/stackprot.c
 create mode 100644 test/py/tests/test_stackprotector.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 26dd254..d3971e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1024,6 +1024,13 @@ F:	include/sqfs.h
 F:	cmd/sqfs.c
 F:	test/py/tests/test_fs/test_squashfs/
 
+STACKPROTECTOR
+M:	Joel Peshkin <joel.peshkin@broadcom.com>
+S:	Maintained
+F:	common/stackprot.c
+F:	cmd/stackprot_test.c
+F:	test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M:	Bharat Gooty <bharat.gooty@broadcom.com>
 M:	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
diff --git a/Makefile b/Makefile
index 902a976..65c5cb8 100644
--- a/Makefile
+++ b/Makefile
@@ -677,7 +677,12 @@ else
 KBUILD_CFLAGS	+= -O2
 endif
 
+ifeq ($(CONFIG_STACKPROTECTOR),y)
+KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
+CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
+else
 KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
+endif
 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
 
 # disable stringop warnings in gcc 8+
diff --git a/cmd/Kconfig b/cmd/Kconfig
index da86a94..054b2f3 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2280,6 +2280,16 @@ config CMD_AVB
 	    avb read_part_hex - read data from partition and output to stdout
 	    avb write_part - write data to partition
 	    avb verify - run full verification chain
+
+config CMD_STACKPROTECTOR_TEST
+	bool "Test command for stack protector"
+	depends on STACKPROTECTOR
+	default n
+	help
+	  Enable stackprot_test command
+	  The stackprot_test command will force a stack overrun to test
+	  the stack smashing detection mechanisms.
+
 endmenu
 
 config CMD_UBI
diff --git a/cmd/Makefile b/cmd/Makefile
index 5b3400a..1d7afea 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
 obj-$(CONFIG_CMD_STRINGS) += strings.o
 obj-$(CONFIG_CMD_SMC) += smccc.o
 obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
+obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
 obj-$(CONFIG_CMD_TERMINAL) += terminal.o
 obj-$(CONFIG_CMD_TIME) += time.o
 obj-$(CONFIG_CMD_TIMER) += timer.o
diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
new file mode 100644
index 0000000..6ad287e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+#include <command.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+				  char *const argv[])
+{
+	char a[128];
+
+	memset(a, 0xa5, 512);
+	return 0;
+}
+
+U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
+	   "test stack protector fail", "");
diff --git a/common/Kconfig b/common/Kconfig
index 2bce8c9..6a94045 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -595,6 +595,23 @@ config TPL_HASH
 	  and the algorithms it supports are defined in common/hash.c. See
 	  also CMD_HASH for command-line access.
 
+config STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection"
+	default n
+	help
+	  Enable stack smash detection through compiler's stack-protector
+	  canary logic
+
+config SPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for SPL"
+	depends on STACKPROTECTOR && SPL
+	default n
+
+config TPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for TPL"
+	depends on STACKPROTECTOR && TPL
+	default n
+
 endmenu
 
 menu "Update support"
diff --git a/common/Makefile b/common/Makefile
index bcf352d..fe71e18 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
 obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
 
 obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
+obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
+
diff --git a/common/stackprot.c b/common/stackprot.c
new file mode 100644
index 0000000..1f1c3b4
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard = (long)(0xfeedf00ddeadbeef & ~0L);
+
+void __stack_chk_fail(void)
+{
+	panic("Stack smashing detected in function:\n%p relocated from %p",
+	      __builtin_extract_return_addr(__builtin_return_address(0)),
+	      __builtin_return_address(0) - gd->reloc_off);
+}
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 58d4ef1..0c82ef2 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -20,11 +20,11 @@ CONFIG_BOOTSTAGE_STASH=y
 CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
 CONFIG_CONSOLE_RECORD=y
 CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
-CONFIG_SILENT_CONSOLE=y
 CONFIG_PRE_CONSOLE_BUFFER=y
 CONFIG_LOG_SYSLOG=y
 CONFIG_LOG_ERROR_RETURN=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_STACKPROTECTOR=y
 CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
@@ -96,6 +96,7 @@ CONFIG_CMD_CRAMFS=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_SQUASHFS=y
 CONFIG_CMD_MTDPARTS=y
+CONFIG_CMD_STACKPROTECTOR_TEST=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
@@ -131,6 +132,7 @@ CONFIG_CPU=y
 CONFIG_DM_DEMO=y
 CONFIG_DM_DEMO_SIMPLE=y
 CONFIG_DM_DEMO_SHAPE=y
+CONFIG_DFU_SF=y
 CONFIG_DMA=y
 CONFIG_DMA_CHANNELS=y
 CONFIG_SANDBOX_DMA=y
@@ -269,14 +271,12 @@ CONFIG_CMD_DHRYSTONE=y
 CONFIG_TPM=y
 CONFIG_LZ4=y
 CONFIG_ERRNO_STR=y
+CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
+CONFIG_EFI_CAPSULE_ON_DISK=y
+CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
+CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
-#
-CONFIG_DFU_SF=y
-CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
-CONFIG_EFI_CAPSULE_ON_DISK=y
-CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
-CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 87021e2..6725201 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -67,6 +67,12 @@ include $(srctree)/scripts/Makefile.lib
 KBUILD_CFLAGS += -ffunction-sections -fdata-sections
 LDFLAGS_FINAL += --gc-sections
 
+ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
+KBUILD_CFLAGS += -fstack-protector-strong
+else
+KBUILD_CFLAGS += -fno-stack-protector
+endif
+
 # FIX ME
 cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
 							$(NOSTDINC_FLAGS)
diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
new file mode 100644
index 0000000..957a2a6
--- /dev/null
+++ b/test/py/tests/test_stackprotector.py
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Broadcom
+
+import pytest
+import signal
+
+ at pytest.mark.buildconfigspec('cmd_stackprotector_test')
+def test_stackprotector(u_boot_console):
+    """Test that the stackprotector function works."""
+
+    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
+    expected_response = 'Stack smashing detected'
+    u_boot_console.wait_for(expected_response)
+    assert(True)
+
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH v4] Add support for stack-protector
  2021-01-11 22:49 ` [PATCH v4] " Joel Peshkin
@ 2021-01-12 15:48   ` Heinrich Schuchardt
  0 siblings, 0 replies; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-01-12 15:48 UTC (permalink / raw)
  To: u-boot

On 11.01.21 23:49, Joel Peshkin wrote:

Somehow the commit message got lost.

> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Usama Arif <usama.arif@arm.com>
> Cc: Sam Protsenko <joe.skb7@gmail.com>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Philippe Reynes <philippe.reynes@softathome.com>
> Cc: Eugeniu Rosca <roscaeugeniu@gmail.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
>
> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
>
> Changes for v4:
>    - Exclude EFI from stackprotector

I cannot find this in the code below.

The safest thing to do is to add a __stack_chk_fail() function to
lib/efi_loader/efi_freestanding.c:

void __stack_chk_fail(void)
{
	for (;;)
		;
}

>    - Cleanups of extra includes and declaration
>
> Changes for v3:
>    - Move test command to cmd/
>    - Update Kconfig names and depends
>    - clean up default canary initialization
>
> Changes for v2:
>    - Add test command and corresponding config
>    - Fixed incorrect description in Kconfig
>    - Add unit test
> ---
>  MAINTAINERS                          |  7 +++++++
>  Makefile                             |  5 +++++
>  cmd/Kconfig                          | 10 ++++++++++
>  cmd/Makefile                         |  1 +
>  cmd/stackprot_test.c                 | 21 +++++++++++++++++++++
>  common/Kconfig                       | 17 +++++++++++++++++
>  common/Makefile                      |  2 ++
>  common/stackprot.c                   | 17 +++++++++++++++++
>  configs/sandbox_defconfig            |  2 ++
>  scripts/Makefile.spl                 |  6 ++++++
>  test/py/tests/test_stackprotector.py | 15 +++++++++++++++
>  11 files changed, 103 insertions(+)
>  create mode 100644 cmd/stackprot_test.c
>  create mode 100644 common/stackprot.c
>  create mode 100644 test/py/tests/test_stackprotector.py
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 52d7307525..2982ffcc07 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1022,6 +1022,13 @@ F:	include/sqfs.h
>  F:	cmd/sqfs.c
>  F:	test/py/tests/test_fs/test_squashfs/
>
> +STACKPROTECTOR
> +M:	Joel Peshkin <joel.peshkin@broadcom.com>
> +S:	Maintained
> +F:	common/stackprot.c
> +F:	cmd/stackprot_test.c
> +F:	test/py/tests/test_stackprotector.py
> +
>  TARGET_BCMNS3
>  M:	Bharat Gooty <bharat.gooty@broadcom.com>
>  M:	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> diff --git a/Makefile b/Makefile
> index 3ee4cc00dd..8c04fcf96c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -677,7 +677,12 @@ else
>  KBUILD_CFLAGS	+= -O2
>  endif
>
> +ifeq ($(CONFIG_STACKPROTECTOR),y)
> +KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
> +CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
> +else
>  KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
> +endif
>  KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
>
>  # disable stringop warnings in gcc 8+
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 1595de999b..04700e4bb4 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2271,6 +2271,16 @@ config CMD_AVB
>  	    avb read_part_hex - read data from partition and output to stdout
>  	    avb write_part - write data to partition
>  	    avb verify - run full verification chain
> +
> +config CMD_STACKPROTECTOR_TEST
> +	bool "Test command for stack protector"
> +	depends on STACKPROTECTOR
> +	default n
> +	help
> +	  Enable stackprot_test command
> +	  The stackprot_test command will force a stack overrun to test
> +	  the stack smashing detection mechanisms.
> +
>  endmenu
>
>  config CMD_UBI
> diff --git a/cmd/Makefile b/cmd/Makefile
> index dd86675bf2..8e8fcea19e 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
>  obj-$(CONFIG_CMD_STRINGS) += strings.o
>  obj-$(CONFIG_CMD_SMC) += smccc.o
>  obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
> +obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
>  obj-$(CONFIG_CMD_TERMINAL) += terminal.o
>  obj-$(CONFIG_CMD_TIME) += time.o
>  obj-$(CONFIG_CMD_TIMER) += timer.o
> diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
> new file mode 100644
> index 0000000000..6ad287e55f
> --- /dev/null
> +++ b/cmd/stackprot_test.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright 2021 Broadcom
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
> +				  char *const argv[])
> +{
> +	char a[128];
> +
> +	memset(a, 0xa5, 512);
> +	return 0;
> +}
> +
> +U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
> +	   "test stack protector fail", "");
> diff --git a/common/Kconfig b/common/Kconfig
> index 2bce8c9ba1..6a94045948 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -595,6 +595,23 @@ config TPL_HASH
>  	  and the algorithms it supports are defined in common/hash.c. See
>  	  also CMD_HASH for command-line access.
>
> +config STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection"
> +	default n
> +	help
> +	  Enable stack smash detection through compiler's stack-protector
> +	  canary logic
> +
> +config SPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for SPL"
> +	depends on STACKPROTECTOR && SPL
> +	default n
> +
> +config TPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for TPL"
> +	depends on STACKPROTECTOR && TPL
> +	default n
> +
>  endmenu
>
>  menu "Update support"
> diff --git a/common/Makefile b/common/Makefile
> index bcf352d016..fe71e18317 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
>  obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
>
>  obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
> +obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
> +
> diff --git a/common/stackprot.c b/common/stackprot.c
> new file mode 100644
> index 0000000000..1f1c3b4273
> --- /dev/null
> +++ b/common/stackprot.c
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright 2021 Broadcom
> + */
> +
> +#include <common.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +unsigned long __stack_chk_guard = (long)(0xfeedf00ddeadbeef & ~0L);
> +
> +void __stack_chk_fail(void)
> +{
> +	panic("Stack smashing detected in function:\n%p relocated from %p",
> +	      __builtin_extract_return_addr(__builtin_return_address(0)),
> +	      __builtin_return_address(0) - gd->reloc_off);

Missing __builtin_extract_return_addr(). I suggest you add a line

void *addr = __builtin_extract_return_addr(__builtin_return_address(0));

and then use that variable for the panic() call.

> +}
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index f1ec701a9f..3e92a58bdb 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -274,3 +274,5 @@ CONFIG_TEST_FDTDEC=y
>  CONFIG_UNIT_TEST=y
>  CONFIG_UT_TIME=y
>  CONFIG_UT_DM=y
> +CONFIG_STACKPROTECTOR=y
> +CONFIG_CMD_STACKPROTECTOR_TEST=y

Please, use make safedefconfig to get the sequence right. If there are
too many unrelated changes, you can split of a separate patch for
realigning sandbox_defconfig first.

Best regards

Heinrich

> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index 9f1f7445d7..1505e4e851 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -63,6 +63,12 @@ include $(srctree)/scripts/Makefile.lib
>  KBUILD_CFLAGS += -ffunction-sections -fdata-sections
>  LDFLAGS_FINAL += --gc-sections
>
> +ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
> +KBUILD_CFLAGS += -fstack-protector-strong
> +else
> +KBUILD_CFLAGS += -fno-stack-protector
> +endif
> +
>  # FIX ME
>  cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
>  							$(NOSTDINC_FLAGS)
> diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
> new file mode 100644
> index 0000000000..957a2a6cf7
> --- /dev/null
> +++ b/test/py/tests/test_stackprotector.py
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Broadcom
> +
> +import pytest
> +import signal
> +
> + at pytest.mark.buildconfigspec('cmd_stackprotector_test')
> +def test_stackprotector(u_boot_console):
> +    """Test that the stackprotector function works."""
> +
> +    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
> +    expected_response = 'Stack smashing detected'
> +    u_boot_console.wait_for(expected_response)
> +    assert(True)
> +
>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v6] Add support for stack-protector
  2021-01-10 15:39 [PATCH] Add support for stack-protector Joel Peshkin
                   ` (5 preceding siblings ...)
  2021-01-11 23:55 ` [PATCH v5] " Joel Peshkin
@ 2021-01-12 16:51 ` Joel Peshkin
  2021-01-12 20:01   ` Heinrich Schuchardt
  2021-01-14 13:59 ` [PATCH v7] " Joel Peshkin
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Joel Peshkin @ 2021-01-12 16:51 UTC (permalink / raw)
  To: u-boot

Cc: Simon Glass <sjg@chromium.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>

Add support for stack protector for UBOOT, SPL, and TPL
as well as new pytest for stackprotector

Changes for v6:
   - Fix commit message
Changes for v5:
   - Rebase
Changes for v4:
   - Exclude EFI from stackprotector
   - Cleanups of extra includes and declaration
Changes for v3:
   - Move test command to cmd/
   - Update Kconfig names and depends
   - clean up default canary initialization
Changes for v2:
   - Add test command and corresponding config
   - Fixed incorrect description in Kconfig
   - Add unit test
---
---
 MAINTAINERS                          |  7 +++++++
 Makefile                             |  5 +++++
 cmd/Kconfig                          | 10 ++++++++++
 cmd/Makefile                         |  1 +
 cmd/stackprot_test.c                 | 21 +++++++++++++++++++++
 common/Kconfig                       | 17 +++++++++++++++++
 common/Makefile                      |  2 ++
 common/stackprot.c                   | 17 +++++++++++++++++
 configs/sandbox_defconfig            | 14 +++++++-------
 scripts/Makefile.spl                 |  6 ++++++
 test/py/tests/test_stackprotector.py | 15 +++++++++++++++
 11 files changed, 108 insertions(+), 7 deletions(-)
 create mode 100644 cmd/stackprot_test.c
 create mode 100644 common/stackprot.c
 create mode 100644 test/py/tests/test_stackprotector.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 26dd254..d3971e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1024,6 +1024,13 @@ F:	include/sqfs.h
 F:	cmd/sqfs.c
 F:	test/py/tests/test_fs/test_squashfs/
 
+STACKPROTECTOR
+M:	Joel Peshkin <joel.peshkin@broadcom.com>
+S:	Maintained
+F:	common/stackprot.c
+F:	cmd/stackprot_test.c
+F:	test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M:	Bharat Gooty <bharat.gooty@broadcom.com>
 M:	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
diff --git a/Makefile b/Makefile
index 902a976..65c5cb8 100644
--- a/Makefile
+++ b/Makefile
@@ -677,7 +677,12 @@ else
 KBUILD_CFLAGS	+= -O2
 endif
 
+ifeq ($(CONFIG_STACKPROTECTOR),y)
+KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
+CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
+else
 KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
+endif
 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
 
 # disable stringop warnings in gcc 8+
diff --git a/cmd/Kconfig b/cmd/Kconfig
index da86a94..054b2f3 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2280,6 +2280,16 @@ config CMD_AVB
 	    avb read_part_hex - read data from partition and output to stdout
 	    avb write_part - write data to partition
 	    avb verify - run full verification chain
+
+config CMD_STACKPROTECTOR_TEST
+	bool "Test command for stack protector"
+	depends on STACKPROTECTOR
+	default n
+	help
+	  Enable stackprot_test command
+	  The stackprot_test command will force a stack overrun to test
+	  the stack smashing detection mechanisms.
+
 endmenu
 
 config CMD_UBI
diff --git a/cmd/Makefile b/cmd/Makefile
index 5b3400a..1d7afea 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
 obj-$(CONFIG_CMD_STRINGS) += strings.o
 obj-$(CONFIG_CMD_SMC) += smccc.o
 obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
+obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
 obj-$(CONFIG_CMD_TERMINAL) += terminal.o
 obj-$(CONFIG_CMD_TIME) += time.o
 obj-$(CONFIG_CMD_TIMER) += timer.o
diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
new file mode 100644
index 0000000..6ad287e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+#include <command.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+				  char *const argv[])
+{
+	char a[128];
+
+	memset(a, 0xa5, 512);
+	return 0;
+}
+
+U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
+	   "test stack protector fail", "");
diff --git a/common/Kconfig b/common/Kconfig
index 2bce8c9..6a94045 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -595,6 +595,23 @@ config TPL_HASH
 	  and the algorithms it supports are defined in common/hash.c. See
 	  also CMD_HASH for command-line access.
 
+config STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection"
+	default n
+	help
+	  Enable stack smash detection through compiler's stack-protector
+	  canary logic
+
+config SPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for SPL"
+	depends on STACKPROTECTOR && SPL
+	default n
+
+config TPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for TPL"
+	depends on STACKPROTECTOR && TPL
+	default n
+
 endmenu
 
 menu "Update support"
diff --git a/common/Makefile b/common/Makefile
index bcf352d..fe71e18 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
 obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
 
 obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
+obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
+
diff --git a/common/stackprot.c b/common/stackprot.c
new file mode 100644
index 0000000..1f1c3b4
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard = (long)(0xfeedf00ddeadbeef & ~0L);
+
+void __stack_chk_fail(void)
+{
+	panic("Stack smashing detected in function:\n%p relocated from %p",
+	      __builtin_extract_return_addr(__builtin_return_address(0)),
+	      __builtin_return_address(0) - gd->reloc_off);
+}
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 58d4ef1..0c82ef2 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -20,11 +20,11 @@ CONFIG_BOOTSTAGE_STASH=y
 CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
 CONFIG_CONSOLE_RECORD=y
 CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
-CONFIG_SILENT_CONSOLE=y
 CONFIG_PRE_CONSOLE_BUFFER=y
 CONFIG_LOG_SYSLOG=y
 CONFIG_LOG_ERROR_RETURN=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_STACKPROTECTOR=y
 CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
@@ -96,6 +96,7 @@ CONFIG_CMD_CRAMFS=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_SQUASHFS=y
 CONFIG_CMD_MTDPARTS=y
+CONFIG_CMD_STACKPROTECTOR_TEST=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
@@ -131,6 +132,7 @@ CONFIG_CPU=y
 CONFIG_DM_DEMO=y
 CONFIG_DM_DEMO_SIMPLE=y
 CONFIG_DM_DEMO_SHAPE=y
+CONFIG_DFU_SF=y
 CONFIG_DMA=y
 CONFIG_DMA_CHANNELS=y
 CONFIG_SANDBOX_DMA=y
@@ -269,14 +271,12 @@ CONFIG_CMD_DHRYSTONE=y
 CONFIG_TPM=y
 CONFIG_LZ4=y
 CONFIG_ERRNO_STR=y
+CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
+CONFIG_EFI_CAPSULE_ON_DISK=y
+CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
+CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
-#
-CONFIG_DFU_SF=y
-CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
-CONFIG_EFI_CAPSULE_ON_DISK=y
-CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
-CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 87021e2..6725201 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -67,6 +67,12 @@ include $(srctree)/scripts/Makefile.lib
 KBUILD_CFLAGS += -ffunction-sections -fdata-sections
 LDFLAGS_FINAL += --gc-sections
 
+ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
+KBUILD_CFLAGS += -fstack-protector-strong
+else
+KBUILD_CFLAGS += -fno-stack-protector
+endif
+
 # FIX ME
 cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
 							$(NOSTDINC_FLAGS)
diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
new file mode 100644
index 0000000..957a2a6
--- /dev/null
+++ b/test/py/tests/test_stackprotector.py
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Broadcom
+
+import pytest
+import signal
+
+ at pytest.mark.buildconfigspec('cmd_stackprotector_test')
+def test_stackprotector(u_boot_console):
+    """Test that the stackprotector function works."""
+
+    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
+    expected_response = 'Stack smashing detected'
+    u_boot_console.wait_for(expected_response)
+    assert(True)
+
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH v6] Add support for stack-protector
  2021-01-12 16:51 ` [PATCH v6] " Joel Peshkin
@ 2021-01-12 20:01   ` Heinrich Schuchardt
  0 siblings, 0 replies; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-01-12 20:01 UTC (permalink / raw)
  To: u-boot

On 12.01.21 17:51, Joel Peshkin wrote:> Cc: Simon Glass
<sjg@chromium.org>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>>
Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
These lines should be below the commit message body.

>
> Add support for stack protector for UBOOT, SPL, and TPL
> as well as new pytest for stackprotector
>

The following lines should be below the first ---.

> Changes for v6:
>    - Fix commit message
> Changes for v5:
>    - Rebase
> Changes for v4:
>    - Exclude EFI from stackprotector
>    - Cleanups of extra includes and declaration
> Changes for v3:
>    - Move test command to cmd/
>    - Update Kconfig names and depends
>    - clean up default canary initialization
> Changes for v2:
>    - Add test command and corresponding config
>    - Fixed incorrect description in Kconfig
>    - Add unit test
> ---
> ---
>  MAINTAINERS                          |  7 +++++++
>  Makefile                             |  5 +++++
>  cmd/Kconfig                          | 10 ++++++++++
>  cmd/Makefile                         |  1 +
>  cmd/stackprot_test.c                 | 21 +++++++++++++++++++++
>  common/Kconfig                       | 17 +++++++++++++++++
>  common/Makefile                      |  2 ++
>  common/stackprot.c                   | 17 +++++++++++++++++
>  configs/sandbox_defconfig            | 14 +++++++-------
>  scripts/Makefile.spl                 |  6 ++++++
>  test/py/tests/test_stackprotector.py | 15 +++++++++++++++
>  11 files changed, 108 insertions(+), 7 deletions(-)
>  create mode 100644 cmd/stackprot_test.c
>  create mode 100644 common/stackprot.c
>  create mode 100644 test/py/tests/test_stackprotector.py
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 26dd254..d3971e8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1024,6 +1024,13 @@ F:	include/sqfs.h
>  F:	cmd/sqfs.c
>  F:	test/py/tests/test_fs/test_squashfs/
>
> +STACKPROTECTOR
> +M:	Joel Peshkin <joel.peshkin@broadcom.com>
> +S:	Maintained
> +F:	common/stackprot.c
> +F:	cmd/stackprot_test.c
> +F:	test/py/tests/test_stackprotector.py
> +
>  TARGET_BCMNS3
>  M:	Bharat Gooty <bharat.gooty@broadcom.com>
>  M:	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> diff --git a/Makefile b/Makefile
> index 902a976..65c5cb8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -677,7 +677,12 @@ else
>  KBUILD_CFLAGS	+= -O2
>  endif
>
> +ifeq ($(CONFIG_STACKPROTECTOR),y)
> +KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
> +CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
> +else
>  KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
> +endif
>  KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
>
>  # disable stringop warnings in gcc 8+
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index da86a94..054b2f3 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2280,6 +2280,16 @@ config CMD_AVB
>  	    avb read_part_hex - read data from partition and output to stdout
>  	    avb write_part - write data to partition
>  	    avb verify - run full verification chain
> +
> +config CMD_STACKPROTECTOR_TEST
> +	bool "Test command for stack protector"
> +	depends on STACKPROTECTOR
> +	default n
> +	help
> +	  Enable stackprot_test command
> +	  The stackprot_test command will force a stack overrun to test
> +	  the stack smashing detection mechanisms.
> +
>  endmenu
>
>  config CMD_UBI
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 5b3400a..1d7afea 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
>  obj-$(CONFIG_CMD_STRINGS) += strings.o
>  obj-$(CONFIG_CMD_SMC) += smccc.o
>  obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
> +obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
>  obj-$(CONFIG_CMD_TERMINAL) += terminal.o
>  obj-$(CONFIG_CMD_TIME) += time.o
>  obj-$(CONFIG_CMD_TIMER) += timer.o
> diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
> new file mode 100644
> index 0000000..6ad287e
> --- /dev/null
> +++ b/cmd/stackprot_test.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright 2021 Broadcom
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
> +				  char *const argv[])
> +{
> +	char a[128];
> +
> +	memset(a, 0xa5, 512);
> +	return 0;
> +}
> +
> +U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
> +	   "test stack protector fail", "");
> diff --git a/common/Kconfig b/common/Kconfig
> index 2bce8c9..6a94045 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -595,6 +595,23 @@ config TPL_HASH
>  	  and the algorithms it supports are defined in common/hash.c. See
>  	  also CMD_HASH for command-line access.
>
> +config STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection"
> +	default n
> +	help
> +	  Enable stack smash detection through compiler's stack-protector
> +	  canary logic
> +
> +config SPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for SPL"
> +	depends on STACKPROTECTOR && SPL
> +	default n
> +
> +config TPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for TPL"
> +	depends on STACKPROTECTOR && TPL
> +	default n
> +
>  endmenu
>
>  menu "Update support"
> diff --git a/common/Makefile b/common/Makefile
> index bcf352d..fe71e18 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
>  obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
>
>  obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
> +obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
> +
> diff --git a/common/stackprot.c b/common/stackprot.c
> new file mode 100644
> index 0000000..1f1c3b4
> --- /dev/null
> +++ b/common/stackprot.c
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright 2021 Broadcom
> + */
> +
> +#include <common.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +unsigned long __stack_chk_guard = (long)(0xfeedf00ddeadbeef & ~0L);
> +
> +void __stack_chk_fail(void)
> +{
> +	panic("Stack smashing detected in function:\n%p relocated from %p",
> +	      __builtin_extract_return_addr(__builtin_return_address(0)),
> +	      __builtin_return_address(0) - gd->reloc_off);

A __builtin_extract_return_addr() is missing here.

I suggest that you first write
__builtin_extract_return_addr(__builtin_return_address(0))
into a variable and than use it in panic().

Best regards

Heinrich

> +}
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 58d4ef1..0c82ef2 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -20,11 +20,11 @@ CONFIG_BOOTSTAGE_STASH=y
>  CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
>  CONFIG_CONSOLE_RECORD=y
>  CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
> -CONFIG_SILENT_CONSOLE=y
>  CONFIG_PRE_CONSOLE_BUFFER=y
>  CONFIG_LOG_SYSLOG=y
>  CONFIG_LOG_ERROR_RETURN=y
>  CONFIG_DISPLAY_BOARDINFO_LATE=y
> +CONFIG_STACKPROTECTOR=y
>  CONFIG_ANDROID_AB=y
>  CONFIG_CMD_CPU=y
>  CONFIG_CMD_LICENSE=y
> @@ -96,6 +96,7 @@ CONFIG_CMD_CRAMFS=y
>  CONFIG_CMD_EXT4_WRITE=y
>  CONFIG_CMD_SQUASHFS=y
>  CONFIG_CMD_MTDPARTS=y
> +CONFIG_CMD_STACKPROTECTOR_TEST=y
>  CONFIG_MAC_PARTITION=y
>  CONFIG_AMIGA_PARTITION=y
>  CONFIG_OF_CONTROL=y
> @@ -131,6 +132,7 @@ CONFIG_CPU=y
>  CONFIG_DM_DEMO=y
>  CONFIG_DM_DEMO_SIMPLE=y
>  CONFIG_DM_DEMO_SHAPE=y
> +CONFIG_DFU_SF=y
>  CONFIG_DMA=y
>  CONFIG_DMA_CHANNELS=y
>  CONFIG_SANDBOX_DMA=y
> @@ -269,14 +271,12 @@ CONFIG_CMD_DHRYSTONE=y
>  CONFIG_TPM=y
>  CONFIG_LZ4=y
>  CONFIG_ERRNO_STR=y
> +CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> +CONFIG_EFI_CAPSULE_ON_DISK=y
> +CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
>  CONFIG_EFI_SECURE_BOOT=y
>  CONFIG_TEST_FDTDEC=y
>  CONFIG_UNIT_TEST=y
>  CONFIG_UT_TIME=y
>  CONFIG_UT_DM=y
> -#
> -CONFIG_DFU_SF=y
> -CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> -CONFIG_EFI_CAPSULE_ON_DISK=y
> -CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> -CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index 87021e2..6725201 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -67,6 +67,12 @@ include $(srctree)/scripts/Makefile.lib
>  KBUILD_CFLAGS += -ffunction-sections -fdata-sections
>  LDFLAGS_FINAL += --gc-sections
>
> +ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
> +KBUILD_CFLAGS += -fstack-protector-strong
> +else
> +KBUILD_CFLAGS += -fno-stack-protector
> +endif
> +
>  # FIX ME
>  cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
>  							$(NOSTDINC_FLAGS)
> diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
> new file mode 100644
> index 0000000..957a2a6
> --- /dev/null
> +++ b/test/py/tests/test_stackprotector.py
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Broadcom
> +
> +import pytest
> +import signal
> +
> + at pytest.mark.buildconfigspec('cmd_stackprotector_test')
> +def test_stackprotector(u_boot_console):
> +    """Test that the stackprotector function works."""
> +
> +    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
> +    expected_response = 'Stack smashing detected'
> +    u_boot_console.wait_for(expected_response)
> +    assert(True)
> +
>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v7] Add support for stack-protector
  2021-01-10 15:39 [PATCH] Add support for stack-protector Joel Peshkin
                   ` (6 preceding siblings ...)
  2021-01-12 16:51 ` [PATCH v6] " Joel Peshkin
@ 2021-01-14 13:59 ` Joel Peshkin
  2021-01-14 14:59   ` Alex Sadovsky
  2021-01-14 20:35 ` [PATCH v8] " Joel Peshkin
  2021-02-09  3:36 ` [PATCH v9] " Joel Peshkin
  9 siblings, 1 reply; 29+ messages in thread
From: Joel Peshkin @ 2021-01-14 13:59 UTC (permalink / raw)
  To: u-boot

Add support for stack protector for UBOOT, SPL, and TPL
as well as new pytest for stackprotector

Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>

Changes for v7:
   - Fix commit message
   - add __builtin_extract_return_addr() calls
Changes for v6:
   - Fix commit message
Changes for v5:
   - Rebase
Changes for v4:
   - Exclude EFI from stackprotector
   - Cleanups of extra includes and declaration
Changes for v3:
   - Move test command to cmd/
   - Update Kconfig names and depends
   - clean up default canary initialization
Changes for v2:
   - Add test command and corresponding config
   - Fixed incorrect description in Kconfig
   - Add unit test
---
---
 MAINTAINERS                          |  7 +++++++
 Makefile                             |  5 +++++
 cmd/Kconfig                          | 10 ++++++++++
 cmd/Makefile                         |  1 +
 cmd/stackprot_test.c                 | 21 +++++++++++++++++++++
 common/Kconfig                       | 17 +++++++++++++++++
 common/Makefile                      |  2 ++
 common/stackprot.c                   | 19 +++++++++++++++++++
 configs/sandbox_defconfig            | 14 +++++++-------
 scripts/Makefile.spl                 |  6 ++++++
 test/py/tests/test_stackprotector.py | 15 +++++++++++++++
 11 files changed, 110 insertions(+), 7 deletions(-)
 create mode 100644 cmd/stackprot_test.c
 create mode 100644 common/stackprot.c
 create mode 100644 test/py/tests/test_stackprotector.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 26dd254..d3971e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1024,6 +1024,13 @@ F:	include/sqfs.h
 F:	cmd/sqfs.c
 F:	test/py/tests/test_fs/test_squashfs/
 
+STACKPROTECTOR
+M:	Joel Peshkin <joel.peshkin@broadcom.com>
+S:	Maintained
+F:	common/stackprot.c
+F:	cmd/stackprot_test.c
+F:	test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M:	Bharat Gooty <bharat.gooty@broadcom.com>
 M:	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
diff --git a/Makefile b/Makefile
index 902a976..65c5cb8 100644
--- a/Makefile
+++ b/Makefile
@@ -677,7 +677,12 @@ else
 KBUILD_CFLAGS	+= -O2
 endif
 
+ifeq ($(CONFIG_STACKPROTECTOR),y)
+KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
+CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
+else
 KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
+endif
 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
 
 # disable stringop warnings in gcc 8+
diff --git a/cmd/Kconfig b/cmd/Kconfig
index da86a94..054b2f3 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2280,6 +2280,16 @@ config CMD_AVB
 	    avb read_part_hex - read data from partition and output to stdout
 	    avb write_part - write data to partition
 	    avb verify - run full verification chain
+
+config CMD_STACKPROTECTOR_TEST
+	bool "Test command for stack protector"
+	depends on STACKPROTECTOR
+	default n
+	help
+	  Enable stackprot_test command
+	  The stackprot_test command will force a stack overrun to test
+	  the stack smashing detection mechanisms.
+
 endmenu
 
 config CMD_UBI
diff --git a/cmd/Makefile b/cmd/Makefile
index 5b3400a..1d7afea 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
 obj-$(CONFIG_CMD_STRINGS) += strings.o
 obj-$(CONFIG_CMD_SMC) += smccc.o
 obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
+obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
 obj-$(CONFIG_CMD_TERMINAL) += terminal.o
 obj-$(CONFIG_CMD_TIME) += time.o
 obj-$(CONFIG_CMD_TIMER) += timer.o
diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
new file mode 100644
index 0000000..6ad287e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+#include <command.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+				  char *const argv[])
+{
+	char a[128];
+
+	memset(a, 0xa5, 512);
+	return 0;
+}
+
+U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
+	   "test stack protector fail", "");
diff --git a/common/Kconfig b/common/Kconfig
index 2bce8c9..6a94045 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -595,6 +595,23 @@ config TPL_HASH
 	  and the algorithms it supports are defined in common/hash.c. See
 	  also CMD_HASH for command-line access.
 
+config STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection"
+	default n
+	help
+	  Enable stack smash detection through compiler's stack-protector
+	  canary logic
+
+config SPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for SPL"
+	depends on STACKPROTECTOR && SPL
+	default n
+
+config TPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for TPL"
+	depends on STACKPROTECTOR && TPL
+	default n
+
 endmenu
 
 menu "Update support"
diff --git a/common/Makefile b/common/Makefile
index bcf352d..fe71e18 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
 obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
 
 obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
+obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
+
diff --git a/common/stackprot.c b/common/stackprot.c
new file mode 100644
index 0000000..d335946
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard = (long)(0xfeedf00ddeadbeef & ~0L);
+
+void __stack_chk_fail(void)
+{
+	void *ra;
+
+	ra = __builtin_extract_return_addr(__builtin_return_address(0));
+	panic("Stack smashing detected in function:\n%p relocated from %p",
+	      ra, ra - gd->reloc_off);
+}
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 58d4ef1..0c82ef2 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -20,11 +20,11 @@ CONFIG_BOOTSTAGE_STASH=y
 CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
 CONFIG_CONSOLE_RECORD=y
 CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
-CONFIG_SILENT_CONSOLE=y
 CONFIG_PRE_CONSOLE_BUFFER=y
 CONFIG_LOG_SYSLOG=y
 CONFIG_LOG_ERROR_RETURN=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_STACKPROTECTOR=y
 CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
@@ -96,6 +96,7 @@ CONFIG_CMD_CRAMFS=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_SQUASHFS=y
 CONFIG_CMD_MTDPARTS=y
+CONFIG_CMD_STACKPROTECTOR_TEST=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
@@ -131,6 +132,7 @@ CONFIG_CPU=y
 CONFIG_DM_DEMO=y
 CONFIG_DM_DEMO_SIMPLE=y
 CONFIG_DM_DEMO_SHAPE=y
+CONFIG_DFU_SF=y
 CONFIG_DMA=y
 CONFIG_DMA_CHANNELS=y
 CONFIG_SANDBOX_DMA=y
@@ -269,14 +271,12 @@ CONFIG_CMD_DHRYSTONE=y
 CONFIG_TPM=y
 CONFIG_LZ4=y
 CONFIG_ERRNO_STR=y
+CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
+CONFIG_EFI_CAPSULE_ON_DISK=y
+CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
+CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
-#
-CONFIG_DFU_SF=y
-CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
-CONFIG_EFI_CAPSULE_ON_DISK=y
-CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
-CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 87021e2..6725201 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -67,6 +67,12 @@ include $(srctree)/scripts/Makefile.lib
 KBUILD_CFLAGS += -ffunction-sections -fdata-sections
 LDFLAGS_FINAL += --gc-sections
 
+ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
+KBUILD_CFLAGS += -fstack-protector-strong
+else
+KBUILD_CFLAGS += -fno-stack-protector
+endif
+
 # FIX ME
 cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
 							$(NOSTDINC_FLAGS)
diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
new file mode 100644
index 0000000..957a2a6
--- /dev/null
+++ b/test/py/tests/test_stackprotector.py
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Broadcom
+
+import pytest
+import signal
+
+ at pytest.mark.buildconfigspec('cmd_stackprotector_test')
+def test_stackprotector(u_boot_console):
+    """Test that the stackprotector function works."""
+
+    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
+    expected_response = 'Stack smashing detected'
+    u_boot_console.wait_for(expected_response)
+    assert(True)
+
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH v7] Add support for stack-protector
  2021-01-14 13:59 ` [PATCH v7] " Joel Peshkin
@ 2021-01-14 14:59   ` Alex Sadovsky
  0 siblings, 0 replies; 29+ messages in thread
From: Alex Sadovsky @ 2021-01-14 14:59 UTC (permalink / raw)
  To: u-boot

Dear Joel,

> Add support for stack protector for UBOOT, SPL, and TPL
> as well as new pytest for stackprotector
>
> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> Changes for v7:
>??? - Fix commit message
>??? - add __builtin_extract_return_addr() calls
> Changes for v6:
>??? - Fix commit message
> Changes for v5:
>??? - Rebase
> Changes for v4:
>??? - Exclude EFI from stackprotector
>??? - Cleanups of extra includes and declaration
> Changes for v3:
>??? - Move test command to cmd/
>??? - Update Kconfig names and depends
>??? - clean up default canary initialization
> Changes for v2:
>??? - Add test command and corresponding config
>??? - Fixed incorrect description in Kconfig
>??? - Add unit test
> ---
> ---

Patch changelog is not a part of commit message, it's placed below the '---' mark. I hope this guide will help you:
https://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

> [...]
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +unsigned long __stack_chk_guard = (long)(0xfeedf00ddeadbeef & ~0L);

'long' and 'unsigned long' are slightly different types. I suggest you the following form (please, note the UL instead of L):

unsigned long __stack_chk_guard = (unsigned long)(0xfeedf00ddeadbeef & ~0UL)

Else gcc will complain like this:

> warning: unsigned conversion from ?long int? to ?long long unsigned int? changes value from ?-1? to ?18446744073709551615? [-Wsign-conversion]
> unsigned long __stack_chk_guard = (long)(0xfeedf00ddeadbeef & ~0L);
> warning: unsigned conversion from ?long int? to ?long unsigned int? changes value from ?-559038737? to ?3735928559? [-Wsign-conversion]

I hope you'll find -Wconversion compiler flag useful, it's one of the flags that often help me to spot some corner cases with potential bugs. IMHO it should be enabled by default.

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v8] Add support for stack-protector
  2021-01-10 15:39 [PATCH] Add support for stack-protector Joel Peshkin
                   ` (7 preceding siblings ...)
  2021-01-14 13:59 ` [PATCH v7] " Joel Peshkin
@ 2021-01-14 20:35 ` Joel Peshkin
  2021-01-15 18:53   ` Heinrich Schuchardt
  2021-01-28  0:57   ` Tom Rini
  2021-02-09  3:36 ` [PATCH v9] " Joel Peshkin
  9 siblings, 2 replies; 29+ messages in thread
From: Joel Peshkin @ 2021-01-14 20:35 UTC (permalink / raw)
  To: u-boot

Add support for stack protector for UBOOT, SPL, and TPL
as well as new pytest for stackprotector

Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
---
Cc: Simon Glass <sjg@chromium.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>

Changes for v8:
   - Fix commit message
   - Force canary to UL type
Changes for v7:
   - Fix commit message
   - add __builtin_extract_return_addr() calls
Changes for v6:
   - Fix commit message
Changes for v5:
   - Rebase
Changes for v4:
   - Exclude EFI from stackprotector
   - Cleanups of extra includes and declaration
Changes for v3:
   - Move test command to cmd/
   - Update Kconfig names and depends
   - clean up default canary initialization
Changes for v2:
   - Add test command and corresponding config
   - Fixed incorrect description in Kconfig
   - Add unit test
---
 MAINTAINERS                          |  7 +++++++
 Makefile                             |  5 +++++
 cmd/Kconfig                          | 10 ++++++++++
 cmd/Makefile                         |  1 +
 cmd/stackprot_test.c                 | 21 +++++++++++++++++++++
 common/Kconfig                       | 17 +++++++++++++++++
 common/Makefile                      |  2 ++
 common/stackprot.c                   | 19 +++++++++++++++++++
 configs/sandbox_defconfig            | 14 +++++++-------
 scripts/Makefile.spl                 |  6 ++++++
 test/py/tests/test_stackprotector.py | 15 +++++++++++++++
 11 files changed, 110 insertions(+), 7 deletions(-)
 create mode 100644 cmd/stackprot_test.c
 create mode 100644 common/stackprot.c
 create mode 100644 test/py/tests/test_stackprotector.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 26dd254..d3971e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1024,6 +1024,13 @@ F:	include/sqfs.h
 F:	cmd/sqfs.c
 F:	test/py/tests/test_fs/test_squashfs/
 
+STACKPROTECTOR
+M:	Joel Peshkin <joel.peshkin@broadcom.com>
+S:	Maintained
+F:	common/stackprot.c
+F:	cmd/stackprot_test.c
+F:	test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M:	Bharat Gooty <bharat.gooty@broadcom.com>
 M:	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
diff --git a/Makefile b/Makefile
index 902a976..65c5cb8 100644
--- a/Makefile
+++ b/Makefile
@@ -677,7 +677,12 @@ else
 KBUILD_CFLAGS	+= -O2
 endif
 
+ifeq ($(CONFIG_STACKPROTECTOR),y)
+KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
+CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
+else
 KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
+endif
 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
 
 # disable stringop warnings in gcc 8+
diff --git a/cmd/Kconfig b/cmd/Kconfig
index da86a94..054b2f3 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2280,6 +2280,16 @@ config CMD_AVB
 	    avb read_part_hex - read data from partition and output to stdout
 	    avb write_part - write data to partition
 	    avb verify - run full verification chain
+
+config CMD_STACKPROTECTOR_TEST
+	bool "Test command for stack protector"
+	depends on STACKPROTECTOR
+	default n
+	help
+	  Enable stackprot_test command
+	  The stackprot_test command will force a stack overrun to test
+	  the stack smashing detection mechanisms.
+
 endmenu
 
 config CMD_UBI
diff --git a/cmd/Makefile b/cmd/Makefile
index 5b3400a..1d7afea 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
 obj-$(CONFIG_CMD_STRINGS) += strings.o
 obj-$(CONFIG_CMD_SMC) += smccc.o
 obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
+obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
 obj-$(CONFIG_CMD_TERMINAL) += terminal.o
 obj-$(CONFIG_CMD_TIME) += time.o
 obj-$(CONFIG_CMD_TIMER) += timer.o
diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
new file mode 100644
index 0000000..6ad287e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+#include <command.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+				  char *const argv[])
+{
+	char a[128];
+
+	memset(a, 0xa5, 512);
+	return 0;
+}
+
+U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
+	   "test stack protector fail", "");
diff --git a/common/Kconfig b/common/Kconfig
index 2bce8c9..6a94045 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -595,6 +595,23 @@ config TPL_HASH
 	  and the algorithms it supports are defined in common/hash.c. See
 	  also CMD_HASH for command-line access.
 
+config STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection"
+	default n
+	help
+	  Enable stack smash detection through compiler's stack-protector
+	  canary logic
+
+config SPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for SPL"
+	depends on STACKPROTECTOR && SPL
+	default n
+
+config TPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for TPL"
+	depends on STACKPROTECTOR && TPL
+	default n
+
 endmenu
 
 menu "Update support"
diff --git a/common/Makefile b/common/Makefile
index bcf352d..fe71e18 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
 obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
 
 obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
+obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
+
diff --git a/common/stackprot.c b/common/stackprot.c
new file mode 100644
index 0000000..282c564
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard = (unsigned long)(0xfeedf00ddeadbeef & ~0UL);
+
+void __stack_chk_fail(void)
+{
+	void *ra;
+
+	ra = __builtin_extract_return_addr(__builtin_return_address(0));
+	panic("Stack smashing detected in function:\n%p relocated from %p",
+	      ra, ra - gd->reloc_off);
+}
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 58d4ef1..0c82ef2 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -20,11 +20,11 @@ CONFIG_BOOTSTAGE_STASH=y
 CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
 CONFIG_CONSOLE_RECORD=y
 CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
-CONFIG_SILENT_CONSOLE=y
 CONFIG_PRE_CONSOLE_BUFFER=y
 CONFIG_LOG_SYSLOG=y
 CONFIG_LOG_ERROR_RETURN=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_STACKPROTECTOR=y
 CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
@@ -96,6 +96,7 @@ CONFIG_CMD_CRAMFS=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_SQUASHFS=y
 CONFIG_CMD_MTDPARTS=y
+CONFIG_CMD_STACKPROTECTOR_TEST=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
@@ -131,6 +132,7 @@ CONFIG_CPU=y
 CONFIG_DM_DEMO=y
 CONFIG_DM_DEMO_SIMPLE=y
 CONFIG_DM_DEMO_SHAPE=y
+CONFIG_DFU_SF=y
 CONFIG_DMA=y
 CONFIG_DMA_CHANNELS=y
 CONFIG_SANDBOX_DMA=y
@@ -269,14 +271,12 @@ CONFIG_CMD_DHRYSTONE=y
 CONFIG_TPM=y
 CONFIG_LZ4=y
 CONFIG_ERRNO_STR=y
+CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
+CONFIG_EFI_CAPSULE_ON_DISK=y
+CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
+CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
-#
-CONFIG_DFU_SF=y
-CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
-CONFIG_EFI_CAPSULE_ON_DISK=y
-CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
-CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 87021e2..6725201 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -67,6 +67,12 @@ include $(srctree)/scripts/Makefile.lib
 KBUILD_CFLAGS += -ffunction-sections -fdata-sections
 LDFLAGS_FINAL += --gc-sections
 
+ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
+KBUILD_CFLAGS += -fstack-protector-strong
+else
+KBUILD_CFLAGS += -fno-stack-protector
+endif
+
 # FIX ME
 cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
 							$(NOSTDINC_FLAGS)
diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
new file mode 100644
index 0000000..957a2a6
--- /dev/null
+++ b/test/py/tests/test_stackprotector.py
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Broadcom
+
+import pytest
+import signal
+
+ at pytest.mark.buildconfigspec('cmd_stackprotector_test')
+def test_stackprotector(u_boot_console):
+    """Test that the stackprotector function works."""
+
+    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
+    expected_response = 'Stack smashing detected'
+    u_boot_console.wait_for(expected_response)
+    assert(True)
+
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH v8] Add support for stack-protector
  2021-01-14 20:35 ` [PATCH v8] " Joel Peshkin
@ 2021-01-15 18:53   ` Heinrich Schuchardt
  2021-01-28  0:57   ` Tom Rini
  1 sibling, 0 replies; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-01-15 18:53 UTC (permalink / raw)
  To: u-boot

On 14.01.21 21:35, Joel Peshkin wrote:
> Add support for stack protector for UBOOT, SPL, and TPL
> as well as new pytest for stackprotector
>
> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
> ---
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

>
> Changes for v8:
>    - Fix commit message
>    - Force canary to UL type
> Changes for v7:
>    - Fix commit message
>    - add __builtin_extract_return_addr() calls
> Changes for v6:
>    - Fix commit message
> Changes for v5:
>    - Rebase
> Changes for v4:
>    - Exclude EFI from stackprotector
>    - Cleanups of extra includes and declaration
> Changes for v3:
>    - Move test command to cmd/
>    - Update Kconfig names and depends
>    - clean up default canary initialization
> Changes for v2:
>    - Add test command and corresponding config
>    - Fixed incorrect description in Kconfig
>    - Add unit test

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v8] Add support for stack-protector
  2021-01-14 20:35 ` [PATCH v8] " Joel Peshkin
  2021-01-15 18:53   ` Heinrich Schuchardt
@ 2021-01-28  0:57   ` Tom Rini
  2021-01-28  8:20     ` Heinrich Schuchardt
  1 sibling, 1 reply; 29+ messages in thread
From: Tom Rini @ 2021-01-28  0:57 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 14, 2021 at 12:35:35PM -0800, Joel Peshkin wrote:

> Add support for stack protector for UBOOT, SPL, and TPL
> as well as new pytest for stackprotector
> 
> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Oddly enough this cases a number of the TPM2 tests to fail on sandbox:
https://dev.azure.com/u-boot/u-boot/_build/results?buildId=1706&view=logs&j=50449d1b-398e-53ae-48fa-6bf338edeb51&t=97605dd2-f5a5-5dd7-2118-315ffdc8bcd6&l=755

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210127/154eafb0/attachment.sig>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v8] Add support for stack-protector
  2021-01-28  0:57   ` Tom Rini
@ 2021-01-28  8:20     ` Heinrich Schuchardt
  2021-01-28 11:00       ` Heinrich Schuchardt
  0 siblings, 1 reply; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-01-28  8:20 UTC (permalink / raw)
  To: u-boot

On 1/28/21 1:57 AM, Tom Rini wrote:
> On Thu, Jan 14, 2021 at 12:35:35PM -0800, Joel Peshkin wrote:
>
>> Add support for stack protector for UBOOT, SPL, and TPL
>> as well as new pytest for stackprotector
>>
>> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
>> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> Oddly enough this cases a number of the TPM2 tests to fail on sandbox:
> https://dev.azure.com/u-boot/u-boot/_build/results?buildId=1706&view=logs&j=50449d1b-398e-53ae-48fa-6bf338edeb51&t=97605dd2-f5a5-5dd7-2118-315ffdc8bcd6&l=755
>

Thanks Tom for the head-up.

The problem is reproducible locally using 'make tests' with the patch
applied on top of origin/master.

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v8] Add support for stack-protector
  2021-01-28  8:20     ` Heinrich Schuchardt
@ 2021-01-28 11:00       ` Heinrich Schuchardt
  2021-01-28 14:33         ` Heinrich Schuchardt
  0 siblings, 1 reply; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-01-28 11:00 UTC (permalink / raw)
  To: u-boot

On 28.01.21 09:20, Heinrich Schuchardt wrote:
> On 1/28/21 1:57 AM, Tom Rini wrote:
>> On Thu, Jan 14, 2021 at 12:35:35PM -0800, Joel Peshkin wrote:
>>
>>> Add support for stack protector for UBOOT, SPL, and TPL
>>> as well as new pytest for stackprotector
>>>
>>> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
>>> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>
>> Oddly enough this cases a number of the TPM2 tests to fail on sandbox:
>> https://dev.azure.com/u-boot/u-boot/_build/results?buildId=1706&view=logs&j=50449d1b-398e-53ae-48fa-6bf338edeb51&t=97605dd2-f5a5-5dd7-2118-315ffdc8bcd6&l=755
>>
>>
>
> Thanks Tom for the head-up.
>
> The problem is reproducible locally using 'make tests' with the patch
> applied on top of origin/master.
>
> Best regards
>
> Heinrich

The stack protector has successfully detected stack smashes. See below.

A secondary finding is: panic("foo") without \n is not correctly flushed
to the console on the sandbox.

test_tpm2_init:

Stack smashing detected in function:
00005563faa7c521 relocated from 000000000004d521

u-boot.map
0x000000000004cf44                do_spi
0x000000000004d9e9                print_byte_string

It seems that the output written in the stack-protector test ends up in
the following test which happens to be test_tpm2_init:

static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
????????????????????????????????  char *const argv[])
{
   4d4db:???????48 81 ec 98 00 00 00 ???sub    $0x98,%rsp
????????char a[128];

????????memset(a, 0xa5, 512);
   4d4e2:???????ba 00 02 00 00       ???mov    $0x200,%edx
   4d4e7:???????be a5 00 00 00       ???mov    $0xa5,%esi
{
   4d4ec:???????64 48 8b 04 25 28 00 ???mov    %fs:0x28,%rax
   4d4f3:???????00 00?
   4d4f5:???????48 89 84 24 88 00 00 ???mov    %rax,0x88(%rsp)
   4d4fc:???????00?
   4d4fd:???????31 c0                ???xor    %eax,%eax
????????memset(a, 0xa5, 512);
   4d4ff:???????48 8d 7c 24 08       ???lea    0x8(%rsp),%rdi
   4d504:???????e8 1f 34 0d 00       ???callq  120928 <memset>
????????return 0;
}
   4d509:???????48 8b 84 24 88 00 00 ???mov    0x88(%rsp),%rax
   4d510:???????00?
   4d511:???????64 48 2b 04 25 28 00 ???sub    %fs:0x28,%rax
   4d518:???????00 00?
   4d51a:???????74 05                ???je     4d521
<do_test_stackprot_fail+0x46>
   4d51c:???????e8 68 48 02 00       ???callq  71d89 <__stack_chk_fail>
>>>4d521:???????31 c0                ???xor    %eax,%eax
   4d523:???????48 81 c4 98 00 00 00 ???add    $0x98,%rsp
   4d52a:???????c3                   ???retq???



TestEfiCapsuleFirmwareFit.test_efi_capsule_fw3
#Stack smashing detected in function:
000055827c21cb58 relocated from 00000000000d8b58

Here we seem to have a real finding.

u-boot.map
0x00000000000d7fae                efi_launch_capsules
0x00000000000da1ec                set_shift_mask

??????? efi_set_variable_int(L"CapsuleLast", &efi_guid_capsule_report,
   d8b10:???????4c 8b 04 24          ???mov    (%rsp),%r8
   d8b14:???????45 31 c9             ???xor    %r9d,%r9d
   d8b17:???????48 8d 35 42 1c 11 00 ???lea    0x111c42(%rip),%rsi
  # 1ea760 <efi_guid_capsule_report>
   d8b1e:???????b9 16 00 00 00       ???mov    $0x16,%ecx
   d8b23:???????ba 07 00 00 80       ???mov    $0x80000007,%edx
   d8b28:???????48 8d 3d e7 61 0c 00 ???lea    0xc61e7(%rip),%rdi
 # 19ed16 <__func__.0+0x132e6>
   d8b2f:???????e8 34 ab 00 00       ???callq  e3668 <efi_set_variable_int>
????????return ret;
   d8b34:???????eb 0a                ???jmp    d8b40
<efi_launch_capsules+0xb92>
????????????????return EFI_DEVICE_ERROR;
   d8b36:???????49 bc 07 00 00 00 00 ???movabs $0x8000000000000007,%r12
   d8b3d:???????00 00 80?
}
   d8b40:???????48 8b 84 24 c8 00 00 ???mov    0xc8(%rsp),%rax
   d8b47:???????00?
   d8b48:???????64 48 2b 04 25 28 00 ???sub    %fs:0x28,%rax
   d8b4f:???????00 00?
   d8b51:???????74 05                ???je     d8b58
<efi_launch_capsules+0xbaa>
   d8b53:???????e8 31 92 f9 ff       ???callq  71d89 <__stack_chk_fail>
>>>d8b58:???????48 81 c4 d8 00 00 00 ???add    $0xd8,%rsp
   d8b5f:???????4c 89 e0             ???mov    %r12,%rax
   d8b62:???????5b                   ???pop    %rbx
   d8b63:???????5d                   ???pop    %rbp
   d8b64:???????41 5c                ???pop    %r12
   d8b66:???????41 5d                ???pop    %r13
   d8b68:???????41 5e                ???pop    %r14
   d8b6a:???????41 5f                ???pop    %r15
   d8b6c:???????c3                   ???retq???


Best regards

Heinrich

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v8] Add support for stack-protector
  2021-01-28 11:00       ` Heinrich Schuchardt
@ 2021-01-28 14:33         ` Heinrich Schuchardt
  0 siblings, 0 replies; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-01-28 14:33 UTC (permalink / raw)
  To: u-boot

On 28.01.21 12:00, Heinrich Schuchardt wrote:
> On 28.01.21 09:20, Heinrich Schuchardt wrote:
>> On 1/28/21 1:57 AM, Tom Rini wrote:
>>> On Thu, Jan 14, 2021 at 12:35:35PM -0800, Joel Peshkin wrote:
>>>
>>>> Add support for stack protector for UBOOT, SPL, and TPL
>>>> as well as new pytest for stackprotector
>>>>
>>>> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
>>>> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>
>>> Oddly enough this cases a number of the TPM2 tests to fail on sandbox:
>>> https://dev.azure.com/u-boot/u-boot/_build/results?buildId=1706&view=logs&j=50449d1b-398e-53ae-48fa-6bf338edeb51&t=97605dd2-f5a5-5dd7-2118-315ffdc8bcd6&l=755
>>>
>>>
>>
>> Thanks Tom for the head-up.
>>
>> The problem is reproducible locally using 'make tests' with the patch
>> applied on top of origin/master.
>>
>> Best regards
>>
>> Heinrich
>
> The stack protector has successfully detected stack smashes. See below.
>
> A secondary finding is: panic("foo") without \n is not correctly flushed
> to the console on the sandbox.

./test/py/test.py --bd=sandbox -ra --build-dir=build-sandbox -k='not
test_stackprotector'
shows no error for tpm2.

./test/py/test.py --bd=sandbox -ra --build-dir=build-sandbox -k='not
test_stackprotector'
shows the error for tpm2.

I think the test test_stackprotector() is broken. I causes the sandbox
to reset but does not wait until reaches the prompt again, e.g add

    u_boot_console.restart_uboot()

instead of assert(true).

@Joel
Could you, please, submit a corrected patch.

@Takahiro
For test/py/tests/test_efi_capsule/test_capsule_firmware.py I always see
a smash. Could you, please, have a look at efi_launch_capsules().

Best regards

Heinrich

>
> test_tpm2_init:
>
> Stack smashing detected in function:
> 00005563faa7c521 relocated from 000000000004d521
>
> u-boot.map
> 0x000000000004cf44                do_spi
> 0x000000000004d9e9                print_byte_string
>
> It seems that the output written in the stack-protector test ends up in
> the following test which happens to be test_tpm2_init:
>
> static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
> ????????????????????????????????  char *const argv[])
> {
>    4d4db:???????48 81 ec 98 00 00 00 ???sub    $0x98,%rsp
> ????????char a[128];
>
> ????????memset(a, 0xa5, 512);
>    4d4e2:???????ba 00 02 00 00       ???mov    $0x200,%edx
>    4d4e7:???????be a5 00 00 00       ???mov    $0xa5,%esi
> {
>    4d4ec:???????64 48 8b 04 25 28 00 ???mov    %fs:0x28,%rax
>    4d4f3:???????00 00?
>    4d4f5:???????48 89 84 24 88 00 00 ???mov    %rax,0x88(%rsp)
>    4d4fc:???????00?
>    4d4fd:???????31 c0                ???xor    %eax,%eax
> ????????memset(a, 0xa5, 512);
>    4d4ff:???????48 8d 7c 24 08       ???lea    0x8(%rsp),%rdi
>    4d504:???????e8 1f 34 0d 00       ???callq  120928 <memset>
> ????????return 0;
> }
>    4d509:???????48 8b 84 24 88 00 00 ???mov    0x88(%rsp),%rax
>    4d510:???????00?
>    4d511:???????64 48 2b 04 25 28 00 ???sub    %fs:0x28,%rax
>    4d518:???????00 00?
>    4d51a:???????74 05                ???je     4d521
> <do_test_stackprot_fail+0x46>
>    4d51c:???????e8 68 48 02 00       ???callq  71d89 <__stack_chk_fail>
>>>> 4d521:???????31 c0                ???xor    %eax,%eax
>    4d523:???????48 81 c4 98 00 00 00 ???add    $0x98,%rsp
>    4d52a:???????c3                   ???retq???
>
>
>
> TestEfiCapsuleFirmwareFit.test_efi_capsule_fw3
> #Stack smashing detected in function:
> 000055827c21cb58 relocated from 00000000000d8b58
>
> Here we seem to have a real finding.
>
> u-boot.map
> 0x00000000000d7fae                efi_launch_capsules
> 0x00000000000da1ec                set_shift_mask
>
> ??????? efi_set_variable_int(L"CapsuleLast", &efi_guid_capsule_report,
>    d8b10:???????4c 8b 04 24          ???mov    (%rsp),%r8
>    d8b14:???????45 31 c9             ???xor    %r9d,%r9d
>    d8b17:???????48 8d 35 42 1c 11 00 ???lea    0x111c42(%rip),%rsi
>   # 1ea760 <efi_guid_capsule_report>
>    d8b1e:???????b9 16 00 00 00       ???mov    $0x16,%ecx
>    d8b23:???????ba 07 00 00 80       ???mov    $0x80000007,%edx
>    d8b28:???????48 8d 3d e7 61 0c 00 ???lea    0xc61e7(%rip),%rdi
>  # 19ed16 <__func__.0+0x132e6>
>    d8b2f:???????e8 34 ab 00 00       ???callq  e3668 <efi_set_variable_int>
> ????????return ret;
>    d8b34:???????eb 0a                ???jmp    d8b40
> <efi_launch_capsules+0xb92>
> ????????????????return EFI_DEVICE_ERROR;
>    d8b36:???????49 bc 07 00 00 00 00 ???movabs $0x8000000000000007,%r12
>    d8b3d:???????00 00 80?
> }
>    d8b40:???????48 8b 84 24 c8 00 00 ???mov    0xc8(%rsp),%rax
>    d8b47:???????00?
>    d8b48:???????64 48 2b 04 25 28 00 ???sub    %fs:0x28,%rax
>    d8b4f:???????00 00?
>    d8b51:???????74 05                ???je     d8b58
> <efi_launch_capsules+0xbaa>
>    d8b53:???????e8 31 92 f9 ff       ???callq  71d89 <__stack_chk_fail>
>>>> d8b58:???????48 81 c4 d8 00 00 00 ???add    $0xd8,%rsp
>    d8b5f:???????4c 89 e0             ???mov    %r12,%rax
>    d8b62:???????5b                   ???pop    %rbx
>    d8b63:???????5d                   ???pop    %rbp
>    d8b64:???????41 5c                ???pop    %r12
>    d8b66:???????41 5d                ???pop    %r13
>    d8b68:???????41 5e                ???pop    %r14
>    d8b6a:???????41 5f                ???pop    %r15
>    d8b6c:???????c3                   ???retq???
>
>
> Best regards
>
> Heinrich
>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v9] Add support for stack-protector
  2021-01-10 15:39 [PATCH] Add support for stack-protector Joel Peshkin
                   ` (8 preceding siblings ...)
  2021-01-14 20:35 ` [PATCH v8] " Joel Peshkin
@ 2021-02-09  3:36 ` Joel Peshkin
       [not found]   ` <794783f5-da5c-65b4-82a4-0f62d1f6a8b0@gmx.de>
  2021-03-22 17:37   ` Heinrich Schuchardt
  9 siblings, 2 replies; 29+ messages in thread
From: Joel Peshkin @ 2021-02-09  3:36 UTC (permalink / raw)
  To: u-boot

Add support for stack protector for UBOOT, SPL, and TPL
as well as new pytest for stackprotector

Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
---
Cc: Simon Glass <sjg@chromium.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>

Changes for v9:
   - Fix pytest script post-test reboot
Changes for v8:
   - Fix commit message
   - Force canary to UL type
Changes for v7:
   - Fix commit message
   - add __builtin_extract_return_addr() calls
Changes for v6:
   - Fix commit message
Changes for v5:
   - Rebase
Changes for v4:
   - Exclude EFI from stackprotector
   - Cleanups of extra includes and declaration
Changes for v3:
   - Move test command to cmd/
   - Update Kconfig names and depends
   - clean up default canary initialization
Changes for v2:
   - Add test command and corresponding config
   - Fixed incorrect description in Kconfig
   - Add unit test
---
 MAINTAINERS                          |  7 +++++++
 Makefile                             |  5 +++++
 cmd/Kconfig                          | 10 ++++++++++
 cmd/Makefile                         |  1 +
 cmd/stackprot_test.c                 | 21 +++++++++++++++++++++
 common/Kconfig                       | 17 +++++++++++++++++
 common/Makefile                      |  2 ++
 common/stackprot.c                   | 19 +++++++++++++++++++
 configs/sandbox_defconfig            | 14 +++++++-------
 scripts/Makefile.spl                 |  6 ++++++
 test/py/tests/test_stackprotector.py | 15 +++++++++++++++
 11 files changed, 110 insertions(+), 7 deletions(-)
 create mode 100644 cmd/stackprot_test.c
 create mode 100644 common/stackprot.c
 create mode 100644 test/py/tests/test_stackprotector.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 26dd254..d3971e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1024,6 +1024,13 @@ F:	include/sqfs.h
 F:	cmd/sqfs.c
 F:	test/py/tests/test_fs/test_squashfs/
 
+STACKPROTECTOR
+M:	Joel Peshkin <joel.peshkin@broadcom.com>
+S:	Maintained
+F:	common/stackprot.c
+F:	cmd/stackprot_test.c
+F:	test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M:	Bharat Gooty <bharat.gooty@broadcom.com>
 M:	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
diff --git a/Makefile b/Makefile
index 902a976..65c5cb8 100644
--- a/Makefile
+++ b/Makefile
@@ -677,7 +677,12 @@ else
 KBUILD_CFLAGS	+= -O2
 endif
 
+ifeq ($(CONFIG_STACKPROTECTOR),y)
+KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
+CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
+else
 KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
+endif
 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
 
 # disable stringop warnings in gcc 8+
diff --git a/cmd/Kconfig b/cmd/Kconfig
index da86a94..054b2f3 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2280,6 +2280,16 @@ config CMD_AVB
 	    avb read_part_hex - read data from partition and output to stdout
 	    avb write_part - write data to partition
 	    avb verify - run full verification chain
+
+config CMD_STACKPROTECTOR_TEST
+	bool "Test command for stack protector"
+	depends on STACKPROTECTOR
+	default n
+	help
+	  Enable stackprot_test command
+	  The stackprot_test command will force a stack overrun to test
+	  the stack smashing detection mechanisms.
+
 endmenu
 
 config CMD_UBI
diff --git a/cmd/Makefile b/cmd/Makefile
index 5b3400a..1d7afea 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
 obj-$(CONFIG_CMD_STRINGS) += strings.o
 obj-$(CONFIG_CMD_SMC) += smccc.o
 obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
+obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
 obj-$(CONFIG_CMD_TERMINAL) += terminal.o
 obj-$(CONFIG_CMD_TIME) += time.o
 obj-$(CONFIG_CMD_TIMER) += timer.o
diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
new file mode 100644
index 0000000..6ad287e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+#include <command.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+				  char *const argv[])
+{
+	char a[128];
+
+	memset(a, 0xa5, 512);
+	return 0;
+}
+
+U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
+	   "test stack protector fail", "");
diff --git a/common/Kconfig b/common/Kconfig
index 2bce8c9..6a94045 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -595,6 +595,23 @@ config TPL_HASH
 	  and the algorithms it supports are defined in common/hash.c. See
 	  also CMD_HASH for command-line access.
 
+config STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection"
+	default n
+	help
+	  Enable stack smash detection through compiler's stack-protector
+	  canary logic
+
+config SPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for SPL"
+	depends on STACKPROTECTOR && SPL
+	default n
+
+config TPL_STACKPROTECTOR
+	bool "Stack Protector buffer overflow detection for TPL"
+	depends on STACKPROTECTOR && TPL
+	default n
+
 endmenu
 
 menu "Update support"
diff --git a/common/Makefile b/common/Makefile
index bcf352d..fe71e18 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
 obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
 
 obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
+obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
+
diff --git a/common/stackprot.c b/common/stackprot.c
new file mode 100644
index 0000000..282c564
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard = (unsigned long)(0xfeedf00ddeadbeef & ~0UL);
+
+void __stack_chk_fail(void)
+{
+	void *ra;
+
+	ra = __builtin_extract_return_addr(__builtin_return_address(0));
+	panic("Stack smashing detected in function:\n%p relocated from %p",
+	      ra, ra - gd->reloc_off);
+}
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 58d4ef1..0c82ef2 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -20,11 +20,11 @@ CONFIG_BOOTSTAGE_STASH=y
 CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
 CONFIG_CONSOLE_RECORD=y
 CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
-CONFIG_SILENT_CONSOLE=y
 CONFIG_PRE_CONSOLE_BUFFER=y
 CONFIG_LOG_SYSLOG=y
 CONFIG_LOG_ERROR_RETURN=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_STACKPROTECTOR=y
 CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
@@ -96,6 +96,7 @@ CONFIG_CMD_CRAMFS=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_SQUASHFS=y
 CONFIG_CMD_MTDPARTS=y
+CONFIG_CMD_STACKPROTECTOR_TEST=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
@@ -131,6 +132,7 @@ CONFIG_CPU=y
 CONFIG_DM_DEMO=y
 CONFIG_DM_DEMO_SIMPLE=y
 CONFIG_DM_DEMO_SHAPE=y
+CONFIG_DFU_SF=y
 CONFIG_DMA=y
 CONFIG_DMA_CHANNELS=y
 CONFIG_SANDBOX_DMA=y
@@ -269,14 +271,12 @@ CONFIG_CMD_DHRYSTONE=y
 CONFIG_TPM=y
 CONFIG_LZ4=y
 CONFIG_ERRNO_STR=y
+CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
+CONFIG_EFI_CAPSULE_ON_DISK=y
+CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
+CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
-#
-CONFIG_DFU_SF=y
-CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
-CONFIG_EFI_CAPSULE_ON_DISK=y
-CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
-CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 87021e2..6725201 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -67,6 +67,12 @@ include $(srctree)/scripts/Makefile.lib
 KBUILD_CFLAGS += -ffunction-sections -fdata-sections
 LDFLAGS_FINAL += --gc-sections
 
+ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
+KBUILD_CFLAGS += -fstack-protector-strong
+else
+KBUILD_CFLAGS += -fno-stack-protector
+endif
+
 # FIX ME
 cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
 							$(NOSTDINC_FLAGS)
diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
new file mode 100644
index 0000000..7aeec5e
--- /dev/null
+++ b/test/py/tests/test_stackprotector.py
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Broadcom
+
+import pytest
+import signal
+
+ at pytest.mark.buildconfigspec('cmd_stackprotector_test')
+def test_stackprotector(u_boot_console):
+    """Test that the stackprotector function works."""
+
+    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
+    expected_response = 'Stack smashing detected'
+    u_boot_console.wait_for(expected_response)
+    u_boot_console.restart_uboot()
+
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH v9] Add support for stack-protector
       [not found]   ` <794783f5-da5c-65b4-82a4-0f62d1f6a8b0@gmx.de>
@ 2021-02-09 20:39     ` Heinrich Schuchardt
  0 siblings, 0 replies; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-02-09 20:39 UTC (permalink / raw)
  To: u-boot

On 09.02.21 13:49, Heinrich Schuchardt wrote:
> On 09.02.21 04:36, Joel Peshkin wrote:
>> Add support for stack protector for UBOOT, SPL, and TPL
>> as well as new pytest for stackprotector
>>
>> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
>
> Before merging the patch the stack smash in
>
> test/py/tests/test_efi_capsule/test_capsule_firmware.py
> function efi_launch_capsules()
>
> must be fixed.

This patch fixes the problem with
test/py/tests/test_efi_capsule/test_capsule_firmware.py:

https://lists.denx.de/pipermail/u-boot/2021-February/440872.html
[PATCH 1/1] efi_loader: fix get_last_capsule()

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v9] Add support for stack-protector
  2021-02-09  3:36 ` [PATCH v9] " Joel Peshkin
       [not found]   ` <794783f5-da5c-65b4-82a4-0f62d1f6a8b0@gmx.de>
@ 2021-03-22 17:37   ` Heinrich Schuchardt
  2021-04-09 22:27     ` Joel Peshkin
  1 sibling, 1 reply; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-03-22 17:37 UTC (permalink / raw)
  To: u-boot

On 09.02.21 04:36, Joel Peshkin wrote:
> Add support for stack protector for UBOOT, SPL, and TPL
> as well as new pytest for stackprotector
>
> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
> ---
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> Changes for v9:
>    - Fix pytest script post-test reboot
> Changes for v8:
>    - Fix commit message
>    - Force canary to UL type
> Changes for v7:
>    - Fix commit message
>    - add __builtin_extract_return_addr() calls
> Changes for v6:
>    - Fix commit message
> Changes for v5:
>    - Rebase
> Changes for v4:
>    - Exclude EFI from stackprotector
>    - Cleanups of extra includes and declaration
> Changes for v3:
>    - Move test command to cmd/
>    - Update Kconfig names and depends
>    - clean up default canary initialization
> Changes for v2:
>    - Add test command and corresponding config
>    - Fixed incorrect description in Kconfig
>    - Add unit test
> ---
>  MAINTAINERS                          |  7 +++++++
>  Makefile                             |  5 +++++
>  cmd/Kconfig                          | 10 ++++++++++
>  cmd/Makefile                         |  1 +
>  cmd/stackprot_test.c                 | 21 +++++++++++++++++++++
>  common/Kconfig                       | 17 +++++++++++++++++
>  common/Makefile                      |  2 ++
>  common/stackprot.c                   | 19 +++++++++++++++++++
>  configs/sandbox_defconfig            | 14 +++++++-------
>  scripts/Makefile.spl                 |  6 ++++++
>  test/py/tests/test_stackprotector.py | 15 +++++++++++++++
>  11 files changed, 110 insertions(+), 7 deletions(-)
>  create mode 100644 cmd/stackprot_test.c
>  create mode 100644 common/stackprot.c
>  create mode 100644 test/py/tests/test_stackprotector.py
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 26dd254..d3971e8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1024,6 +1024,13 @@ F:	include/sqfs.h
>  F:	cmd/sqfs.c
>  F:	test/py/tests/test_fs/test_squashfs/
>
> +STACKPROTECTOR
> +M:	Joel Peshkin <joel.peshkin@broadcom.com>
> +S:	Maintained
> +F:	common/stackprot.c
> +F:	cmd/stackprot_test.c
> +F:	test/py/tests/test_stackprotector.py
> +
>  TARGET_BCMNS3
>  M:	Bharat Gooty <bharat.gooty@broadcom.com>
>  M:	Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> diff --git a/Makefile b/Makefile
> index 902a976..65c5cb8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -677,7 +677,12 @@ else
>  KBUILD_CFLAGS	+= -O2
>  endif
>
> +ifeq ($(CONFIG_STACKPROTECTOR),y)
> +KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
> +CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
> +else
>  KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
> +endif
>  KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
>
>  # disable stringop warnings in gcc 8+
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index da86a94..054b2f3 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2280,6 +2280,16 @@ config CMD_AVB
>  	    avb read_part_hex - read data from partition and output to stdout
>  	    avb write_part - write data to partition
>  	    avb verify - run full verification chain
> +
> +config CMD_STACKPROTECTOR_TEST
> +	bool "Test command for stack protector"
> +	depends on STACKPROTECTOR
> +	default n
> +	help
> +	  Enable stackprot_test command
> +	  The stackprot_test command will force a stack overrun to test
> +	  the stack smashing detection mechanisms.
> +
>  endmenu
>
>  config CMD_UBI
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 5b3400a..1d7afea 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
>  obj-$(CONFIG_CMD_STRINGS) += strings.o
>  obj-$(CONFIG_CMD_SMC) += smccc.o
>  obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
> +obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
>  obj-$(CONFIG_CMD_TERMINAL) += terminal.o
>  obj-$(CONFIG_CMD_TIME) += time.o
>  obj-$(CONFIG_CMD_TIMER) += timer.o
> diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
> new file mode 100644
> index 0000000..6ad287e
> --- /dev/null
> +++ b/cmd/stackprot_test.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright 2021 Broadcom
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;

Hello Jo?l,

This line is not needed.

> +
> +static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
> +				  char *const argv[])
> +{
> +	char a[128];
> +
> +	memset(a, 0xa5, 512);
> +	return 0;
> +}
> +
> +U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
> +	   "test stack protector fail", "");
> diff --git a/common/Kconfig b/common/Kconfig
> index 2bce8c9..6a94045 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -595,6 +595,23 @@ config TPL_HASH
>  	  and the algorithms it supports are defined in common/hash.c. See
>  	  also CMD_HASH for command-line access.
>
> +config STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection"
> +	default n
> +	help
> +	  Enable stack smash detection through compiler's stack-protector
> +	  canary logic
> +
> +config SPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for SPL"
> +	depends on STACKPROTECTOR && SPL
> +	default n
> +
> +config TPL_STACKPROTECTOR
> +	bool "Stack Protector buffer overflow detection for TPL"
> +	depends on STACKPROTECTOR && TPL
> +	default n
> +
>  endmenu
>
>  menu "Update support"
> diff --git a/common/Makefile b/common/Makefile
> index bcf352d..fe71e18 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
>  obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
>
>  obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
> +obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
> +
> diff --git a/common/stackprot.c b/common/stackprot.c
> new file mode 100644
> index 0000000..282c564
> --- /dev/null
> +++ b/common/stackprot.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright 2021 Broadcom
> + */
> +
> +#include <common.h>

You need

    #include <asm/global_data.h>

here due to recently merged restructuring of includes by Simon.

> +
> +DECLARE_GLOBAL_DATA_PTR;

common/stackprot.c:8:1: warning: data definition has no type or storage
class
    8 | DECLARE_GLOBAL_DATA_PTR;
      | ^~~~~~~~~~~~~~~~~~~~~~~
common/stackprot.c:8:1: warning: type defaults to ?int? in declaration
of ?DECLARE_GLOBAL_DATA_PTR? [-Wimplicit-int]
common/stackprot.c: In function ?__stack_chk_fail?:
common/stackprot.c:18:17: error: ?gd? undeclared (first use in this
function)
   18 |        ra, ra - gd->reloc_off);


> +
> +unsigned long __stack_chk_guard = (unsigned long)(0xfeedf00ddeadbeef & ~0UL);
> +
> +void __stack_chk_fail(void)
> +{
> +	void *ra;
> +
> +	ra = __builtin_extract_return_addr(__builtin_return_address(0));
> +	panic("Stack smashing detected in function:\n%p relocated from %p",
> +	      ra, ra - gd->reloc_off);
> +}
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 58d4ef1..0c82ef2 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -20,11 +20,11 @@ CONFIG_BOOTSTAGE_STASH=y
>  CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
>  CONFIG_CONSOLE_RECORD=y
>  CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
> -CONFIG_SILENT_CONSOLE=y
>  CONFIG_PRE_CONSOLE_BUFFER=y
>  CONFIG_LOG_SYSLOG=y
>  CONFIG_LOG_ERROR_RETURN=y
>  CONFIG_DISPLAY_BOARDINFO_LATE=y
> +CONFIG_STACKPROTECTOR=y
>  CONFIG_ANDROID_AB=y
>  CONFIG_CMD_CPU=y
>  CONFIG_CMD_LICENSE=y
> @@ -96,6 +96,7 @@ CONFIG_CMD_CRAMFS=y
>  CONFIG_CMD_EXT4_WRITE=y
>  CONFIG_CMD_SQUASHFS=y
>  CONFIG_CMD_MTDPARTS=y
> +CONFIG_CMD_STACKPROTECTOR_TEST=y
>  CONFIG_MAC_PARTITION=y
>  CONFIG_AMIGA_PARTITION=y
>  CONFIG_OF_CONTROL=y
> @@ -131,6 +132,7 @@ CONFIG_CPU=y
>  CONFIG_DM_DEMO=y
>  CONFIG_DM_DEMO_SIMPLE=y
>  CONFIG_DM_DEMO_SHAPE=y
> +CONFIG_DFU_SF=y
>  CONFIG_DMA=y
>  CONFIG_DMA_CHANNELS=y
>  CONFIG_SANDBOX_DMA=y
> @@ -269,14 +271,12 @@ CONFIG_CMD_DHRYSTONE=y
>  CONFIG_TPM=y
>  CONFIG_LZ4=y
>  CONFIG_ERRNO_STR=y
> +CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> +CONFIG_EFI_CAPSULE_ON_DISK=y
> +CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
>  CONFIG_EFI_SECURE_BOOT=y
>  CONFIG_TEST_FDTDEC=y
>  CONFIG_UNIT_TEST=y
>  CONFIG_UT_TIME=y
>  CONFIG_UT_DM=y
> -#
> -CONFIG_DFU_SF=y
> -CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> -CONFIG_EFI_CAPSULE_ON_DISK=y
> -CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> -CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index 87021e2..6725201 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -67,6 +67,12 @@ include $(srctree)/scripts/Makefile.lib
>  KBUILD_CFLAGS += -ffunction-sections -fdata-sections
>  LDFLAGS_FINAL += --gc-sections
>
> +ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
> +KBUILD_CFLAGS += -fstack-protector-strong
> +else
> +KBUILD_CFLAGS += -fno-stack-protector
> +endif
> +
>  # FIX ME
>  cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
>  							$(NOSTDINC_FLAGS)
> diff --git a/test/py/tests/test_stackprotector.py b/test/py/tests/test_stackprotector.py
> new file mode 100644
> index 0000000..7aeec5e
> --- /dev/null
> +++ b/test/py/tests/test_stackprotector.py
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Broadcom
> +
> +import pytest
> +import signal
> +
> + at pytest.mark.buildconfigspec('cmd_stackprotector_test')
> +def test_stackprotector(u_boot_console):
> +    """Test that the stackprotector function works."""
> +
> +    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
> +    expected_response = 'Stack smashing detected'
> +    u_boot_console.wait_for(expected_response)
> +    u_boot_console.restart_uboot()
> +
>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v9] Add support for stack-protector
  2021-03-22 17:37   ` Heinrich Schuchardt
@ 2021-04-09 22:27     ` Joel Peshkin
  2021-04-10 10:11       ` Heinrich Schuchardt
  0 siblings, 1 reply; 29+ messages in thread
From: Joel Peshkin @ 2021-04-09 22:27 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

Has there been any progress in getting the EFI erors fixed so that this can
be committed?  There seems to be little point in my refreshing this patch
until that is done.

Thanks,

-Joel


On Mon, Mar 22, 2021 at 10:37 AM Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:

> On 09.02.21 04:36, Joel Peshkin wrote:
> > Add support for stack protector for UBOOT, SPL, and TPL
> > as well as new pytest for stackprotector
> >
> > Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
> > ---
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >
> > Changes for v9:
> >    - Fix pytest script post-test reboot
> > Changes for v8:
> >    - Fix commit message
> >    - Force canary to UL type
> > Changes for v7:
> >    - Fix commit message
> >    - add __builtin_extract_return_addr() calls
> > Changes for v6:
> >    - Fix commit message
> > Changes for v5:
> >    - Rebase
> > Changes for v4:
> >    - Exclude EFI from stackprotector
> >    - Cleanups of extra includes and declaration
> > Changes for v3:
> >    - Move test command to cmd/
> >    - Update Kconfig names and depends
> >    - clean up default canary initialization
> > Changes for v2:
> >    - Add test command and corresponding config
> >    - Fixed incorrect description in Kconfig
> >    - Add unit test
> > ---
> >  MAINTAINERS                          |  7 +++++++
> >  Makefile                             |  5 +++++
> >  cmd/Kconfig                          | 10 ++++++++++
> >  cmd/Makefile                         |  1 +
> >  cmd/stackprot_test.c                 | 21 +++++++++++++++++++++
> >  common/Kconfig                       | 17 +++++++++++++++++
> >  common/Makefile                      |  2 ++
> >  common/stackprot.c                   | 19 +++++++++++++++++++
> >  configs/sandbox_defconfig            | 14 +++++++-------
> >  scripts/Makefile.spl                 |  6 ++++++
> >  test/py/tests/test_stackprotector.py | 15 +++++++++++++++
> >  11 files changed, 110 insertions(+), 7 deletions(-)
> >  create mode 100644 cmd/stackprot_test.c
> >  create mode 100644 common/stackprot.c
> >  create mode 100644 test/py/tests/test_stackprotector.py
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 26dd254..d3971e8 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1024,6 +1024,13 @@ F:     include/sqfs.h
> >  F:   cmd/sqfs.c
> >  F:   test/py/tests/test_fs/test_squashfs/
> >
> > +STACKPROTECTOR
> > +M:   Joel Peshkin <joel.peshkin@broadcom.com>
> > +S:   Maintained
> > +F:   common/stackprot.c
> > +F:   cmd/stackprot_test.c
> > +F:   test/py/tests/test_stackprotector.py
> > +
> >  TARGET_BCMNS3
> >  M:   Bharat Gooty <bharat.gooty@broadcom.com>
> >  M:   Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> > diff --git a/Makefile b/Makefile
> > index 902a976..65c5cb8 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -677,7 +677,12 @@ else
> >  KBUILD_CFLAGS        += -O2
> >  endif
> >
> > +ifeq ($(CONFIG_STACKPROTECTOR),y)
> > +KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
> > +CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
> > +else
> >  KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
> > +endif
> >  KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
> >
> >  # disable stringop warnings in gcc 8+
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index da86a94..054b2f3 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -2280,6 +2280,16 @@ config CMD_AVB
> >           avb read_part_hex - read data from partition and output to
> stdout
> >           avb write_part - write data to partition
> >           avb verify - run full verification chain
> > +
> > +config CMD_STACKPROTECTOR_TEST
> > +     bool "Test command for stack protector"
> > +     depends on STACKPROTECTOR
> > +     default n
> > +     help
> > +       Enable stackprot_test command
> > +       The stackprot_test command will force a stack overrun to test
> > +       the stack smashing detection mechanisms.
> > +
> >  endmenu
> >
> >  config CMD_UBI
> > diff --git a/cmd/Makefile b/cmd/Makefile
> > index 5b3400a..1d7afea 100644
> > --- a/cmd/Makefile
> > +++ b/cmd/Makefile
> > @@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
> >  obj-$(CONFIG_CMD_STRINGS) += strings.o
> >  obj-$(CONFIG_CMD_SMC) += smccc.o
> >  obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
> > +obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
> >  obj-$(CONFIG_CMD_TERMINAL) += terminal.o
> >  obj-$(CONFIG_CMD_TIME) += time.o
> >  obj-$(CONFIG_CMD_TIMER) += timer.o
> > diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
> > new file mode 100644
> > index 0000000..6ad287e
> > --- /dev/null
> > +++ b/cmd/stackprot_test.c
> > @@ -0,0 +1,21 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + *  Copyright 2021 Broadcom
> > + */
> > +
> > +#include <common.h>
> > +#include <command.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
>
> Hello Jo?l,
>
> This line is not needed.
>
> > +
> > +static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int
> argc,
> > +                               char *const argv[])
> > +{
> > +     char a[128];
> > +
> > +     memset(a, 0xa5, 512);
> > +     return 0;
> > +}
> > +
> > +U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
> > +        "test stack protector fail", "");
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 2bce8c9..6a94045 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -595,6 +595,23 @@ config TPL_HASH
> >         and the algorithms it supports are defined in common/hash.c. See
> >         also CMD_HASH for command-line access.
> >
> > +config STACKPROTECTOR
> > +     bool "Stack Protector buffer overflow detection"
> > +     default n
> > +     help
> > +       Enable stack smash detection through compiler's stack-protector
> > +       canary logic
> > +
> > +config SPL_STACKPROTECTOR
> > +     bool "Stack Protector buffer overflow detection for SPL"
> > +     depends on STACKPROTECTOR && SPL
> > +     default n
> > +
> > +config TPL_STACKPROTECTOR
> > +     bool "Stack Protector buffer overflow detection for TPL"
> > +     depends on STACKPROTECTOR && TPL
> > +     default n
> > +
> >  endmenu
> >
> >  menu "Update support"
> > diff --git a/common/Makefile b/common/Makefile
> > index bcf352d..fe71e18 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
> >  obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
> >
> >  obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
> > +obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
> > +
> > diff --git a/common/stackprot.c b/common/stackprot.c
> > new file mode 100644
> > index 0000000..282c564
> > --- /dev/null
> > +++ b/common/stackprot.c
> > @@ -0,0 +1,19 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + *  Copyright 2021 Broadcom
> > + */
> > +
> > +#include <common.h>
>
> You need
>
>     #include <asm/global_data.h>
>
> here due to recently merged restructuring of includes by Simon.
>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
>
> common/stackprot.c:8:1: warning: data definition has no type or storage
> class
>     8 | DECLARE_GLOBAL_DATA_PTR;
>       | ^~~~~~~~~~~~~~~~~~~~~~~
> common/stackprot.c:8:1: warning: type defaults to ?int? in declaration
> of ?DECLARE_GLOBAL_DATA_PTR? [-Wimplicit-int]
> common/stackprot.c: In function ?__stack_chk_fail?:
> common/stackprot.c:18:17: error: ?gd? undeclared (first use in this
> function)
>    18 |        ra, ra - gd->reloc_off);
>
>
> > +
> > +unsigned long __stack_chk_guard = (unsigned long)(0xfeedf00ddeadbeef &
> ~0UL);
> > +
> > +void __stack_chk_fail(void)
> > +{
> > +     void *ra;
> > +
> > +     ra = __builtin_extract_return_addr(__builtin_return_address(0));
> > +     panic("Stack smashing detected in function:\n%p relocated from %p",
> > +           ra, ra - gd->reloc_off);
> > +}
> > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > index 58d4ef1..0c82ef2 100644
> > --- a/configs/sandbox_defconfig
> > +++ b/configs/sandbox_defconfig
> > @@ -20,11 +20,11 @@ CONFIG_BOOTSTAGE_STASH=y
> >  CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
> >  CONFIG_CONSOLE_RECORD=y
> >  CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
> > -CONFIG_SILENT_CONSOLE=y
> >  CONFIG_PRE_CONSOLE_BUFFER=y
> >  CONFIG_LOG_SYSLOG=y
> >  CONFIG_LOG_ERROR_RETURN=y
> >  CONFIG_DISPLAY_BOARDINFO_LATE=y
> > +CONFIG_STACKPROTECTOR=y
> >  CONFIG_ANDROID_AB=y
> >  CONFIG_CMD_CPU=y
> >  CONFIG_CMD_LICENSE=y
> > @@ -96,6 +96,7 @@ CONFIG_CMD_CRAMFS=y
> >  CONFIG_CMD_EXT4_WRITE=y
> >  CONFIG_CMD_SQUASHFS=y
> >  CONFIG_CMD_MTDPARTS=y
> > +CONFIG_CMD_STACKPROTECTOR_TEST=y
> >  CONFIG_MAC_PARTITION=y
> >  CONFIG_AMIGA_PARTITION=y
> >  CONFIG_OF_CONTROL=y
> > @@ -131,6 +132,7 @@ CONFIG_CPU=y
> >  CONFIG_DM_DEMO=y
> >  CONFIG_DM_DEMO_SIMPLE=y
> >  CONFIG_DM_DEMO_SHAPE=y
> > +CONFIG_DFU_SF=y
> >  CONFIG_DMA=y
> >  CONFIG_DMA_CHANNELS=y
> >  CONFIG_SANDBOX_DMA=y
> > @@ -269,14 +271,12 @@ CONFIG_CMD_DHRYSTONE=y
> >  CONFIG_TPM=y
> >  CONFIG_LZ4=y
> >  CONFIG_ERRNO_STR=y
> > +CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> > +CONFIG_EFI_CAPSULE_ON_DISK=y
> > +CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> > +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> >  CONFIG_EFI_SECURE_BOOT=y
> >  CONFIG_TEST_FDTDEC=y
> >  CONFIG_UNIT_TEST=y
> >  CONFIG_UT_TIME=y
> >  CONFIG_UT_DM=y
> > -#
> > -CONFIG_DFU_SF=y
> > -CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> > -CONFIG_EFI_CAPSULE_ON_DISK=y
> > -CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> > -CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> > index 87021e2..6725201 100644
> > --- a/scripts/Makefile.spl
> > +++ b/scripts/Makefile.spl
> > @@ -67,6 +67,12 @@ include $(srctree)/scripts/Makefile.lib
> >  KBUILD_CFLAGS += -ffunction-sections -fdata-sections
> >  LDFLAGS_FINAL += --gc-sections
> >
> > +ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
> > +KBUILD_CFLAGS += -fstack-protector-strong
> > +else
> > +KBUILD_CFLAGS += -fno-stack-protector
> > +endif
> > +
> >  # FIX ME
> >  cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS) $(UBOOTINCLUDE) \
> >                                                       $(NOSTDINC_FLAGS)
> > diff --git a/test/py/tests/test_stackprotector.py
> b/test/py/tests/test_stackprotector.py
> > new file mode 100644
> > index 0000000..7aeec5e
> > --- /dev/null
> > +++ b/test/py/tests/test_stackprotector.py
> > @@ -0,0 +1,15 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2021 Broadcom
> > +
> > +import pytest
> > +import signal
> > +
> > + at pytest.mark.buildconfigspec('cmd_stackprotector_test')
> > +def test_stackprotector(u_boot_console):
> > +    """Test that the stackprotector function works."""
> > +
> > +    u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
> > +    expected_response = 'Stack smashing detected'
> > +    u_boot_console.wait_for(expected_response)
> > +    u_boot_console.restart_uboot()
> > +
> >
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4209 bytes
Desc: S/MIME Cryptographic Signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210409/3dc53395/attachment.bin>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v9] Add support for stack-protector
  2021-04-09 22:27     ` Joel Peshkin
@ 2021-04-10 10:11       ` Heinrich Schuchardt
  2021-04-10 11:17         ` Tom Rini
  0 siblings, 1 reply; 29+ messages in thread
From: Heinrich Schuchardt @ 2021-04-10 10:11 UTC (permalink / raw)
  To: u-boot

On 4/10/21 12:27 AM, Joel Peshkin wrote:
>
> Hi Heinrich,
>
> Has there been any progress in getting the EFI erors?fixed so that this
> can be committed?? There seems to be little point in my refreshing this
> patch until that is done.

I have fixed up your patch to work with EFI and will add it to my next
pull request.

I did not get qemu-x86_64_defconfig and qemu-x86_defconfig to build with
CONFIG_STACK_PROTECTOR=y, CONFIG_SPL_STACK_PROTECTOR = no.

arch/x86/cpu/irq.c and arch/x86/cpu/mtrr.c give me an undefined
reference to `__stack_chk_fail' when SPL is built. It seems that these
files are only built once instead of separately for SPL and main U-Boot.

Best regards

Heinrich

>
> Thanks,
>
> -Joel
>
>
> On Mon, Mar 22, 2021 at 10:37 AM Heinrich Schuchardt <xypron.glpk@gmx.de
> <mailto:xypron.glpk@gmx.de>> wrote:
>
>     On 09.02.21 04:36, Joel Peshkin wrote:
>      > Add support for stack protector for UBOOT, SPL, and TPL
>      > as well as new pytest for stackprotector
>      >
>      > Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com
>     <mailto:joel.peshkin@broadcom.com>>
>      > ---
>      > Cc: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org>>
>      > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de
>     <mailto:xypron.glpk@gmx.de>>
>      >
>      > Changes for v9:
>      >? ? - Fix pytest script post-test reboot
>      > Changes for v8:
>      >? ? - Fix commit message
>      >? ? - Force canary to UL type
>      > Changes for v7:
>      >? ? - Fix commit message
>      >? ? - add __builtin_extract_return_addr() calls
>      > Changes for v6:
>      >? ? - Fix commit message
>      > Changes for v5:
>      >? ? - Rebase
>      > Changes for v4:
>      >? ? - Exclude EFI from stackprotector
>      >? ? - Cleanups of extra includes and declaration
>      > Changes for v3:
>      >? ? - Move test command to cmd/
>      >? ? - Update Kconfig names and depends
>      >? ? - clean up default canary initialization
>      > Changes for v2:
>      >? ? - Add test command and corresponding config
>      >? ? - Fixed incorrect description in Kconfig
>      >? ? - Add unit test
>      > ---
>      >? MAINTAINERS? ? ? ? ? ? ? ? ? ? ? ? ? |? 7 +++++++
>      >? Makefile? ? ? ? ? ? ? ? ? ? ? ? ? ? ?|? 5 +++++
>      >? cmd/Kconfig? ? ? ? ? ? ? ? ? ? ? ? ? | 10 ++++++++++
>      >? cmd/Makefile? ? ? ? ? ? ? ? ? ? ? ? ?|? 1 +
>      >? cmd/stackprot_test.c? ? ? ? ? ? ? ? ?| 21 +++++++++++++++++++++
>      >? common/Kconfig? ? ? ? ? ? ? ? ? ? ? ?| 17 +++++++++++++++++
>      >? common/Makefile? ? ? ? ? ? ? ? ? ? ? |? 2 ++
>      >? common/stackprot.c? ? ? ? ? ? ? ? ? ?| 19 +++++++++++++++++++
>      >? configs/sandbox_defconfig? ? ? ? ? ? | 14 +++++++-------
>      >? scripts/Makefile.spl? ? ? ? ? ? ? ? ?|? 6 ++++++
>      >? test/py/tests/test_stackprotector.py | 15 +++++++++++++++
>      >? 11 files changed, 110 insertions(+), 7 deletions(-)
>      >? create mode 100644 cmd/stackprot_test.c
>      >? create mode 100644 common/stackprot.c
>      >? create mode 100644 test/py/tests/test_stackprotector.py
>      >
>      > diff --git a/MAINTAINERS b/MAINTAINERS
>      > index 26dd254..d3971e8 100644
>      > --- a/MAINTAINERS
>      > +++ b/MAINTAINERS
>      > @@ -1024,6 +1024,13 @@ F:? ? ?include/sqfs.h
>      >? F:? ?cmd/sqfs.c
>      >? F:? ?test/py/tests/test_fs/test_squashfs/
>      >
>      > +STACKPROTECTOR
>      > +M:? ?Joel Peshkin <joel.peshkin@broadcom.com
>     <mailto:joel.peshkin@broadcom.com>>
>      > +S:? ?Maintained
>      > +F:? ?common/stackprot.c
>      > +F:? ?cmd/stackprot_test.c
>      > +F:? ?test/py/tests/test_stackprotector.py
>      > +
>      >? TARGET_BCMNS3
>      >? M:? ?Bharat Gooty <bharat.gooty@broadcom.com
>     <mailto:bharat.gooty@broadcom.com>>
>      >? M:? ?Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com
>     <mailto:rayagonda.kokatanur@broadcom.com>>
>      > diff --git a/Makefile b/Makefile
>      > index 902a976..65c5cb8 100644
>      > --- a/Makefile
>      > +++ b/Makefile
>      > @@ -677,7 +677,12 @@ else
>      >? KBUILD_CFLAGS? ? ? ? += -O2
>      >? endif
>      >
>      > +ifeq ($(CONFIG_STACKPROTECTOR),y)
>      > +KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
>      > +CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
>      > +else
>      >? KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
>      > +endif
>      >? KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
>      >
>      >? # disable stringop warnings in gcc 8+
>      > diff --git a/cmd/Kconfig b/cmd/Kconfig
>      > index da86a94..054b2f3 100644
>      > --- a/cmd/Kconfig
>      > +++ b/cmd/Kconfig
>      > @@ -2280,6 +2280,16 @@ config CMD_AVB
>      >? ? ? ? ? ?avb read_part_hex - read data from partition and output
>     to stdout
>      >? ? ? ? ? ?avb write_part - write data to partition
>      >? ? ? ? ? ?avb verify - run full verification chain
>      > +
>      > +config CMD_STACKPROTECTOR_TEST
>      > +? ? ?bool "Test command for stack protector"
>      > +? ? ?depends on STACKPROTECTOR
>      > +? ? ?default n
>      > +? ? ?help
>      > +? ? ? ?Enable stackprot_test command
>      > +? ? ? ?The stackprot_test command will force a stack overrun to test
>      > +? ? ? ?the stack smashing detection mechanisms.
>      > +
>      >? endmenu
>      >
>      >? config CMD_UBI
>      > diff --git a/cmd/Makefile b/cmd/Makefile
>      > index 5b3400a..1d7afea 100644
>      > --- a/cmd/Makefile
>      > +++ b/cmd/Makefile
>      > @@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
>      >? obj-$(CONFIG_CMD_STRINGS) += strings.o
>      >? obj-$(CONFIG_CMD_SMC) += smccc.o
>      >? obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
>      > +obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
>      >? obj-$(CONFIG_CMD_TERMINAL) += terminal.o
>      >? obj-$(CONFIG_CMD_TIME) += time.o
>      >? obj-$(CONFIG_CMD_TIMER) += timer.o
>      > diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
>      > new file mode 100644
>      > index 0000000..6ad287e
>      > --- /dev/null
>      > +++ b/cmd/stackprot_test.c
>      > @@ -0,0 +1,21 @@
>      > +// SPDX-License-Identifier: GPL-2.0+
>      > +/*
>      > + *? Copyright 2021 Broadcom
>      > + */
>      > +
>      > +#include <common.h>
>      > +#include <command.h>
>      > +
>      > +DECLARE_GLOBAL_DATA_PTR;
>
>     Hello Jo?l,
>
>     This line is not needed.
>
>      > +
>      > +static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int
>     flag, int argc,
>      > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?char *const argv[])
>      > +{
>      > +? ? ?char a[128];
>      > +
>      > +? ? ?memset(a, 0xa5, 512);
>      > +? ? ?return 0;
>      > +}
>      > +
>      > +U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
>      > +? ? ? ? "test stack protector fail", "");
>      > diff --git a/common/Kconfig b/common/Kconfig
>      > index 2bce8c9..6a94045 100644
>      > --- a/common/Kconfig
>      > +++ b/common/Kconfig
>      > @@ -595,6 +595,23 @@ config TPL_HASH
>      >? ? ? ? ?and the algorithms it supports are defined in
>     common/hash.c. See
>      >? ? ? ? ?also CMD_HASH for command-line access.
>      >
>      > +config STACKPROTECTOR
>      > +? ? ?bool "Stack Protector buffer overflow detection"
>      > +? ? ?default n
>      > +? ? ?help
>      > +? ? ? ?Enable stack smash detection through compiler's
>     stack-protector
>      > +? ? ? ?canary logic
>      > +
>      > +config SPL_STACKPROTECTOR
>      > +? ? ?bool "Stack Protector buffer overflow detection for SPL"
>      > +? ? ?depends on STACKPROTECTOR && SPL
>      > +? ? ?default n
>      > +
>      > +config TPL_STACKPROTECTOR
>      > +? ? ?bool "Stack Protector buffer overflow detection for TPL"
>      > +? ? ?depends on STACKPROTECTOR && TPL
>      > +? ? ?default n
>      > +
>      >? endmenu
>      >
>      >? menu "Update support"
>      > diff --git a/common/Makefile b/common/Makefile
>      > index bcf352d..fe71e18 100644
>      > --- a/common/Makefile
>      > +++ b/common/Makefile
>      > @@ -138,3 +138,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
>      >? obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
>      >
>      >? obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
>      > +obj-$(CONFIG_$(SPL_TPL_)STACKPROTECTOR) += stackprot.o
>      > +
>      > diff --git a/common/stackprot.c b/common/stackprot.c
>      > new file mode 100644
>      > index 0000000..282c564
>      > --- /dev/null
>      > +++ b/common/stackprot.c
>      > @@ -0,0 +1,19 @@
>      > +// SPDX-License-Identifier: GPL-2.0+
>      > +/*
>      > + *? Copyright 2021 Broadcom
>      > + */
>      > +
>      > +#include <common.h>
>
>     You need
>
>      ? ? #include <asm/global_data.h>
>
>     here due to recently merged restructuring of includes by Simon.
>
>      > +
>      > +DECLARE_GLOBAL_DATA_PTR;
>
>     common/stackprot.c:8:1: warning: data definition has no type or storage
>     class
>      ? ? 8 | DECLARE_GLOBAL_DATA_PTR;
>      ? ? ? | ^~~~~~~~~~~~~~~~~~~~~~~
>     common/stackprot.c:8:1: warning: type defaults to ?int? in declaration
>     of ?DECLARE_GLOBAL_DATA_PTR? [-Wimplicit-int]
>     common/stackprot.c: In function ?__stack_chk_fail?:
>     common/stackprot.c:18:17: error: ?gd? undeclared (first use in this
>     function)
>      ? ?18 |? ? ? ? ra, ra - gd->reloc_off);
>
>
>      > +
>      > +unsigned long __stack_chk_guard = (unsigned
>     long)(0xfeedf00ddeadbeef & ~0UL);
>      > +
>      > +void __stack_chk_fail(void)
>      > +{
>      > +? ? ?void *ra;
>      > +
>      > +? ? ?ra =
>     __builtin_extract_return_addr(__builtin_return_address(0));
>      > +? ? ?panic("Stack smashing detected in function:\n%p relocated
>     from %p",
>      > +? ? ? ? ? ?ra, ra - gd->reloc_off);
>      > +}
>      > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
>      > index 58d4ef1..0c82ef2 100644
>      > --- a/configs/sandbox_defconfig
>      > +++ b/configs/sandbox_defconfig
>      > @@ -20,11 +20,11 @@ CONFIG_BOOTSTAGE_STASH=y
>      >? CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
>      >? CONFIG_CONSOLE_RECORD=y
>      >? CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
>      > -CONFIG_SILENT_CONSOLE=y
>      >? CONFIG_PRE_CONSOLE_BUFFER=y
>      >? CONFIG_LOG_SYSLOG=y
>      >? CONFIG_LOG_ERROR_RETURN=y
>      >? CONFIG_DISPLAY_BOARDINFO_LATE=y
>      > +CONFIG_STACKPROTECTOR=y
>      >? CONFIG_ANDROID_AB=y
>      >? CONFIG_CMD_CPU=y
>      >? CONFIG_CMD_LICENSE=y
>      > @@ -96,6 +96,7 @@ CONFIG_CMD_CRAMFS=y
>      >? CONFIG_CMD_EXT4_WRITE=y
>      >? CONFIG_CMD_SQUASHFS=y
>      >? CONFIG_CMD_MTDPARTS=y
>      > +CONFIG_CMD_STACKPROTECTOR_TEST=y
>      >? CONFIG_MAC_PARTITION=y
>      >? CONFIG_AMIGA_PARTITION=y
>      >? CONFIG_OF_CONTROL=y
>      > @@ -131,6 +132,7 @@ CONFIG_CPU=y
>      >? CONFIG_DM_DEMO=y
>      >? CONFIG_DM_DEMO_SIMPLE=y
>      >? CONFIG_DM_DEMO_SHAPE=y
>      > +CONFIG_DFU_SF=y
>      >? CONFIG_DMA=y
>      >? CONFIG_DMA_CHANNELS=y
>      >? CONFIG_SANDBOX_DMA=y
>      > @@ -269,14 +271,12 @@ CONFIG_CMD_DHRYSTONE=y
>      >? CONFIG_TPM=y
>      >? CONFIG_LZ4=y
>      >? CONFIG_ERRNO_STR=y
>      > +CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
>      > +CONFIG_EFI_CAPSULE_ON_DISK=y
>      > +CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
>      > +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
>      >? CONFIG_EFI_SECURE_BOOT=y
>      >? CONFIG_TEST_FDTDEC=y
>      >? CONFIG_UNIT_TEST=y
>      >? CONFIG_UT_TIME=y
>      >? CONFIG_UT_DM=y
>      > -#
>      > -CONFIG_DFU_SF=y
>      > -CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
>      > -CONFIG_EFI_CAPSULE_ON_DISK=y
>      > -CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
>      > -CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
>      > diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
>      > index 87021e2..6725201 100644
>      > --- a/scripts/Makefile.spl
>      > +++ b/scripts/Makefile.spl
>      > @@ -67,6 +67,12 @@ include $(srctree)/scripts/Makefile.lib
>      >? KBUILD_CFLAGS += -ffunction-sections -fdata-sections
>      >? LDFLAGS_FINAL += --gc-sections
>      >
>      > +ifeq ($(CONFIG_$(SPL_TPL_)STACKPROTECTOR),y)
>      > +KBUILD_CFLAGS += -fstack-protector-strong
>      > +else
>      > +KBUILD_CFLAGS += -fno-stack-protector
>      > +endif
>      > +
>      >? # FIX ME
>      >? cpp_flags := $(KBUILD_CPPFLAGS) $(PLATFORM_CPPFLAGS)
>     $(UBOOTINCLUDE) \
>      >
>      ?$(NOSTDINC_FLAGS)
>      > diff --git a/test/py/tests/test_stackprotector.py
>     b/test/py/tests/test_stackprotector.py
>      > new file mode 100644
>      > index 0000000..7aeec5e
>      > --- /dev/null
>      > +++ b/test/py/tests/test_stackprotector.py
>      > @@ -0,0 +1,15 @@
>      > +# SPDX-License-Identifier: GPL-2.0
>      > +# Copyright (c) 2021 Broadcom
>      > +
>      > +import pytest
>      > +import signal
>      > +
>      > + at pytest.mark.buildconfigspec('cmd_stackprotector_test')
>      > +def test_stackprotector(u_boot_console):
>      > +? ? """Test that the stackprotector function works."""
>      > +
>      > +
>     u_boot_console.run_command('stackprot_test',wait_for_prompt=False)
>      > +? ? expected_response = 'Stack smashing detected'
>      > +? ? u_boot_console.wait_for(expected_response)
>      > +? ? u_boot_console.restart_uboot()
>      > +
>      >
>

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH v9] Add support for stack-protector
  2021-04-10 10:11       ` Heinrich Schuchardt
@ 2021-04-10 11:17         ` Tom Rini
  0 siblings, 0 replies; 29+ messages in thread
From: Tom Rini @ 2021-04-10 11:17 UTC (permalink / raw)
  To: u-boot

On Sat, Apr 10, 2021 at 12:11:33PM +0200, Heinrich Schuchardt wrote:
> On 4/10/21 12:27 AM, Joel Peshkin wrote:
> > 
> > Hi Heinrich,
> > 
> > Has there been any progress in getting the EFI erors?fixed so that this
> > can be committed?? There seems to be little point in my refreshing this
> > patch until that is done.
> 
> I have fixed up your patch to work with EFI and will add it to my next
> pull request.

If all of CI now works with -fstack-protector enabled, I'll take it up
in my tree.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210410/e007209b/attachment.sig>

^ permalink raw reply	[flat|nested] 29+ messages in thread
end of thread, other threads:[~2021-04-10 11:17 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-10 15:39 [PATCH] Add support for stack-protector Joel Peshkin
2021-01-10 16:18 ` Heinrich Schuchardt
2021-01-10 19:44   ` Joel Peshkin
2021-01-10 22:20     ` Heinrich Schuchardt
2021-01-10 22:40 ` Alex Sadovsky
2021-01-11  0:23   ` Joel Peshkin
2021-01-11  3:10 ` [PATCH v2] " Joel Peshkin
2021-01-11  9:59   ` Heinrich Schuchardt
2021-01-11 16:20 ` [PATCH v3] " Joel Peshkin
2021-01-11 18:12   ` Heinrich Schuchardt
2021-01-11 22:49 ` [PATCH v4] " Joel Peshkin
2021-01-12 15:48   ` Heinrich Schuchardt
2021-01-11 23:55 ` [PATCH v5] " Joel Peshkin
2021-01-12 16:51 ` [PATCH v6] " Joel Peshkin
2021-01-12 20:01   ` Heinrich Schuchardt
2021-01-14 13:59 ` [PATCH v7] " Joel Peshkin
2021-01-14 14:59   ` Alex Sadovsky
2021-01-14 20:35 ` [PATCH v8] " Joel Peshkin
2021-01-15 18:53   ` Heinrich Schuchardt
2021-01-28  0:57   ` Tom Rini
2021-01-28  8:20     ` Heinrich Schuchardt
2021-01-28 11:00       ` Heinrich Schuchardt
2021-01-28 14:33         ` Heinrich Schuchardt
2021-02-09  3:36 ` [PATCH v9] " Joel Peshkin
     [not found]   ` <794783f5-da5c-65b4-82a4-0f62d1f6a8b0@gmx.de>
2021-02-09 20:39     ` Heinrich Schuchardt
2021-03-22 17:37   ` Heinrich Schuchardt
2021-04-09 22:27     ` Joel Peshkin
2021-04-10 10:11       ` Heinrich Schuchardt
2021-04-10 11:17         ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.