[PATCH] Add support for stack-protector

Heinrich Schuchardt
2021-01-10T16:18:56Z

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)
>