Message ID | 20210111031052.40327-1-joel.peshkin@broadcom.com |
---|---|
State | Changes Requested, archived |
Delegated to: | Tom Rini |
Headers | show |
Series | [v2] Add support for stack-protector | expand |
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 > + > +@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) > + >
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) +
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