[PATCH v2 2/2] cmd: mvebu/bubt: Check for A38x/A37xx OTP secure bits and secure boot
Stefan Roese
sr at denx.de
Tue Aug 23 07:05:33 CEST 2022
Hi Pali,
On 09.08.22 21:42, Pali Rohár wrote:
> For obvious reasons BootROMS rejects unsigned images when secure boot is
> enabled in OTP secure bits. So check for OPT secure bits and do not allow
> flashing unsigned images when secure boot is enabled. Access to OTP via
> U-Boot fuse API is currently implemented only for A38x and A37xx SoCs.
>
> Additionally Armada 3700 BootROM rejects signed trusted image when secure
> boot is not enabled in OTP. So add also check for this case. On the other
> hand Armada 38x BootROM acceps images with secure boot header when secure
> boot is not enabled in OTP.
>
> OTP secure bits may have burned also boot device source. Check it also and
> reject flashing images to target storage which does not match OTP.
>
> Signed-off-by: Pali Rohár <pali at kernel.org>
>
> ---
> Changes in v2:
> * Add missing dependency on MVEBU_EFUSE
> * Use only for A38x and A37xx
Running a world CI build leads to these errors:
$ make clearfog_gt_8k_defconfig
...
$ make -sj
cmd/mvebu/bubt.c:809:12: warning: 'fuse_read_u64' defined but not used
[-Wunused-function]
809 | static u64 fuse_read_u64(u32 bank)
| ^~~~~~~~~~~~~
Could you please please take a look and fix this issue?
Thanks,
Stefan
> ---
> cmd/mvebu/Kconfig | 1 +
> cmd/mvebu/bubt.c | 127 +++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 120 insertions(+), 8 deletions(-)
>
> diff --git a/cmd/mvebu/Kconfig b/cmd/mvebu/Kconfig
> index 120397d6d4d0..9ec3aa983a51 100644
> --- a/cmd/mvebu/Kconfig
> +++ b/cmd/mvebu/Kconfig
> @@ -5,6 +5,7 @@ config CMD_MVEBU_BUBT
> bool "bubt"
> select SHA256 if ARMADA_3700
> select SHA512 if ARMADA_3700
> + select MVEBU_EFUSE if ARMADA_38X || ARMADA_3700
> help
> bubt - Burn a u-boot image to flash
> For details about bubt command please see the documentation
> diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
> index 3b6ffb7ffd1f..feb1e778a98c 100644
> --- a/cmd/mvebu/bubt.c
> +++ b/cmd/mvebu/bubt.c
> @@ -13,6 +13,8 @@
> #include <vsprintf.h>
> #include <errno.h>
> #include <dm.h>
> +#include <fuse.h>
> +#include <mach/efuse.h>
>
> #include <spi_flash.h>
> #include <spi.h>
> @@ -121,6 +123,17 @@ struct a38x_main_hdr_v1 {
> u8 checksum; /* 0x1F */
> };
>
> +/*
> + * Header for the optional headers, version 1 (Armada 370/XP/375/38x/39x)
> + */
> +struct a38x_opt_hdr_v1 {
> + u8 headertype;
> + u8 headersz_msb;
> + u16 headersz_lsb;
> + u8 data[0];
> +};
> +#define A38X_OPT_HDR_V1_SECURE_TYPE 0x1
> +
> struct a38x_boot_mode {
> unsigned int id;
> const char *name;
> @@ -706,6 +719,7 @@ static int check_image_header(void)
> {
> u8 checksum;
> u32 checksum32, exp_checksum32;
> + u32 offset, size;
> const struct a38x_main_hdr_v1 *hdr =
> (struct a38x_main_hdr_v1 *)get_load_addr();
> const size_t image_size = a38x_header_size(hdr);
> @@ -752,6 +766,38 @@ static int check_image_header(void)
> printf("Image checksum...OK!\n");
> return 0;
> }
> +
> +#if defined(CONFIG_ARMADA_38X)
> +static int a38x_image_is_secure(const struct a38x_main_hdr_v1 *hdr)
> +{
> + u32 image_size = a38x_header_size(hdr);
> + struct a38x_opt_hdr_v1 *ohdr;
> + u32 ohdr_size;
> +
> + if (hdr->version != 1)
> + return 0;
> +
> + if (!hdr->ext)
> + return 0;
> +
> + ohdr = (struct a38x_opt_hdr_v1 *)(hdr + 1);
> + do {
> + if (ohdr->headertype == A38X_OPT_HDR_V1_SECURE_TYPE)
> + return 1;
> +
> + ohdr_size = (ohdr->headersz_msb << 16) | le16_to_cpu(ohdr->headersz_lsb);
> +
> + if (!*((u8 *)ohdr + ohdr_size - 4))
> + break;
> +
> + ohdr = (struct a38x_opt_hdr_v1 *)((u8 *)ohdr + ohdr_size);
> + if ((u8 *)ohdr >= (u8 *)hdr + image_size)
> + break;
> + } while (1);
> +
> + return 0;
> +}
> +#endif
> #else /* Not ARMADA? */
> static int check_image_header(void)
> {
> @@ -760,20 +806,58 @@ static int check_image_header(void)
> }
> #endif
>
> +static u64 fuse_read_u64(u32 bank)
> +{
> + u32 val[2];
> + int ret;
> +
> + ret = fuse_read(bank, 0, &val[0]);
> + if (ret < 0)
> + return -1;
> +
> + ret = fuse_read(bank, 1, &val[1]);
> + if (ret < 0)
> + return -1;
> +
> + return ((u64)val[1] << 32) | val[0];
> +}
> +
> +#if defined(CONFIG_ARMADA_3700)
> +static inline u8 maj3(u8 val)
> +{
> + /* return majority vote of 3 bits */
> + return ((val & 0x7) == 3 || (val & 0x7) > 4) ? 1 : 0;
> +}
> +#endif
> +
> static int bubt_check_boot_mode(const struct bubt_dev *dst)
> {
> #if defined(CONFIG_ARMADA_3700) || defined(CONFIG_ARMADA_32BIT)
> - int mode;
> + int mode, secure_mode;
> #if defined(CONFIG_ARMADA_3700)
> const struct tim_boot_flash_sign *boot_modes = tim_boot_flash_signs;
> const struct common_tim_data *hdr =
> (struct common_tim_data *)get_load_addr();
> u32 id = hdr->boot_flash_sign;
> + int is_secure = hdr->trusted != 0;
> + u64 otp_secure_bits = fuse_read_u64(1);
> + int otp_secure_boot = ((maj3(otp_secure_bits >> 0) << 0) |
> + (maj3(otp_secure_bits >> 4) << 1)) == 2;
> + unsigned int otp_boot_device = (maj3(otp_secure_bits >> 48) << 0) |
> + (maj3(otp_secure_bits >> 52) << 1) |
> + (maj3(otp_secure_bits >> 56) << 2) |
> + (maj3(otp_secure_bits >> 60) << 3);
> #elif defined(CONFIG_ARMADA_32BIT)
> const struct a38x_boot_mode *boot_modes = a38x_boot_modes;
> const struct a38x_main_hdr_v1 *hdr =
> (struct a38x_main_hdr_v1 *)get_load_addr();
> u32 id = hdr->blockid;
> +#if defined(CONFIG_ARMADA_38X)
> + int is_secure = a38x_image_is_secure(hdr);
> + u64 otp_secure_bits = fuse_read_u64(EFUSE_LINE_SECURE_BOOT);
> + int otp_secure_boot = otp_secure_bits & 0x1;
> + unsigned int otp_boot_device = (otp_secure_bits >> 8) & 0x7;
> +#endif
> #endif
>
> for (mode = 0; boot_modes[mode].name; mode++) {
> @@ -786,15 +870,42 @@ static int bubt_check_boot_mode(const struct bubt_dev *dst)
> return -ENOEXEC;
> }
>
> - if (strcmp(boot_modes[mode].name, dst->name) == 0)
> - return 0;
> + if (strcmp(boot_modes[mode].name, dst->name) != 0) {
> + printf("Error: image meant to be booted from \"%s\", not \"%s\"!\n",
> + boot_modes[mode].name, dst->name);
> + return -ENOEXEC;
> + }
>
> - printf("Error: image meant to be booted from \"%s\", not \"%s\"!\n",
> - boot_modes[mode].name, dst->name);
> - return -ENOEXEC;
> -#else
> - return 0;
> +#if defined(CONFIG_ARMADA_38X) || defined(CONFIG_ARMADA_3700)
> + if (otp_secure_bits == (u64)-1) {
> + printf("Error: cannot read OTP secure bits\n");
> + return -ENOEXEC;
> + } else {
> + if (otp_secure_boot && !is_secure) {
> + printf("Error: secure boot is enabled in OTP but image does not have secure boot header!\n");
> + return -ENOEXEC;
> + } else if (!otp_secure_boot && is_secure) {
> +#if defined(CONFIG_ARMADA_3700)
> + /*
> + * Armada 3700 BootROM rejects trusted image when secure boot is not enabled.
> + * Armada 385 BootROM accepts image with secure boot header also when secure boot is not enabled.
> + */
> + printf("Error: secure boot is disabled in OTP but image has secure boot header!\n");
> + return -ENOEXEC;
> #endif
> + } else if (otp_boot_device && otp_boot_device != id) {
> + for (secure_mode = 0; boot_modes[secure_mode].name; secure_mode++) {
> + if (boot_modes[secure_mode].id == otp_boot_device)
> + break;
> + }
> + printf("Error: boot source is set to \"%s\" in OTP but image is for \"%s\"!\n",
> + boot_modes[secure_mode].name ?: "unknown", dst->name);
> + return -ENOEXEC;
> + }
> + }
> +#endif
> +#endif
> + return 0;
> }
>
> static int bubt_verify(const struct bubt_dev *dst)
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
More information about the U-Boot
mailing list