diff mbox series

[v9] Add support for stack-protector

Message ID 20210209033607.70955-1-joel.peshkin@broadcom.com
State Superseded, archived
Delegated to: Tom Rini
Headers show
Series [v9] Add support for stack-protector | expand

Commit Message

Joel Peshkin Feb. 9, 2021, 3:36 a.m. UTC
Add support for stack protector for UBOOT, SPL, and TPL
as well as new pytest for stackprotector

Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
---
Cc: Simon Glass <sjg@chromium.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>

Changes for v9:
   - Fix pytest script post-test reboot
Changes for v8:
   - Fix commit message
   - Force canary to UL type
Changes for v7:
   - Fix commit message
   - add __builtin_extract_return_addr() calls
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                   | 19 +++++++++++++++++++
 configs/sandbox_defconfig            | 14 +++++++-------
 scripts/Makefile.spl                 |  6 ++++++
 test/py/tests/test_stackprotector.py | 15 +++++++++++++++
 11 files changed, 110 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

Comments

Heinrich Schuchardt Feb. 9, 2021, 8:39 p.m. UTC | #1
On 09.02.21 13:49, Heinrich Schuchardt wrote:
> On 09.02.21 04:36, Joel Peshkin wrote:
>> Add support for stack protector for UBOOT, SPL, and TPL
>> as well as new pytest for stackprotector
>>
>> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
>
> Before merging the patch the stack smash in
>
> test/py/tests/test_efi_capsule/test_capsule_firmware.py
> function efi_launch_capsules()
>
> must be fixed.

This patch fixes the problem with
test/py/tests/test_efi_capsule/test_capsule_firmware.py:

https://lists.denx.de/pipermail/u-boot/2021-February/440872.html
[PATCH 1/1] efi_loader: fix get_last_capsule()

Best regards

Heinrich
Heinrich Schuchardt March 22, 2021, 5:37 p.m. UTC | #2
On 09.02.21 04:36, Joel Peshkin wrote:
> Add support for stack protector for UBOOT, SPL, and TPL
> as well as new pytest for stackprotector
>
> Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
> ---
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> Changes for v9:
>    - Fix pytest script post-test reboot
> Changes for v8:
>    - Fix commit message
>    - Force canary to UL type
> Changes for v7:
>    - Fix commit message
>    - add __builtin_extract_return_addr() calls
> 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                   | 19 +++++++++++++++++++
>  configs/sandbox_defconfig            | 14 +++++++-------
>  scripts/Makefile.spl                 |  6 ++++++
>  test/py/tests/test_stackprotector.py | 15 +++++++++++++++
>  11 files changed, 110 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 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 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;

Hello Joël,

This line is not needed.

> +
> +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-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 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..282c564
> --- /dev/null
> +++ b/common/stackprot.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  Copyright 2021 Broadcom
> + */
> +
> +#include <common.h>

You need

    #include <asm/global_data.h>

here due to recently merged restructuring of includes by Simon.

> +
> +DECLARE_GLOBAL_DATA_PTR;

common/stackprot.c:8:1: warning: data definition has no type or storage
class
    8 | DECLARE_GLOBAL_DATA_PTR;
      | ^~~~~~~~~~~~~~~~~~~~~~~
common/stackprot.c:8:1: warning: type defaults to ‘int’ in declaration
of ‘DECLARE_GLOBAL_DATA_PTR’ [-Wimplicit-int]
common/stackprot.c: In function ‘__stack_chk_fail’:
common/stackprot.c:18:17: error: ‘gd’ undeclared (first use in this
function)
   18 |        ra, ra - gd->reloc_off);


> +
> +unsigned long __stack_chk_guard = (unsigned long)(0xfeedf00ddeadbeef & ~0UL);
> +
> +void __stack_chk_fail(void)
> +{
> +	void *ra;
> +
> +	ra = __builtin_extract_return_addr(__builtin_return_address(0));
> +	panic("Stack smashing detected in function:\n%p relocated from %p",
> +	      ra, ra - gd->reloc_off);
> +}
> 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..7aeec5e
> --- /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)
> +    u_boot_console.restart_uboot()
> +
>
Joel Peshkin April 9, 2021, 10:27 p.m. UTC | #3
Hi Heinrich,

Has there been any progress in getting the EFI erors fixed so that this can
be committed?  There seems to be little point in my refreshing this patch
until that is done.

Thanks,

-Joel


On Mon, Mar 22, 2021 at 10:37 AM Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:

> On 09.02.21 04:36, Joel Peshkin wrote:
> > Add support for stack protector for UBOOT, SPL, and TPL
> > as well as new pytest for stackprotector
> >
> > Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com>
> > ---
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >
> > Changes for v9:
> >    - Fix pytest script post-test reboot
> > Changes for v8:
> >    - Fix commit message
> >    - Force canary to UL type
> > Changes for v7:
> >    - Fix commit message
> >    - add __builtin_extract_return_addr() calls
> > 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                   | 19 +++++++++++++++++++
> >  configs/sandbox_defconfig            | 14 +++++++-------
> >  scripts/Makefile.spl                 |  6 ++++++
> >  test/py/tests/test_stackprotector.py | 15 +++++++++++++++
> >  11 files changed, 110 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 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 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;
>
> Hello Joël,
>
> This line is not needed.
>
> > +
> > +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-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 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..282c564
> > --- /dev/null
> > +++ b/common/stackprot.c
> > @@ -0,0 +1,19 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + *  Copyright 2021 Broadcom
> > + */
> > +
> > +#include <common.h>
>
> You need
>
>     #include <asm/global_data.h>
>
> here due to recently merged restructuring of includes by Simon.
>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
>
> common/stackprot.c:8:1: warning: data definition has no type or storage
> class
>     8 | DECLARE_GLOBAL_DATA_PTR;
>       | ^~~~~~~~~~~~~~~~~~~~~~~
> common/stackprot.c:8:1: warning: type defaults to ‘int’ in declaration
> of ‘DECLARE_GLOBAL_DATA_PTR’ [-Wimplicit-int]
> common/stackprot.c: In function ‘__stack_chk_fail’:
> common/stackprot.c:18:17: error: ‘gd’ undeclared (first use in this
> function)
>    18 |        ra, ra - gd->reloc_off);
>
>
> > +
> > +unsigned long __stack_chk_guard = (unsigned long)(0xfeedf00ddeadbeef &
> ~0UL);
> > +
> > +void __stack_chk_fail(void)
> > +{
> > +     void *ra;
> > +
> > +     ra = __builtin_extract_return_addr(__builtin_return_address(0));
> > +     panic("Stack smashing detected in function:\n%p relocated from %p",
> > +           ra, ra - gd->reloc_off);
> > +}
> > 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..7aeec5e
> > --- /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)
> > +    u_boot_console.restart_uboot()
> > +
> >
>
>
Heinrich Schuchardt April 10, 2021, 10:11 a.m. UTC | #4
On 4/10/21 12:27 AM, Joel Peshkin wrote:
>
> Hi Heinrich,
>
> Has there been any progress in getting the EFI erors fixed so that this
> can be committed?  There seems to be little point in my refreshing this
> patch until that is done.

I have fixed up your patch to work with EFI and will add it to my next
pull request.

I did not get qemu-x86_64_defconfig and qemu-x86_defconfig to build with
CONFIG_STACK_PROTECTOR=y, CONFIG_SPL_STACK_PROTECTOR = no.

arch/x86/cpu/irq.c and arch/x86/cpu/mtrr.c give me an undefined
reference to `__stack_chk_fail' when SPL is built. It seems that these
files are only built once instead of separately for SPL and main U-Boot.

Best regards

Heinrich

>
> Thanks,
>
> -Joel
>
>
> On Mon, Mar 22, 2021 at 10:37 AM Heinrich Schuchardt <xypron.glpk@gmx.de
> <mailto:xypron.glpk@gmx.de>> wrote:
>
>     On 09.02.21 04:36, Joel Peshkin wrote:
>      > Add support for stack protector for UBOOT, SPL, and TPL
>      > as well as new pytest for stackprotector
>      >
>      > Signed-off-by: Joel Peshkin <joel.peshkin@broadcom.com
>     <mailto:joel.peshkin@broadcom.com>>
>      > ---
>      > Cc: Simon Glass <sjg@chromium.org <mailto:sjg@chromium.org>>
>      > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de
>     <mailto:xypron.glpk@gmx.de>>
>      >
>      > Changes for v9:
>      >    - Fix pytest script post-test reboot
>      > Changes for v8:
>      >    - Fix commit message
>      >    - Force canary to UL type
>      > Changes for v7:
>      >    - Fix commit message
>      >    - add __builtin_extract_return_addr() calls
>      > 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                   | 19 +++++++++++++++++++
>      >  configs/sandbox_defconfig            | 14 +++++++-------
>      >  scripts/Makefile.spl                 |  6 ++++++
>      >  test/py/tests/test_stackprotector.py | 15 +++++++++++++++
>      >  11 files changed, 110 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
>     <mailto: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
>     <mailto:bharat.gooty@broadcom.com>>
>      >  M:   Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com
>     <mailto: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 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 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;
>
>     Hello Joël,
>
>     This line is not needed.
>
>      > +
>      > +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-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 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..282c564
>      > --- /dev/null
>      > +++ b/common/stackprot.c
>      > @@ -0,0 +1,19 @@
>      > +// SPDX-License-Identifier: GPL-2.0+
>      > +/*
>      > + *  Copyright 2021 Broadcom
>      > + */
>      > +
>      > +#include <common.h>
>
>     You need
>
>          #include <asm/global_data.h>
>
>     here due to recently merged restructuring of includes by Simon.
>
>      > +
>      > +DECLARE_GLOBAL_DATA_PTR;
>
>     common/stackprot.c:8:1: warning: data definition has no type or storage
>     class
>          8 | DECLARE_GLOBAL_DATA_PTR;
>            | ^~~~~~~~~~~~~~~~~~~~~~~
>     common/stackprot.c:8:1: warning: type defaults to ‘int’ in declaration
>     of ‘DECLARE_GLOBAL_DATA_PTR’ [-Wimplicit-int]
>     common/stackprot.c: In function ‘__stack_chk_fail’:
>     common/stackprot.c:18:17: error: ‘gd’ undeclared (first use in this
>     function)
>         18 |        ra, ra - gd->reloc_off);
>
>
>      > +
>      > +unsigned long __stack_chk_guard = (unsigned
>     long)(0xfeedf00ddeadbeef & ~0UL);
>      > +
>      > +void __stack_chk_fail(void)
>      > +{
>      > +     void *ra;
>      > +
>      > +     ra =
>     __builtin_extract_return_addr(__builtin_return_address(0));
>      > +     panic("Stack smashing detected in function:\n%p relocated
>     from %p",
>      > +           ra, ra - gd->reloc_off);
>      > +}
>      > 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..7aeec5e
>      > --- /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)
>      > +    u_boot_console.restart_uboot()
>      > +
>      >
>
Tom Rini April 10, 2021, 11:17 a.m. UTC | #5
On Sat, Apr 10, 2021 at 12:11:33PM +0200, Heinrich Schuchardt wrote:
> On 4/10/21 12:27 AM, Joel Peshkin wrote:
> > 
> > Hi Heinrich,
> > 
> > Has there been any progress in getting the EFI erors fixed so that this
> > can be committed?  There seems to be little point in my refreshing this
> > patch until that is done.
> 
> I have fixed up your patch to work with EFI and will add it to my next
> pull request.

If all of CI now works with -fstack-protector enabled, I'll take it up
in my tree.
diff mbox series

Patch

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 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 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-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 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..282c564
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,19 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include <common.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard = (unsigned long)(0xfeedf00ddeadbeef & ~0UL);
+
+void __stack_chk_fail(void)
+{
+	void *ra;
+
+	ra = __builtin_extract_return_addr(__builtin_return_address(0));
+	panic("Stack smashing detected in function:\n%p relocated from %p",
+	      ra, ra - gd->reloc_off);
+}
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..7aeec5e
--- /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)
+    u_boot_console.restart_uboot()
+