[PATCH] BRCMNAND: Fix reporting of uncorrectable errors on subpages during page read

2021-12-20 Thread Joel Peshkin 
Previously, a subpage with an uncorrectable error followed by a subpage
with a correctable error would return an erroneous correctable status.

Signed-off-by: Joel Peshkin 
Cc: Simon Glass 
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c 
b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index f8434ca..74c9348 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1632,7 +1632,7 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, 
struct nand_chip *chip,
mtd->oobsize / trans,
host->hwcfg.sector_size_1k);
 
-   if (!ret) {
+   if (ret != -EBADMSG) {
*err_addr = brcmnand_read_reg(ctrl,
BRCMNAND_UNCORR_ADDR) |
((u64)(brcmnand_read_reg(ctrl,
-- 
1.8.3.1

Re: [PATCH v9] Add support for stack-protector

2021-04-09 Thread Joel Peshkin 
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 
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 
> > ---
> > Cc: Simon Glass 
> > Cc: Heinrich Schuchardt 
> >
> > 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 
> > +S:   Maintained
> > +F:   common/stackprot.c
> > +F:   cmd/stackprot_test.c
> > +F:   test/py/tests/test_stackprotector.py
> > +
> >  TARGET_BCMNS3
> >  M:   Bharat Gooty 
> >  M:   Rayagonda Kokatanur 
> > 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_TERMINA

Re: Locking down U-Boot env with ENV_WRITEABLE_LIST

2021-04-06 Thread Joel Peshkin 
I've been using bootstopkeysha256 and, if I want to make it completely
impossible to enter, just setting it to an impossible value.   To do this,
I do have an additional patch to common/autoboot.c that calls the password
mechanism one last time after a bootcmd fails and loops until reset if it
isn't satisfied (never allowing it to fall through to the CLI)


On Tue, Apr 6, 2021 at 2:54 PM Sean Anderson  wrote:

>
>
> On 4/6/21 4:10 PM, Marek Vasut wrote:
>  > On 4/6/21 9:52 PM, Tim Harvey wrote:
>  >> On Mon, Apr 5, 2021 at 10:36 AM Marek Vasut  wrote:
>  >>>
>  >>> On 4/5/21 6:27 PM, Tim Harvey wrote:
>   On Sat, Apr 3, 2021 at 12:09 PM Marek Vasut  wrote:
>  >
>  > On 4/3/21 6:43 PM, Tim Harvey wrote:
>  >
>  > Hi,
>  >
>  > [...]
>  >
>  > And those config options I had enabled in u-boot defconfig:
>  >
>  > CONFIG_CMD_ENV_CALLBACK=y
>  > CONFIG_CMD_ENV_FLAGS=y
>  > CONFIG_ENV_IS_NOWHERE=y
>  > CONFIG_ENV_IS_IN_MMC=y
>  > CONFIG_ENV_APPEND=y
>  > CONFIG_ENV_WRITEABLE_LIST=y
>  > CONFIG_ENV_ACCESS_IGNORE_FORCE=y
>  
>   Do you really define both ENV_IS_NOWHERE and ENV_IS_IN_MMC? From
> what
>   I see if you define ENV_IS_NOWHERE none of the others will be
> used.
>  >>>
>  >>> Yes, having two ENV drivers is mandatory. One provides the base
> env (the
>  >>> nowhere) and the other is used to import the filtered extras from
>  >>> external env.
>  >>
>  >> Enabling ENV_IS_NOWHERE does not work as you describe in my
> testing.
>  >> I'm testing this with imx8mm_venice_defconfig on U-Boot 2021.01
> and as
>  >> soon as I define ENV_IS_NOWHERE then env_load is never called
> because
>  >> when env_relocate is called env is not valid yet so
> env_set_default is
>  >> called and env_load is 'never' called, thus mmc env is never
> loaded.
>  >> This is all from
>  >>
> https://elixir.bootlin.com/u-boot/v2021.01/source/env/common.c#L259
>  >
>  > Maybe this [PATCH V3] env: Fix invalid env handling in env_init()
> patch
>  > is missing ?
>  >
>  > [...]
>  >
>  >> /* Link Definitions */
>  >> #define CONFIG_LOADADDR0x4048
>  >> #define CONFIG_SYS_LOAD_ADDR   CONFIG_LOADADDR''
>  >>
>  >> With this configuration a blank env in flash results in the entire
>  >> default env showing in an 'env print' (ie all the stuff from
>  >> include/env_default.h 'default_environment') but as soon as I put
> an
>  >> env in flash (ie saveenv) and reset now the only env is what was
> set
>  >> via running code (ethaddr's fdtcontroladdr stdin/err/out). The only
>  >> different I expected is that I expected the default env from
>  >> include/env_default.h to be there on an initial boot with no valid
> mmc
>  >> env.
>  >
>  > Maybe the aforementioned patch is missing ?
>  
>   Marek,
>  
>   Yes, that patch fixes the issue I'm seeing of mmc not initializing
> and
>   now the default env shows up as expected when MMC env is empty or
>   populated.
>  
>   Thanks - hopefully that patch gets merged soon... I did respond with
> a
>   Tested-By.
>  >>>
>  >>> Oh, good. You might want to notify Tom about that, so it would get
> picked.
>  >>
>  >> Marek,
>  >>
>  >> One additional thing I did get stuck on is that if your writable
>  >> var(s) appears in the default environment the default will take
>  >> precedence over the FLASH env. This does make sense, but requires that
>  >> you create a default FLASH env (ie mkenvimage) and put it in the
>  >> appropriate place. I figured I would mention this for anyone else that
>  >> ends up reading this thread for help.
>  >
>  > Can you maybe write some better env documentation and submit a patch,
> now that you got it all figured out ?
>  >
>  >> One last thing that I have not yet figured out how to work-around is
>  >> how to best disable the shell completely so that if there is any
>  >> failure in your bootcmd you don't simply drop into a completely
>  >> insecure U-Boot shell. What is your recommendation there?
>  >
>  > set bootdelay=-2 disables the prompt access.
>
> This is insufficient. If the end of the boot command is reached, U-Boot
> will fall back to the shell. One must ensure that the bootcmd never
> exists. This can be done either by placing the whole command in a loop,
> recursively calling another command, or by resetting the board if no
> boot commands work. See e.g. [1] for a good writeup.
>
> --Sean
>
> [1]
> https://labs.f-secure.com/assets/BlogFiles/2020-05-u-booting-securely-wp-final.pdf
>


smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v2 1/1] efi_loader: fix get_last_capsule()

2021-03-03 Thread Joel Peshkin 
Hi Takahiro Akashi,

   The issue here is causing a failure in the EFI tests whenever the
compiler is checking to make sure the code is not overrunning the stack.
Fixing it is absolutely necessary.  To see this problem, please apply
https://patchwork.ozlabs.org/project/uboot/patch/20210209033607.70955-1-joel.pesh...@broadcom.com/
and then run the pytests on the sandbox build.  You will see that this
failure occurs during the EFI tests and that is the only portion of uboot
expressing such a  bug during the test suites.

Regards,

Joel



On Thu, Feb 11, 2021 at 3:05 AM Heinrich Schuchardt 
wrote:

> fix get_last_capsule() leads to writes beyond the stack allocated buffer.
> This was indicated when enabling the stack protector.
>
> utf16_utf8_strcpy() only stops copying when reaching '\0'. The current
> invocation always writes beyond the end of value[].
>
> The output length of utf16_utf8_strcpy() may be longer than the number of
> UTF-16 tokens. E.g has "CapsuleКиев" has 11 UTF-16 tokens but 15 UTF-8
> tokens. Hence, using utf16_utf8_strcpy() without checking the input may
> lead to further writes beyond value[].
>
> The current invocation of strict_strtoul() reads beyond the end of value[].
>
> A non-hexadecimal value after "Capsule" (e.g. "Capsule") must result in
> an error. We cat catch this by checking the return value of
> strict_strtoul().
>
> A value that is too short after "Capsule" (e.g. "Capsule0") must result in
> an error. We must check the string length of value[].
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> v2:
> check for non-ANSI character
> ---
>  lib/efi_loader/efi_capsule.c | 18 +-
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index d39d731080..0017f0c0db 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -42,20 +42,28 @@ static struct efi_file_handle *bootdev_root;
>  static __maybe_unused unsigned int get_last_capsule(void)
>  {
> u16 value16[11]; /* "Capsule": non-null-terminated */
> -   char value[11], *p;
> +   char value[5];
> efi_uintn_t size;
> unsigned long index = 0x;
> efi_status_t ret;
> +   int i;
>
> size = sizeof(value16);
> ret = efi_get_variable_int(L"CapsuleLast",
> &efi_guid_capsule_report,
>NULL, &size, value16, NULL);
> -   if (ret != EFI_SUCCESS || u16_strncmp(value16, L"Capsule", 7))
> +   if (ret != EFI_SUCCESS || size != 22 ||
> +   u16_strncmp(value16, L"Capsule", 7))
> goto err;
> +   for (i = 0; i < 4; ++i) {
> +   u16 c = value16[i + 7];
>
> -   p = value;
> -   utf16_utf8_strcpy(&p, value16);
> -   strict_strtoul(&value[7], 16, &index);
> +   if (!c || c > 0x7f)
> +   goto err;
> +   value[i] = c;
> +   }
> +   value[4] = 0;
> +   if (strict_strtoul(value, 16, &index))
> +   index = 0x;
>  err:
> return index;
>  }
> --
> 2.30.0
>
>


smime.p7s
Description: S/MIME Cryptographic Signature

[PATCH v9] Add support for stack-protector

2021-02-08 Thread Joel Peshkin 
Add support for stack protector for UBOOT, SPL, and TPL
as well as new pytest for stackprotector

Signed-off-by: Joel Peshkin 
---
Cc: Simon Glass 
Cc: Heinrich Schuchardt 

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 
+S: Maintained
+F: common/stackprot.c
+F: cmd/stackprot_test.c
+F: test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M: Bharat Gooty 
 M: Rayagonda Kokatanur 
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 000..6ad287e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include 
+#include 
+
+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

[PATCH v8] Add support for stack-protector

2021-01-14 Thread Joel Peshkin 
Add support for stack protector for UBOOT, SPL, and TPL
as well as new pytest for stackprotector

Signed-off-by: Joel Peshkin 
---
Cc: Simon Glass 
Cc: Heinrich Schuchardt 

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 
+S: Maintained
+F: common/stackprot.c
+F: cmd/stackprot_test.c
+F: test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M: Bharat Gooty 
 M: Rayagonda Kokatanur 
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 000..6ad287e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include 
+#include 
+
+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

[PATCH v7] Add support for stack-protector

2021-01-14 Thread Joel Peshkin 
Add support for stack protector for UBOOT, SPL, and TPL
as well as new pytest for stackprotector

Signed-off-by: Joel Peshkin 
Cc: Simon Glass 
Cc: Heinrich Schuchardt 

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 
+S: Maintained
+F: common/stackprot.c
+F: cmd/stackprot_test.c
+F: test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M: Bharat Gooty 
 M: Rayagonda Kokatanur 
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 000..6ad287e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include 
+#include 
+
+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
+   de

Re: [PATCH] Add fixdefconfig script to update lists of defconfig files from savedefconfig

2021-01-13 Thread Joel Peshkin 
No worries.  As long as there is a mechanism that will work OK.

On Wed, Jan 13, 2021 at 12:16 PM Tom Rini  wrote:

> On Wed, Jan 13, 2021 at 09:10:41AM -0700, Simon Glass wrote:
>
> > Hi Joel,
> >
> > On Mon, 11 Jan 2021 at 20:01, Joel Peshkin 
> wrote:
> > >
> > > Cc: Simon Glass 
> > > Cc: Heinrich Schuchardt 
> > > ---
> > >  scripts/fixdefconfig | 25 +
> > >  1 file changed, 25 insertions(+)
> > >  create mode 100755 scripts/fixdefconfig
> >
> > +Tom Rini
> >
> > I normally use moveconfig for this...Tom how do you do it?
>
> Yes, this is a single-threaded version of "moveconfig.py -sC".  This is
> useful I suppose in the case where you have to sync maybe a dozen files
> rather than one or two, or all of them.  But I'm not sure it's worth
> applying, sorry.
>
> --
> Tom
>


smime.p7s
Description: S/MIME Cryptographic Signature

[PATCH v6] Add support for stack-protector

2021-01-12 Thread Joel Peshkin 
Cc: Simon Glass 
Cc: Heinrich Schuchardt 
Signed-off-by: Joel Peshkin 

Add support for stack protector for UBOOT, SPL, and TPL
as well as new pytest for stackprotector

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 
+S: Maintained
+F: common/stackprot.c
+F: cmd/stackprot_test.c
+F: test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M: Bharat Gooty 
 M: Rayagonda Kokatanur 
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 000..6ad287e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include 
+#include 
+
+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

[PATCH] Add fixdefconfig script to update lists of defconfig files from savedefconfig

2021-01-11 Thread Joel Peshkin 
Cc: Simon Glass 
Cc: Heinrich Schuchardt 
---
 scripts/fixdefconfig | 25 +
 1 file changed, 25 insertions(+)
 create mode 100755 scripts/fixdefconfig

diff --git a/scripts/fixdefconfig b/scripts/fixdefconfig
new file mode 100755
index 000..7f36762
--- /dev/null
+++ b/scripts/fixdefconfig
@@ -0,0 +1,25 @@
+#!/bin/bash
+
+if [ -z "$*" -o  "${1%_defconfig}" = "$1" ]
+then
+  echo "Usage:  $0 [defconfig_file...]"
+  echo " Normalizes each listed defconfig and replaces it with the normalized"
+  echo "version. The original is renamed with an extension of .old appended"
+  exit 1
+fi
+
+tmp=tmp_build_$$
+mkdir $tmp
+for config in $*
+do
+  base=`basename $config`
+  make O=$tmp/$base $base \
+&& make O=$tmp/$base $base \
+&& make O=$tmp/$base savedefconfig \
+&& diff -q $tmp/$base/defconfig configs/$base \
+|| mv configs/$base configs/$base.old \
+&& mv $tmp/$base/defconfig configs/$base
+  rm -rf $tmp/$base
+done
+rmdir $tmp
+
-- 
1.8.3.1

[PATCH v5] Add support for stack-protector

2021-01-11 Thread Joel Peshkin 
Cc: Joel Peshkin ,
Simon Glass ,
Bin Meng ,
Jagan Teki ,
Kever Yang ,
Heinrich Schuchardt ,
AKASHI Takahiro ,
Usama Arif ,
Sam Protsenko ,
Masahiro Yamada ,
Philippe Reynes ,
Eugeniu Rosca ,
Jan Kiszka 
Signed-off-by: Joel Peshkin 

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 
+S: Maintained
+F: common/stackprot.c
+F: cmd/stackprot_test.c
+F: test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M: Bharat Gooty 
 M: Rayagonda Kokatanur 
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 000..6ad287e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include 
+#include 
+
+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 STACKPR

[PATCH v4] Add support for stack-protector

2021-01-11 Thread Joel Peshkin 
Cc: Simon Glass 
Cc: Bin Meng 
Cc: Jagan Teki 
Cc: Kever Yang 
Cc: Heinrich Schuchardt 
Cc: AKASHI Takahiro 
Cc: Usama Arif 
Cc: Sam Protsenko 
Cc: Masahiro Yamada 
Cc: Philippe Reynes 
Cc: Eugeniu Rosca 
Cc: Jan Kiszka 

Signed-off-by: Joel Peshkin 

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|  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 
+S: Maintained
+F: common/stackprot.c
+F: cmd/stackprot_test.c
+F: test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M: Bharat Gooty 
 M: Rayagonda Kokatanur 
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 00..6ad287e55f
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include 
+#include 
+
+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/

[PATCH v3] Add support for stack-protector

2021-01-11 Thread Joel Peshkin 
Cc: Simon Glass 
Cc: Bin Meng 
Cc: Jagan Teki 
Cc: Kever Yang 
Cc: Heinrich Schuchardt 
Cc: AKASHI Takahiro 
Cc: Usama Arif 
Cc: Sam Protsenko 
Cc: Masahiro Yamada 
Cc: Philippe Reynes 
Cc: Eugeniu Rosca 
Cc: Jan Kiszka 

Signed-off-by: Joel Peshkin 

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 |  4 
 cmd/Kconfig  | 10 ++
 cmd/Makefile |  1 +
 cmd/stackprot_test.c | 26 ++
 common/Kconfig   | 17 +
 common/Makefile  |  2 ++
 common/stackprot.c   | 18 ++
 configs/sandbox_defconfig|  2 ++
 scripts/Makefile.spl |  6 ++
 test/py/tests/test_stackprotector.py | 15 +++
 11 files changed, 108 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 
+S: Maintained
+F: common/stackprot.c
+F: cmd/stackprot_test.c
+F: test/py/tests/test_stackprotector.py
+
 TARGET_BCMNS3
 M: Bharat Gooty 
 M: Rayagonda Kokatanur 
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/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 00..ea952c075e
--- /dev/null
+++ b/cmd/stackprot_test.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[]);
+
+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..5a053c7336 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 gcc built-in 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/Make

Re: [PATCH] Add support for stack-protector

2021-01-10 Thread Joel Peshkin 
Hi Alex,

   Yeah, I think I'll wind up with some ifdef code for the static init.
 In the case of arm (32-bit), there is actually a GCC bug that causes it to
use the address of the canary value instead of the canary value itself.
GCC upstream just fixed that a few days ago, but it may be a year or so
before the appropriate GCC version is widely available.

I may eventually add an optional mechanism to allow the value to be
changed (very carefully) at runtime.  This has to be done early enough that
we cannot wait for an RNG to be identified via DTB, but there are a few
ways to keep arm and aarch64 from being too predictable and some boards may
have peripherals that can provide a sufficiently variable value.

-Joel




On Sun, Jan 10, 2021 at 2:40 PM Alex Sadovsky <
nable.mainin...@googlemail.com> wrote:

> Hi,
> > +
> > +unsigned long __stack_chk_guard = 0xfeedf00ddeadbeef;
>
> sizeof(unsigned long) isn't always 8, even gcc issues a warning when it's
> invoked with proper options (e.g. 32-bit build):
>
> > warning: conversion from ‘long long unsigned int’ to ‘long unsigned int’
> changes value from ‘18369602397475290863’ to ‘3735928559’ [-Woverflow]
>
> Maybe there's some better way to initialize this variable. E.g. with #if …
> #else … #endif or using some initialization function that is invoked early.
> I should also mention that a fixed canary value doesn't actually bring
> proper protection against exploits, thus run-time initialization with a
> random value is usually preferred.
>
> I'm not sure whether it's important at all in bootloader code, I just
> wanted to be sure that it isn't unnoticed.
>
> Cheers, Alex.
>
>


smime.p7s
Description: S/MIME Cryptographic Signature

[PATCH v2] Add support for stack-protector

2021-01-10 Thread Joel Peshkin 
Cc: Simon Glass 
Cc: Bin Meng 
Cc: Jagan Teki 
Cc: Kever Yang 
Cc: Heinrich Schuchardt 
Cc: AKASHI Takahiro 
Cc: Usama Arif 
Cc: Sam Protsenko 
Cc: Masahiro Yamada 
Cc: Philippe Reynes 
Cc: Eugeniu Rosca 
Cc: Jan Kiszka 

Signed-off-by: Joel Peshkin 

Changes for v2:
   - Add test command and corresponding config
   - Fixed incorrect description in Kconfig
   - Add unit test
---
 Makefile |  4 +++
 common/Kconfig   | 22 ++
 common/Makefile  |  2 ++
 common/stackprot.c   | 44 
 configs/sandbox_defconfig|  2 ++
 scripts/Makefile.spl |  6 
 test/py/tests/test_stackprotector.py | 15 ++
 7 files changed, 95 insertions(+)
 create mode 100644 common/stackprot.c
 create mode 100644 test/py/tests/test_stackprotector.py

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..f773babd3a 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -595,6 +595,28 @@ 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 STACKPROTECTOR_TEST_COMMAND
+   bool "Test command for stack protector"
+   depends on STACKPROTECTOR
+   default n
+   help
+ Enable stackprot_test command
+
+config SPL_STACKPROTECTOR
+   bool "Stack Protector buffer overflow detection for SPL"
+   default n
+
+config TPL_STACKPROTECTOR
+   bool "Stack Protector buffer overflow detection for 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 00..d602dc7de1
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard =
+(unsigned long)((unsigned long long)0xfeedf00ddeadbeef &
+   ~((unsigned long)0));
+
+void __stack_chk_fail(void)
+{
+   panic("Stack smashing detected in function: %p relocated from %p",
+ __builtin_return_address(0),
+ __builtin_return_address(0) - gd->reloc_off);
+}
+
+#ifdef CONFIG_STACKPROTECTOR_TEST_COMMAND
+static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[]);
+
+int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+   char a[128];
+   char b[256];
+   int i;
+   for (i = 0; i < 256; i++) {
+   b[i] = i;
+   }
+   memcpy(a, b, 512);
+   return (0);
+}
+
+U_BOOT_CMD(stackprot_test, 1, 1, do_test_stackprot_fail,
+  "test stack protector fail", "");
+
+#endif
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index f1ec701a9f..da3aca2551 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_STACKPROTECTOR_TEST_COMMAND=y
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
ind

Re: [PATCH] Add support for stack-protector

2021-01-10 Thread Joel Peshkin 
Hi Heinrich,

  Thank you for your comments.  I have 2 questions about how to proceed

1) Unit test
  I can add a function that can be used to trigger an overrun, but I am not
sure where to include it as the stack protector prints the error message
and then resets uboot so it wouldn't fir in a unit test suite.

  I could add a CONFIG_STACKPROTECTOR_TEST_FAIL to add a
"test_stackprotector fail" command to the CLI and you could call the
underlying stackprot_test_fail() from code hacked into SPL and TPL

2) Standalone/EFI
What we did for our own standalone code was to add the KBUILD_CFLAGS +=
-fno-stack-protector   to the Makefile for our specific standalone.   The
problem is there is no generic place from which all standalone/EFI is
called, so I could just leave this for maintainers of specific
standalone/EPI programs to add IF they are enabling STACKPROTECTOR (If they
don't enable it, they don't need to do anything) or I could add
KBUILD_CFLAGS += -fno-stack-protector  to  both lib/efi_setlftest/Makefile
and lib/efi_loader/Makefile

What would you suggest?

Regards,

Joel


smime.p7s
Description: S/MIME Cryptographic Signature

[PATCH] Add support for stack-protector

2021-01-10 Thread Joel Peshkin 
Cc: Simon Glass 
Cc: Bin Meng 
Cc: Jagan Teki 
Cc: Kever Yang 
Cc: Heinrich Schuchardt 
Cc: AKASHI Takahiro 
Cc: Usama Arif 
Cc: Sam Protsenko 
Cc: Masahiro Yamada 
Cc: Philippe Reynes 
Cc: Eugeniu Rosca 
Cc: Jan Kiszka 

Signed-off-by: Joel Peshkin 

---

 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"
+   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 00..7c95b8544f
--- /dev/null
+++ b/common/stackprot.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Copyright 2021 Broadcom
+ */
+
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+unsigned long __stack_chk_guard = 0xfeedf00ddeadbeef;
+
+void __stack_chk_fail(void)
+{
+   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)
-- 
2.27.0

[PATCH v3] Add optional salt to AUTOBOOT_STOP_STR_SHA256

2020-11-21 Thread Joel Peshkin 
Adds an optional SALT value to AUTOBOOT_STOP_STR_SHA256.   If a string
followed by a ":" is prepended to the sha256, the portion to the left
of the colon will be used as a salt and the password will be appended
to the salt before the sha256 is computed and compared.

Signed-off-by: Joel Peshkin 
Cc: Simon Glass 
Cc: Bin Meng 
Cc: Patrick Delaunay 
Cc: Heiko Schocher 
Cc: Heinrich Schuchardt 
Cc: Joel Peshkin 
To: u-boot@lists.denx.de

---
Changes for v2:
   - Increase MAX_DELAY_STOP_STR
   - Check salt size against MAX_DELAY_STOP_STR before copying
   - Minor cleanup
Changes for v3:
   - Cleanup changing (c) to c after review feedback
---
 common/Kconfig.boot |  5 -
 common/autoboot.c   | 12 ++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index 3f6d9c1..8a98672 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -819,7 +819,10 @@ config AUTOBOOT_STOP_STR_SHA256
  This option adds the feature to only stop the autobooting,
  and therefore boot into the U-Boot prompt, when the input
  string / password matches a values that is encypted via
- a SHA256 hash and saved in the environment.
+ a SHA256 hash and saved in the environment variable
+ "bootstopkeysha256". If the value in that variable
+ includes a ":", the portion prior to the ":" will be treated
+ as a salt value.
 
 config AUTOBOOT_USE_MENUKEY
bool "Allow a specify key to run a menu from the environment"
diff --git a/common/autoboot.c b/common/autoboot.c
index e628baf..ddb6246 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -25,7 +25,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define MAX_DELAY_STOP_STR 32
+#define MAX_DELAY_STOP_STR 64
 
 #ifndef DEBUG_BOOTKEYS
 #define DEBUG_BOOTKEYS 0
@@ -80,6 +80,7 @@ static int passwd_abort_sha256(uint64_t etime)
u8 sha_env[SHA256_SUM_LEN];
u8 *sha;
char *presskey;
+   char *c;
const char *algo_name = "sha256";
u_int presskey_len = 0;
int abort = 0;
@@ -89,6 +90,14 @@ static int passwd_abort_sha256(uint64_t etime)
if (sha_env_str == NULL)
sha_env_str = AUTOBOOT_STOP_STR_SHA256;
 
+   presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
+   c = strstr(sha_env_str, ":");
+   if (c && (c - sha_env_str < MAX_DELAY_STOP_STR)) {
+   /* preload presskey with salt */
+   memcpy(presskey, sha_env_str, c - sha_env_str);
+   presskey_len = c - sha_env_str;
+   sha_env_str = c + 1;
+   }
/*
 * Generate the binary value from the environment hash value
 * so that we can compare this value with the computed hash
@@ -100,7 +109,6 @@ static int passwd_abort_sha256(uint64_t etime)
return 0;
}
 
-   presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
sha = malloc_cache_aligned(SHA256_SUM_LEN);
size = SHA256_SUM_LEN;
/*
-- 
1.8.3.1



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v2] Add optional salt to AUTOBOOT_STOP_STR_SHA256

2020-11-20 Thread Joel Peshkin 
Hi Heinrich,

   Thank you for the review.   I increased the max size to 64 characters.
 The size, in the end, is the size of the salt plus the size of the
password the human user would type.   In most places I have seen salt used,
it is only a few characters (modern Linux password databases use 8) and the
actual password (as opposed to its sha256) is unlikely to be more than 15
characters.

Regards,

Joel


On Fri, Nov 20, 2020 at 10:05 AM Joel Peshkin 
wrote:

> From: Joel Peshkin 
>
> Adds an optional SALT value to AUTOBOOT_STOP_STR_SHA256.   If a string
> followed by a ":" is prepended to the sha256, the portion to the left
> of the colon will be used as a salt and the password will be appended
> to the salt before the sha256 is computed and compared.
>
> Signed-off-by: Joel Peshkin 
> Cc: Simon Glass 
> Cc: Bin Meng 
> Cc: Patrick Delaunay 
> Cc: Heiko Schocher 
> Cc: tr...@konsulko.com
> Cc: Heinrich Schuchardt 
> Cc: Joel Peshkin 
> To: u-boot@lists.denx.de
>
> ---
> Changes for v2:
>- Increase MAX_DELAY_STOP_STR
>- Check salt size against MAX_DELAY_STOP_STR before copying
>- Minor cleanup
> ---
>  common/Kconfig.boot |  5 -
>  common/autoboot.c   | 12 ++--
>  2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/common/Kconfig.boot b/common/Kconfig.boot
> index 3f6d9c1..8a98672 100644
> --- a/common/Kconfig.boot
> +++ b/common/Kconfig.boot
> @@ -819,7 +819,10 @@ config AUTOBOOT_STOP_STR_SHA256
>   This option adds the feature to only stop the autobooting,
>   and therefore boot into the U-Boot prompt, when the input
>   string / password matches a values that is encypted via
> - a SHA256 hash and saved in the environment.
> + a SHA256 hash and saved in the environment variable
> + "bootstopkeysha256". If the value in that variable
> + includes a ":", the portion prior to the ":" will be treated
> + as a salt value.
>
>  config AUTOBOOT_USE_MENUKEY
> bool "Allow a specify key to run a menu from the environment"
> diff --git a/common/autoboot.c b/common/autoboot.c
> index e628baf..982b561 100644
> --- a/common/autoboot.c
> +++ b/common/autoboot.c
> @@ -25,7 +25,7 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> -#define MAX_DELAY_STOP_STR 32
> +#define MAX_DELAY_STOP_STR 64
>
>  #ifndef DEBUG_BOOTKEYS
>  #define DEBUG_BOOTKEYS 0
> @@ -80,6 +80,7 @@ static int passwd_abort_sha256(uint64_t etime)
> u8 sha_env[SHA256_SUM_LEN];
> u8 *sha;
> char *presskey;
> +   char *c;
> const char *algo_name = "sha256";
> u_int presskey_len = 0;
> int abort = 0;
> @@ -89,6 +90,14 @@ static int passwd_abort_sha256(uint64_t etime)
> if (sha_env_str == NULL)
> sha_env_str = AUTOBOOT_STOP_STR_SHA256;
>
> +   presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
> +   c = strstr(sha_env_str, ":");
> +   if ((c) && (c - sha_env_str < MAX_DELAY_STOP_STR)) {
> +   /* preload presskey with salt */
> +   memcpy(presskey, sha_env_str, c - sha_env_str);
> +   presskey_len = c - sha_env_str;
> +   sha_env_str = c + 1;
> +   }
> /*
>  * Generate the binary value from the environment hash value
>  * so that we can compare this value with the computed hash
> @@ -100,7 +109,6 @@ static int passwd_abort_sha256(uint64_t etime)
> return 0;
> }
>
> -   presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
> sha = malloc_cache_aligned(SHA256_SUM_LEN);
> size = SHA256_SUM_LEN;
> /*
> --
> 1.8.3.1
>
>


smime.p7s
Description: S/MIME Cryptographic Signature

[RFC] regarding Uboot AUTOBOOT_STOP_STR_SHA256

2020-11-20 Thread Joel Peshkin 
Hi Stefan,

I have a patch (
https://patchwork.ozlabs.org/project/uboot/patch/20201120180524.30251-1-jp933...@xl-irv-13.lvn.broadcom.net/
) under review that adds optional SALT to AUTOBOOT_STOP_STR_SHA256 without
breaking backward compatibility.  As I see that you were involved with the
original requirements, I want to bring that patch to your attention and ask
you about several others that I plan to follow with.   I presume I should
explicitly add you to the CC on those.

   Do you have any concerns about the following changes??

1) Prior to entering the autoboot.c, it is possible that there would
already be characters in the UART receive buffer that are not part of the
password.  For my application, it is preferable to swallow any existing
characters before starting to accept characters into the password.   Is
this something that would be a problem for anyone who might rely on
beginning to type the password before the device gets to the prompt?  Would
I need to make this a config option?

2) If the password is not given, autoboot will not permit CLI access before
bootcmd.  However, if bootcmd returns (most likely fails), the current
implementation permits access without a password.  My requirements would
require the password even if the boot fails.  Is this something that should
be configurable or should I make the change to autoboot so that, if the
boot fails, the password is required again and the device will require
reset if the password is not given then?

3) The passwords themselves may be relatively long and we do not want to
have a long timeout increasing boot time.  My current implementation treats
the timeout as the time to BEGIN typing the password and adds one second
for every character typed.  So, I can have a 5 second timeout and, if the
password is 10 characters long, then the user doesn't time out as long as
they start typing within the 5 seconds and average one character per
second.   Would this be something that needs to be configurable in Kconfig
or would it be OK to specify the added 1 second time in a #define ??

Thank You,

Joel Peshkin


smime.p7s
Description: S/MIME Cryptographic Signature

[PATCH v2] Add optional salt to AUTOBOOT_STOP_STR_SHA256

2020-11-20 Thread Joel Peshkin 
From: Joel Peshkin 

Adds an optional SALT value to AUTOBOOT_STOP_STR_SHA256.   If a string
followed by a ":" is prepended to the sha256, the portion to the left
of the colon will be used as a salt and the password will be appended
to the salt before the sha256 is computed and compared.

Signed-off-by: Joel Peshkin 
Cc: Simon Glass 
Cc: Bin Meng 
Cc: Patrick Delaunay 
Cc: Heiko Schocher 
Cc: tr...@konsulko.com
Cc: Heinrich Schuchardt 
Cc: Joel Peshkin 
To: u-boot@lists.denx.de

---
Changes for v2:
   - Increase MAX_DELAY_STOP_STR
   - Check salt size against MAX_DELAY_STOP_STR before copying
   - Minor cleanup
---
 common/Kconfig.boot |  5 -
 common/autoboot.c   | 12 ++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index 3f6d9c1..8a98672 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -819,7 +819,10 @@ config AUTOBOOT_STOP_STR_SHA256
  This option adds the feature to only stop the autobooting,
  and therefore boot into the U-Boot prompt, when the input
  string / password matches a values that is encypted via
- a SHA256 hash and saved in the environment.
+ a SHA256 hash and saved in the environment variable
+ "bootstopkeysha256". If the value in that variable
+ includes a ":", the portion prior to the ":" will be treated
+ as a salt value.
 
 config AUTOBOOT_USE_MENUKEY
bool "Allow a specify key to run a menu from the environment"
diff --git a/common/autoboot.c b/common/autoboot.c
index e628baf..982b561 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -25,7 +25,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define MAX_DELAY_STOP_STR 32
+#define MAX_DELAY_STOP_STR 64
 
 #ifndef DEBUG_BOOTKEYS
 #define DEBUG_BOOTKEYS 0
@@ -80,6 +80,7 @@ static int passwd_abort_sha256(uint64_t etime)
u8 sha_env[SHA256_SUM_LEN];
u8 *sha;
char *presskey;
+   char *c;
const char *algo_name = "sha256";
u_int presskey_len = 0;
int abort = 0;
@@ -89,6 +90,14 @@ static int passwd_abort_sha256(uint64_t etime)
if (sha_env_str == NULL)
sha_env_str = AUTOBOOT_STOP_STR_SHA256;
 
+   presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
+   c = strstr(sha_env_str, ":");
+   if ((c) && (c - sha_env_str < MAX_DELAY_STOP_STR)) {
+   /* preload presskey with salt */
+   memcpy(presskey, sha_env_str, c - sha_env_str);
+   presskey_len = c - sha_env_str;
+   sha_env_str = c + 1;
+   }
/*
 * Generate the binary value from the environment hash value
 * so that we can compare this value with the computed hash
@@ -100,7 +109,6 @@ static int passwd_abort_sha256(uint64_t etime)
return 0;
}
 
-   presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
sha = malloc_cache_aligned(SHA256_SUM_LEN);
size = SHA256_SUM_LEN;
/*
-- 
1.8.3.1



smime.p7s
Description: S/MIME Cryptographic Signature

[PATCH] Add optional salt to AUTOBOOT_STOP_STR_SHA256

2020-11-19 Thread Joel Peshkin 
From: Joel Peshkin 

Adds an optional SALT value to AUTOBOOT_STOP_STR_SHA256.   If a string 
followed by a ":" is prepended to the sha256, the portion to the left 
of the colon will be used as a salt and the password will be appended
to the salt before the sha256 is computed and compared.

Signed-off-by: Joel Peshkin 
---
 common/Kconfig.boot |  5 -
 common/autoboot.c   | 10 +-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index 3f6d9c1..8a98672 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -819,7 +819,10 @@ config AUTOBOOT_STOP_STR_SHA256
  This option adds the feature to only stop the autobooting,
  and therefore boot into the U-Boot prompt, when the input
  string / password matches a values that is encypted via
- a SHA256 hash and saved in the environment.
+ a SHA256 hash and saved in the environment variable
+ "bootstopkeysha256". If the value in that variable
+ includes a ":", the portion prior to the ":" will be treated
+ as a salt value.
 
 config AUTOBOOT_USE_MENUKEY
bool "Allow a specify key to run a menu from the environment"
diff --git a/common/autoboot.c b/common/autoboot.c
index e628baf..0c4e6ff 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -80,6 +80,7 @@ static int passwd_abort_sha256(uint64_t etime)
u8 sha_env[SHA256_SUM_LEN];
u8 *sha;
char *presskey;
+   char *c;
const char *algo_name = "sha256";
u_int presskey_len = 0;
int abort = 0;
@@ -89,6 +90,14 @@ static int passwd_abort_sha256(uint64_t etime)
if (sha_env_str == NULL)
sha_env_str = AUTOBOOT_STOP_STR_SHA256;
 
+   presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
+   c = strstr(sha_env_str, ":");
+   if (c) {
+   /* preload presskey with salt */
+   memcpy(presskey, sha_env_str, c - sha_env_str);
+   presskey_len += c - sha_env_str;
+   sha_env_str = c + 1;
+   }
/*
 * Generate the binary value from the environment hash value
 * so that we can compare this value with the computed hash
@@ -100,7 +109,6 @@ static int passwd_abort_sha256(uint64_t etime)
return 0;
}
 
-   presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
sha = malloc_cache_aligned(SHA256_SUM_LEN);
size = SHA256_SUM_LEN;
/*
-- 
1.8.3.1



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [U-Boot] Buffer overrun risk in UBI SPL for secure boot

2019-09-09 Thread Joel Peshkin 
Hi Heiko,

Adding a size limit without breaking things turns out to be much more
difficult that it would seem.  So, instead of capping the size, we have
changed the memory map we are using for uboot.  It is probably worthwhile
for others using UBISPL in a secure boot nevironment to do the same.

   Traditionally, uboot SPL or TPL loads or relocates to an address near
the top of memory and then builds its stack downwards from the top of
memory.   That means that any address we use for a volume.load_address will
eventually step on something if the volume is large enough.   So, we move
everything down by a size that is sufficient for any image that UBISPL may
need to load (32M) and place the CONFIG_SPL_LOAD_FIT_ADDRESS  Above the
stack where it can grow without hitting anything until it causes an
exception.

   I'm not sure if there is anything else to be done for this situation
except to caution people implementing secure boot environments to be aware
of their surroundings.

Regards,

Joel Peshkin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] Buffer overrun risk in UBI SPL for secure boot

2019-09-04 Thread Joel Peshkin 
Hi Heiko,

   The place where the issue comes up is in ubispl_load_volumes(), but that
calls ipl_load() internally.

   I guess there are several options

1) Create a distinct ubispl_scan() function to do the scan without loading
anything and then a new load volume function that takes offset and limit
arguments.   This would have the advantage that a SPL that  needs to parse
one volume before loading the next would only need to scan once, then could
check sizes, then load the individual volumes as needed.  At the same time,
when we need to validate a FIT header before even deciding what we want to
load from it, we would be able to read more incrementally.

2) Add the size limit to struct ubispl_load and not worry about anyone with
non-upstreamed code forgetting to set it

3) Add the size limit to struct ubispl_load but have ubispl_load_volumes()
set the size limit to 0 (unlimited) before calling a new
ubispl_load_volumes_bounded() function.

   Which do you prefer?

   Do I presume correctly that your denx.de email address implies that an
approach you approve will be acceptable if correctly implemented?

Thanks,

Joel Peshkin


On Wed, Sep 4, 2019 at 6:01 AM Heiko Schocher  wrote:

> Hello Joel,
>
> Am 04.09.2019 um 06:57 schrieb Joel Peshkin:
> > It seems that, in the process of doing any sort of secure boot chain of
> > trust, anything loading a UBI volume in preparation to authenticate it,
> > will load a volume of unknown size into a buffer prior to checking the
> > signature of that volume.
>
> Hmm.. it is not an unknown size, it loads the complete UBI Volume, so
> it is the size of the UBI Volume ...
> but yes, if an attacker changes the size of UBI Volumes ...
>
> Could you please give us a reference to which piece of code you refer?
>
> is it this part:
>
> drivers/mtd/ubispl/ubispl.c
> static int ipl_load(struct ubi_scan_info *ubi, const u32 vol_id, uint8_t
> *laddr)
>
> yes ... no size...
>
> > Has anyone considered a solution for this?  Should all implementations
> just
> > carve out a buffer at the top of memory for ubispl_load_volume or should
> > the ubispl_load data structure be amended to include a size?  It would
> seem
> > appropriate to include a size, but not clear how to do that without
> > breaking compatibility with existing implementations.
>
> Hmm... yes may an option to add a maxsize to ubispl_load data struct
> ubispl_load.
>
> regarding backwards compatibility, there are not much boards yet, which
> use this feature:
>
> hs@threadripper:u-boot  [master] $ grep -lr SPL_UBI | grep defconfig
> configs/am335x_igep003x_defconfig
> configs/igep00x0_defconfig
> hs@threadripper:u-boot  [master] $
>
> So this should be solveable problem.
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] Buffer overrun risk in UBI SPL for secure boot

2019-09-03 Thread Joel Peshkin 
It seems that, in the process of doing any sort of secure boot chain of
trust, anything loading a UBI volume in preparation to authenticate it,
will load a volume of unknown size into a buffer prior to checking the
signature of that volume.

Has anyone considered a solution for this?  Should all implementations just
carve out a buffer at the top of memory for ubispl_load_volume or should
the ubispl_load data structure be amended to include a size?  It would seem
appropriate to include a size, but not clear how to do that without
breaking compatibility with existing implementations.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] Announcement -- Upstreaming of U-Boot support for Broadcom router devices

2019-08-01 Thread Joel Peshkin 
Hi All:

   Up to now, there has been limited 3rd party support for Broadcom's 63xx
and 68xx devices in U-Boot.   At this point, we plan to introduce
vendor-supported U-Boot to all of the ARM-based  BCM63XX and BCM68XX
devices as well as some additional family members.  These devices can be
broadly considered as the devices used in DSL and PON routers as well as in
retail and enterprise wireless access points.

   We are doing this in coordination with some of our partners who have
made previous contributions.   To avoid namespace collisions,  we plan to
place device-specific files under mach-bcmbca/.   Generally, rather than
add support for specific boards, we will add support for specific devices
and leave it up to configuration and the DTB selected at runtime to
distinguish among numerous boards supported by the same build.  There are
also a few generally useful features we hope to contribute.

   Several team members at Broadcom will be making contributions in this
effort.  We are a bit light on U-Boot development experience, so feedback
and advice will be appreciated.  We will begin distinct threads on this
list for specific related topics.

-Joel Peshkin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot