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 commandThe stackprot_test command will force a stack overrun to testthe 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-protectorcanary 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
+@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)