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-protectorcanary 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
+@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)