[PATCH] Add support for stack-protector

[Heinrich Schuchardt]

On 1/10/21 4:39 PM, Joel Peshkin wrote:
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Bin Meng <bmeng.cn at gmail.com>
> Cc: Jagan Teki <jagan at amarulasolutions.com>
> Cc: Kever Yang <kever.yang at rock-chips.com>
> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Cc: AKASHI Takahiro <takahiro.akashi at linaro.org>
> Cc: Usama Arif <usama.arif at arm.com>
> Cc: Sam Protsenko <joe.skb7 at gmail.com>
> Cc: Masahiro Yamada <masahiroy at kernel.org>
> Cc: Philippe Reynes <philippe.reynes at softathome.com>
> Cc: Eugeniu Rosca <roscaeugeniu at gmail.com>
> Cc: Jan Kiszka <jan.kiszka at siemens.com>
>
> Signed-off-by: Joel Peshkin <joel.peshkin at 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)
>