On 11.01.21 23:49, Joel Peshkin wrote:
Somehow the commit message got lost.
> Cc: Simon Glass <s...@chromium.org>
> Cc: Bin Meng <bmeng...@gmail.com>
> Cc: Jagan Teki <ja...@amarulasolutions.com>
> Cc: Kever Yang <kever.y...@rock-chips.com>
> Cc: Heinrich Schuchardt <xypron.g...@gmx.de>
> Cc: AKASHI Takahiro <takahiro.aka...@linaro.org>
> Cc: Usama Arif <usama.a...@arm.com>
> Cc: Sam Protsenko <joe.s...@gmail.com>
> Cc: Masahiro Yamada <masahi...@kernel.org>
> Cc: Philippe Reynes <philippe.rey...@softathome.com>
> Cc: Eugeniu Rosca <roscaeuge...@gmail.com>
> Cc: Jan Kiszka <jan.kis...@siemens.com>
>
> Signed-off-by: Joel Peshkin <joel.pesh...@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.pesh...@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.go...@broadcom.com>
> M: Rayagonda Kokatanur <rayagonda.kokata...@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
> +
> +@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)
> +
>
