Minor improvements to secure boot and enable on beaglebone

Asked by 4 months ago
This series fixes a few problems that have come up since the secure boot series was merged: - A recent commit broken the assumption that u-boot.bin ends at a known address (thus making things appended to U-Boot inaccessible from the code). This is fixed for Beaglebone and also a new test is added to the Makefile to ensure that it does not break again. All boards have been tested. - A way is needed to provide an externally-build device tree binary for U-Boot. This allows signing to happen outside the U-Boot build system. - The .img files generated by an OMAP build need to include the FDT if one is appended. - Adding signatures to an FDT can cause the FDT to run out of space. The fix is to regenerate the FDT from scratch with different dtc parameters, so pretty painful. Instead, we automatically expand the FDT. The last two commits enable secure boot on Beaglebone (this will have no effect unless signed images are used). This could be moved to a separate configuration if required, or these patches could even be ignored: am33xx/omap: Enable FIT support am33xx/omap: Enable secure boot with CONFIG_FIT_SIGNATURE This series has been run through buildman: /tools/buildman/buildman -b talk2 -s Summary of 12 commits for 1210 boards (32 threads, 1 job per thread) 01: Prepare v2014.04 blackfin: + bf609-ezkit m68k: + M54455EVB_a66 M5329AFEE M5249EVB M5208EVBE eb_cpu5282 M54451EVB astro_mcf5373l M54418TWR_serial_rmii M54455EVB_intel M5475FFE M5282EVB M54455EVB_i66 M5475GFE M5253DEMO M54455EVB_stm33 M5485BFE M5485DFE TASREG M5329BFEE M52277EVB M5475EFE M5475CFE cobra5272 M5485AFE M53017EVB M5485HFE M5235EVB M5253EVBE M54418TWR_nand_mii M54418TWR_nand_rmii_lowfreq M5475BFE M54418TWR_nand_rmii M5475DFE M5275EVB M52277EVB_stmicro eb_cpu5282_internal M54451EVB_stmicro M5485GFE M5373EVB M5485EFE M5485FFE M54418TWR M5235EVB_Flash32 M54418TWR_serial_mii M5485CFE M54455EVB M5475AFE M5272C3 powerpc: + SIMPC8313_SP P1023RDS_NAND MPC8569MDS_NAND P2020RDB_NAND MPC8536DS_NAND P1020RDB_NAND MPC8315ERDB_NAND P1011RDB_NAND SIMPC8313_LP MPC8572DS_NAND P2010RDB_NAND sparc: + grsim grsim_leon2 gr_cpci_ax2000 gr_xc3s_1500 gr_ep2s60 sh: + rsk7269 rsk7264 rsk7203 nios2: + nios2-generic PK1C20 microblaze: + microblaze-generic openrisc: + openrisc-generic arm: + tricorder tricorder_flash 02: Check that u-boot.bin size looks correct arm: + am335x_evm_uart5 am335x_evm_uart4 am335x_evm_uart1 am335x_evm_uart3 am335x_boneblack am335x_evm_usbspl am335x_evm_nor cm_t335 am335x_evm_norboot am335x_evm_spiboot am335x_evm am335x_evm_uart2 mx31ads 03: ti: am335x: Fix the U-Boot binary output arm: am335x_evm_uart5 am335x_evm_uart4 am335x_evm_uart1 am335x_evm_uart3 am335x_boneblack am335x_evm_usbspl am335x_evm_nor am335x_evm_norboot am335x_evm_spiboot am335x_evm am335x_evm_uart2 04: am33xx/omap: Allow cache enable for all Sitara/OMAP 05: hash: Export functions to find and show hash 06: fdt: Add DEV_TREE_BIN option to specify a device tree binary file 07: fdt: Update functions which write to an FDT to return -ENOSPC 08: mkimage: Automatically make space in FDT when full 09: arm: ti: Increase malloc size to 16MB for armv7 boards 10: am33xx/omap: Enable CONFIG_OF_CONTROL 11: am33xx/omap: Enable FIT support 12: am33xx/omap: Enable secure boot with CONFIG_FIT_SIGNATURE The breakage in 02 is because I add the check before fixing the problem, in order to verify what is affected. The order can be changed when applying if required. Changes in v2: - Add new patch to check u-boot.bin size against symbol table - Add new patch to ensure the hash section is inside the image for am335x - Update to cover all omap devices - Adjust for kbuild changes - Fix line over 80cols - Move device tree files into arch/arm/dts Simon Glass (11): Check that u-boot.bin size looks correct ti: am335x: Fix the U-Boot binary output am33xx/omap: Allow cache enable for all Sitara/OMAP hash: Export functions to find and show hash fdt: Add DEV_TREE_BIN option to specify a device tree binary file fdt: Update functions which write to an FDT to return -ENOSPC mkimage: Automatically make space in FDT when full arm: ti: Increase malloc size to 16MB for armv7 boards am33xx/omap: Enable CONFIG_OF_CONTROL am33xx/omap: Enable FIT support am33xx/omap: Enable secure boot with CONFIG_FIT_SIGNATURE Makefile | 16 +- arch/arm/cpu/armv7/am33xx/board.c | 8 - arch/arm/cpu/armv7/omap-common/Makefile | 4 + arch/arm/cpu/armv7/omap-common/hwinit-common.c | 42 -- arch/arm/cpu/armv7/omap-common/omap-cache.c | 56 +++ arch/arm/cpu/armv7/omap3/board.c | 8 - arch/arm/dts/Makefile | 1 + arch/arm/dts/am335x-bone-common.dtsi | 262 ++++++++++ arch/arm/dts/am335x-boneblack.dts | 17 + arch/arm/dts/am33xx.dtsi | 649 +++++++++++++++++++++++++ arch/arm/dts/dt-bindings/gpio/gpio.h | 15 + arch/arm/dts/dt-bindings/pinctrl/am33xx.h | 42 ++ arch/arm/dts/dt-bindings/pinctrl/omap.h | 55 +++ arch/arm/dts/tps65217.dtsi | 56 +++ board/ti/am335x/u-boot.lds | 3 +- common/hash.c | 13 +- common/image-fit.c | 4 +- doc/README.fdt-control | 16 +- dts/Makefile | 4 + include/configs/am335x_evm.h | 9 + include/configs/ti_armv7_common.h | 2 +- include/hash.h | 22 + include/rsa.h | 3 +- lib/rsa/rsa-sign.c | 28 +- tools/fit_image.c | 165 +++++-- tools/image-host.c | 26 +- 26 files changed, 1381 insertions(+), 145 deletions(-) create mode 100644 arch/arm/cpu/armv7/omap-common/omap-cache.c create mode 100644 arch/arm/dts/am335x-bone-common.dtsi create mode 100644 arch/arm/dts/am335x-boneblack.dts create mode 100644 arch/arm/dts/am33xx.dtsi create mode 100644 arch/arm/dts/dt-bindings/gpio/gpio.h create mode 100644 arch/arm/dts/dt-bindings/pinctrl/am33xx.h create mode 100644 arch/arm/dts/dt-bindings/pinctrl/omap.h create mode 100644 arch/arm/dts/tps65217.dtsi

Your Answer

Name:
Reply:

All Answers

Answer by 4 months ago
Enable booting a FIT containing a kernel/device tree. Signed-off-by: Simon Glass sjg [ at ] chromium.org Changes in v2: None include/configs/am335x_evm.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h index 3642dc6..4356d37 100644 --- a/include/configs/am335x_evm.h +++ b/include/configs/am335x_evm.h @@ -19,6 +19,7 @@ #include <configs/ti_am335x_common.h> #ifndef CONFIG_SPL_BUILD +# define CONFIG_FIT # define CONFIG_OF_CONTROL # define CONFIG_OF_SEPARATE # define CONFIG_DEFAULT_DEVICE_TREE am335x-boneblack
Answer by 4 months ago
These functions are generally useful for displaying a hash value and finding available algorithms, so export them. Signed-off-by: Simon Glass sjg [ at ] chromium.org Changes in v2: None common/hash.c | 13 ++++++------- include/hash.h | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/common/hash.c b/common/hash.c index 872cd85..f82b9dd 100644 --- a/common/hash.c +++ b/common/hash.c @@ -204,7 +204,7 @@ static int parse_verify_sum(struct hash_algo *algo, char *verify_str, u8 *vsum, return 0; -static struct hash_algo *find_hash_algo(const char *name) +struct hash_algo *hash_find_algo(const char *name) int i; @@ -216,8 +216,7 @@ static struct hash_algo *find_hash_algo(const char *name) return NULL; -static void show_hash(struct hash_algo *algo, ulong addr, ulong len, - u8 *output) +void hash_show(struct hash_algo *algo, ulong addr, ulong len, u8 *output) int i; @@ -231,7 +230,7 @@ int hash_block(const char *algo_name, const void *data, unsigned int len, struct hash_algo *algo; - algo = find_hash_algo(algo_name); + algo = hash_find_algo(algo_name); if (!algo) { debug("Unknown hash algorithm '%s'\n", algo_name); return -EPROTONOSUPPORT; @@ -265,7 +264,7 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, u8 vsum[HASH_MAX_DIGEST_SIZE]; void *buf; - algo = find_hash_algo(algo_name); + algo = hash_find_algo(algo_name); if (!algo) { printf("Unknown hash algorithm '%s'\n", algo_name); return CMD_RET_USAGE; @@ -298,7 +297,7 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, if (memcmp(output, vsum, algo->digest_size) != 0) { int i; - show_hash(algo, addr, len, output); + hash_show(algo, addr, len, output); printf(" != "); for (i = 0; i < algo->digest_size; i++) printf("%02x", vsum[i]); @@ -306,7 +305,7 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, return 1; } else { - show_hash(algo, addr, len, output); + hash_show(algo, addr, len, output); printf("\n"); if (argc) { diff --git a/include/hash.h b/include/hash.h index e92d272..c69bc25 100644 --- a/include/hash.h +++ b/include/hash.h @@ -77,4 +77,26 @@ int hash_command(const char *algo_name, int flags, cmd_tbl_t *cmdtp, int flag, int hash_block(const char *algo_name, const void *data, unsigned int len, uint8_t *output, int *output_size); +/** + * hash_find_algo() - Find an algorithm by name + * + * @name: Name of algorithm to search for + * @return pointer to algorithm structure, or NULL if not found + */ +struct hash_algo *hash_find_algo(const char *name); +/** + * hash_show() - Print out a hash algorithm and value + * + * You will get a message like this (without a newline at the end): + * + * "sha1 for 9eb3337c ... 9eb3338f ==> 7942ef1df479fd3130f716eb9613d107dab7e257" + * + * @algo: Algorithm used for hash + * @addr: Address of data that was hashed + * @len: Length of data that was hashed + * @output: Hash value to display + */ +void hash_show(struct hash_algo *algo, ulong addr, ulong len, u8 *output); #endif
Answer by 4 months ago
This should include the hash so that image_binary_size is really at the end of the image, and not some 300 bytes earlier. Signed-off-by: Simon Glass sjg [ at ] chromium.org Changes in v2: - Add new patch to ensure the hash section is inside the image for am335x board/ti/am335x/u-boot.lds | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/board/ti/am335x/u-boot.lds b/board/ti/am335x/u-boot.lds index a9e3d34..a2dda92 100644 --- a/board/ti/am335x/u-boot.lds +++ b/board/ti/am335x/u-boot.lds @@ -77,6 +77,8 @@ SECTIONS *(.__rel_dyn_end) + .hash : { *(.hash*) } .end : *(.__end) @@ -117,7 +119,6 @@ SECTIONS .dynbss : { *(.dynbss) } .dynstr : { *(.dynstr*) } .dynamic : { *(.dynamic*) } - .hash : { *(.hash*) } .gnu.hash : { *(.gnu.hash) } .plt : { *(.plt*) } .interp : { *(.interp*) }
Answer by 4 months ago
Enable the cache for all devices, unless CONFIG_SYS_DCACHE_OFF is defined. This speeds up the Beaglebone Black boot considerable. (Tested only on Beaglebone Black with SD card boot) Signed-off-by: Simon Glass sjg [ at ] chromium.org Changes in v2: - Update to cover all omap devices arch/arm/cpu/armv7/am33xx/board.c | 8 ---- arch/arm/cpu/armv7/omap-common/Makefile | 4 ++ arch/arm/cpu/armv7/omap-common/hwinit-common.c | 42 ------------------- arch/arm/cpu/armv7/omap-common/omap-cache.c | 56 ++++++++++++++++++++++++++ arch/arm/cpu/armv7/omap3/board.c | 8 ---- 5 files changed, 60 insertions(+), 58 deletions(-) create mode 100644 arch/arm/cpu/armv7/omap-common/omap-cache.c diff --git a/arch/arm/cpu/armv7/am33xx/board.c b/arch/arm/cpu/armv7/am33xx/board.c index fb44cc8..92f3f22 100644 --- a/arch/arm/cpu/armv7/am33xx/board.c +++ b/arch/arm/cpu/armv7/am33xx/board.c @@ -243,11 +243,3 @@ void s_init(void) sdram_init(); #endif -#ifndef CONFIG_SYS_DCACHE_OFF -void enable_caches(void) -{ - /* Enable D-cache. I-cache is already enabled in start.S */ - dcache_enable(); -} -#endif /* !CONFIG_SYS_DCACHE_OFF */ diff --git a/arch/arm/cpu/armv7/omap-common/Makefile b/arch/arm/cpu/armv7/omap-common/Makefile index 59f5352..b7400d5 100644 --- a/arch/arm/cpu/armv7/omap-common/Makefile +++ b/arch/arm/cpu/armv7/omap-common/Makefile @@ -22,6 +22,10 @@ obj-y += pipe3-phy.o obj-$(CONFIG_SCSI_AHCI_PLAT) += sata.o endif +ifeq ($(CONFIG_SYS_DCACHE_OFF),) +obj-y += omap-cache.o +endif ifeq ($(CONFIG_OMAP34XX),) obj-y += boot-common.o obj-y += lowlevel_init.o diff --git a/arch/arm/cpu/armv7/omap-common/hwinit-common.c b/arch/arm/cpu/armv7/omap-common/hwinit-common.c index 8ebc0ce..16baa73 100644 --- a/arch/arm/cpu/armv7/omap-common/hwinit-common.c +++ b/arch/arm/cpu/armv7/omap-common/hwinit-common.c @@ -18,13 +18,8 @@ #include <asm/emif.h> #include <asm/omap_common.h> #include <linux/compiler.h> -#include <asm/cache.h> #include <asm/system.h> -#define ARMV7_DCACHE_WRITEBACK 0xe -#define ARMV7_DOMAIN_CLIENT 1 -#define ARMV7_DOMAIN_MASK (0x3 << 0) DECLARE_GLOBAL_DATA_PTR; void do_set_mux(u32 base, struct pad_conf_entry const *array, int size) @@ -260,40 +255,3 @@ int print_cpuinfo(void) return 0; #endif -#ifndef CONFIG_SYS_DCACHE_OFF -void enable_caches(void) -{ - /* Enable D-cache. I-cache is already enabled in start.S */ - dcache_enable(); -} -void dram_bank_mmu_setup(int bank) -{ - bd_t *bd = gd->bd; - int i; - u32 start = bd->bi_dram[bank].start >> 20; - u32 size = bd->bi_dram[bank].size >> 20; - u32 end = start + size; - debug("%s: bank: %d\n", __func__, bank); - for (i = start; i < end; i++) - set_section_dcache(i, ARMV7_DCACHE_WRITEBACK); -} -void arm_init_domains(void) -{ - u32 reg; - reg = get_dacr(); - /* - * Set DOMAIN to client access so that all permissions - * set in pagetables are validated by the mmu. - */ - reg = ~ARMV7_DOMAIN_MASK; - reg |= ARMV7_DOMAIN_CLIENT; - set_dacr(reg); -} -#endif diff --git a/arch/arm/cpu/armv7/omap-common/omap-cache.c b/arch/arm/cpu/armv7/omap-common/omap-cache.c new file mode 100644 index 0000000..579bebf --- /dev/null +++ b/arch/arm/cpu/armv7/omap-common/omap-cache.c @@ -0,0 +1,56 @@ +/* + * + * Common functions for OMAP4/5 based boards + * + * (C) Copyright 2010 + * Texas Instruments, <www.ti.com> + * + * Author : + * Aneesh V aneesh [ at ] ti.com + * Steve Sakoman steve [ at ] sakoman.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ +#include <common.h> +#include <asm/cache.h> +DECLARE_GLOBAL_DATA_PTR; +#define ARMV7_DCACHE_WRITEBACK 0xe +#define ARMV7_DOMAIN_CLIENT 1 +#define ARMV7_DOMAIN_MASK (0x3 << 0) +void enable_caches(void) +{ + /* Enable D-cache. I-cache is already enabled in start.S */ + dcache_enable(); +} +void dram_bank_mmu_setup(int bank) +{ + bd_t *bd = gd->bd; + int i; + u32 start = bd->bi_dram[bank].start >> 20; + u32 size = bd->bi_dram[bank].size >> 20; + u32 end = start + size; + debug("%s: bank: %d\n", __func__, bank); + for (i = start; i < end; i++) + set_section_dcache(i, ARMV7_DCACHE_WRITEBACK); +} +void arm_init_domains(void) +{ + u32 reg; + reg = get_dacr(); + /* + * Set DOMAIN to client access so that all permissions + * set in pagetables are validated by the mmu. + */ + reg = ~ARMV7_DOMAIN_MASK; + reg |= ARMV7_DOMAIN_CLIENT; + set_dacr(reg); +} diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c index 2922816..74c037b 100644 --- a/arch/arm/cpu/armv7/omap3/board.c +++ b/arch/arm/cpu/armv7/omap3/board.c @@ -478,11 +478,3 @@ void omap3_outer_cache_disable(void) omap3_update_aux_cr(0, 0x2); #endif /* !CONFIG_SYS_L2CACHE_OFF */ -#ifndef CONFIG_SYS_DCACHE_OFF -void enable_caches(void) -{ - /* Enable D-cache. I-cache is already enabled in start.S */ - dcache_enable(); -} -#endif /* !CONFIG_SYS_DCACHE_OFF */
Answer by 4 months ago
In some cases, an externally-built device tree binary is required to be attached to U-Boot. An example is when using image signing, since in that case the .dtb file must include the public keys. Add a DEV_TREE_BIN option to the Makefile, and update the documentation. Usage is something like: make DEV_TREE_BIN=boot/am335x-boneblack-pubkey.dtb Signed-off-by: Simon Glass sjg [ at ] chromium.org Changes in v2: - Adjust for kbuild changes Makefile | 2 +- doc/README.fdt-control | 16 ++++++++++++++-- dts/Makefile | 4 ++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 6ca4bf6..aed07a2 100644 --- a/Makefile +++ b/Makefile @@ -815,7 +815,7 @@ MKIMAGEFLAGS_u-boot.kwb = -n $(srctree)/$(CONFIG_SYS_KWD_CONFIG:"%"=%) \ MKIMAGEFLAGS_u-boot.pbl = -n $(srctree)/$(CONFIG_SYS_FSL_PBL_RCW:"%"=%) \ -R $(srctree)/$(CONFIG_SYS_FSL_PBL_PBI:"%"=%) -T pblimage -u-boot.img u-boot.kwb u-boot.pbl: u-boot.bin FORCE +u-boot.img u-boot.kwb u-boot.pbl: u-boot$(if $(CONFIG_OF_SEPARATE),-dtb,).bin FORCE $(call if_changed,mkimage) u-boot.sha1: u-boot.bin diff --git a/doc/README.fdt-control b/doc/README.fdt-control index 86bae68..8a4aa7a 100644 --- a/doc/README.fdt-control +++ b/doc/README.fdt-control @@ -122,7 +122,8 @@ This should include your CPU or SOC's device tree file, placed in arch/<arch>/dts, and then make any adjustments required. If CONFIG_OF_EMBED is defined, then it will be picked up and built into -the U-Boot image (including u-boot.bin). +the U-Boot image (including u-boot.bin). This is suitable for debugging +and development only and is not recommended for production devices. If CONFIG_OF_SEPARATE is defined, then it will be built and placed in a u-boot.dtb file alongside u-boot.bin. A common approach is then to @@ -130,7 +131,10 @@ join the two: cat u-boot.bin u-boot.dtb >image.bin -and then flash image.bin onto your board. +and then flash image.bin onto your board. Note that U-Boot creates +u-boot-dtb.bin which does the above step for you also. If you are using +CONFIG_SPL_FRAMEWORK, then u-boot.img will be built to include the device +tree binary. If CONFIG_OF_HOSTFILE is defined, then it will be read from a file on startup. This is only useful for sandbox. Use the -d flag to U-Boot to @@ -138,6 +142,14 @@ specify the file to read. You cannot use more than one of these options at the same time. +To use a device tree file that you have compiled yourself, pass +DEV_TREE_BIN=<filename> to 'make', as in: + make DEV_TREE_BIN=boot/am335x-boneblack-pubkey.dtb +Then U-Boot will copy that file to u-boot.dtb, put it in the .img file +if used, and u-boot-dtb.bin. If you wish to put the fdt at a different address in memory, you can define the "fdtcontroladdr" environment variable. This is the hex address of the fdt binary blob, and will override either of the options. diff --git a/dts/Makefile b/dts/Makefile index e59550c..8bb5212 100644 --- a/dts/Makefile +++ b/dts/Makefile @@ -12,7 +12,11 @@ ifeq ($(DEVICE_TREE),) DEVICE_TREE := unset endif +ifneq ($(DEV_TREE_BIN),) +DTB := $(DEV_TREE_BIN) +else DTB := arch/$(ARCH)/dts/$(DEVICE_TREE).dtb +endif quiet_cmd_copy = COPY $@ cmd_copy = cp $< $@
Answer by 4 months ago
Enable secure boot functionality. Signed-off-by: Simon Glass sjg [ at ] chromium.org Changes in v2: None include/configs/am335x_evm.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h index 4356d37..7b4d7bf 100644 --- a/include/configs/am335x_evm.h +++ b/include/configs/am335x_evm.h @@ -20,6 +20,8 @@ #ifndef CONFIG_SPL_BUILD # define CONFIG_FIT +# define CONFIG_FIT_SIGNATURE +# define CONFIG_RSA # define CONFIG_OF_CONTROL # define CONFIG_OF_SEPARATE # define CONFIG_DEFAULT_DEVICE_TREE am335x-boneblack
Answer by 4 months ago
The current size of 1MB is not enough use to use DFU. Increase it for ARMv7 boards, all of which should have 32MB or more SDRAM. With this change it is possible to do 'dfu mmc 0' on a Beaglebone Black. Signed-off-by: Simon Glass sjg [ at ] chromium.org Changes in v2: None include/configs/ti_armv7_common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/configs/ti_armv7_common.h b/include/configs/ti_armv7_common.h index 69d69a5..a48eef1 100644 --- a/include/configs/ti_armv7_common.h +++ b/include/configs/ti_armv7_common.h @@ -104,7 +104,7 @@ * we are on so we do not need to rely on the command prompt. We set a * console baudrate of 115200 and use the default baud rate table. */ -#define CONFIG_SYS_MALLOC_LEN (1024 << 10) +#define CONFIG_SYS_MALLOC_LEN (16 << 20) #define CONFIG_SYS_HUSH_PARSER #define CONFIG_SYS_PROMPT "U-Boot# " #define CONFIG_SYS_CONSOLE_INFO_QUIET
Answer by 4 months ago
When writing values into an FDT it is possible that there will be insufficient space. If the caller gets a useful error in the then it can potentially deal with the situation. Adjust these functions to return -ENOSPC when the FDT is full. Signed-off-by: Simon Glass sjg [ at ] chromium.org Changes in v2: - Fix line over 80cols common/image-fit.c | 4 ++-- include/rsa.h | 3 ++- lib/rsa/rsa-sign.c | 28 +++++++++++++++++++--------- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/common/image-fit.c b/common/image-fit.c index b94a3fe..8ba73a6 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -832,7 +832,7 @@ static int fit_image_hash_get_ignore(const void *fit, int noffset, int *ignore) * returns: * 0, on success - * -1, on property read failure + * -ENOSPC if no space in device tree, -1 for other error */ int fit_set_timestamp(void *fit, int noffset, time_t timestamp) @@ -846,7 +846,7 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp) printf("Can't set '%s' property for '%s' node (%s)\n", FIT_TIMESTAMP_PROP, fit_get_name(fit, noffset, NULL), fdt_strerror(ret)); - return -1; + return ret == -FDT_ERR_NOSPACE ? -ENOSPC : -1; return 0; diff --git a/include/rsa.h b/include/rsa.h index add4c78..0db76cc 100644 --- a/include/rsa.h +++ b/include/rsa.h @@ -46,7 +46,8 @@ int rsa_sign(struct image_sign_info *info, * @info: Specifies key and FIT information * @keydest: Destination FDT blob for public key data - * @return: 0, on success, -ve on error + * @return: 0, on success, -ENOSPC if the keydest FDT blob ran out of space, + other -ve value on error */ int rsa_add_verify_data(struct image_sign_info *info, void *keydest); #else diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c index 549130e..ef9a2f4 100644 --- a/lib/rsa/rsa-sign.c +++ b/lib/rsa/rsa-sign.c @@ -427,20 +427,30 @@ int rsa_add_verify_data(struct image_sign_info *info, void *keydest) ret = fdt_setprop_string(keydest, node, "key-name-hint", info->keyname); - ret |= fdt_setprop_u32(keydest, node, "rsa,num-bits", bits); - ret |= fdt_setprop_u32(keydest, node, "rsa,n0-inverse", n0_inv); - ret |= fdt_add_bignum(keydest, node, "rsa,modulus", modulus, bits); - ret |= fdt_add_bignum(keydest, node, "rsa,r-squared", r_squared, bits); - ret |= fdt_setprop_string(keydest, node, FIT_ALGO_PROP, - info->algo->name); + if (!ret) + ret = fdt_setprop_u32(keydest, node, "rsa,num-bits", bits); + if (!ret) + ret = fdt_setprop_u32(keydest, node, "rsa,n0-inverse", n0_inv); + if (!ret) { + ret = fdt_add_bignum(keydest, node, "rsa,modulus", modulus, + bits); + } + if (!ret) { + ret = fdt_add_bignum(keydest, node, "rsa,r-squared", r_squared, + bits); + } + if (!ret) { + ret = fdt_setprop_string(keydest, node, FIT_ALGO_PROP, + info->algo->name); + } if (info->require_keys) { - fdt_setprop_string(keydest, node, "required", - info->require_keys); + ret = fdt_setprop_string(keydest, node, "required", + info->require_keys); BN_free(modulus); BN_free(r_squared); if (ret) - return -EIO; + return ret == FDT_ERR_NOSPACE ? -ENOSPC : -EIO; return 0;
Answer by 4 months ago
Check that the image size matches the size we get from u-boot.bin. If it doesn't, that generally means that some extra sections are being added to u-boot.bin, meaning that it is not possible to access data appended to the U-Boot binary. This is used for device tree, so needs to work. This problem was introduced by commit b02bfc4. By adding a test we can prevent a reccurence. Signed-off-by: Simon Glass sjg [ at ] chromium.org Changes in v2: - Add new patch to check u-boot.bin size against symbol table Makefile | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c91c10e..6ca4bf6 100644 --- a/Makefile +++ b/Makefile @@ -695,7 +695,7 @@ DO_STATIC_RELA = endif # Always append ALL so that arch config.mk's can add custom ones -ALL-y += u-boot.srec u-boot.bin System.map +ALL-y += u-boot.srec u-boot.bin System.map binary_size_check ALL-$(CONFIG_NAND_U_BOOT) += u-boot-nand.bin ALL-$(CONFIG_ONENAND_U_BOOT) += u-boot-onenand.bin @@ -768,6 +768,18 @@ u-boot.hex u-boot.srec: u-boot FORCE OBJCOPYFLAGS_u-boot.bin := -O binary +binary_size_check: u-boot.bin System.map FORCE + @file_size=`stat -c %s u-boot.bin` ; \ + map_size=$(shell cat System.map | \ + awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" & end != "") print strtonum("0x" end) - strtonum("0x" start)}'); \ + if [ "" != "$$map_size" ]; then \ + if test $$map_size -ne $$file_size; then \ + echo "System.map shows a binary size of $$map_size" > ; \ + echo " but u-boot.bin shows $$file_size" > ; \ + exit 1; \ + fi \ + fi u-boot.bin: u-boot FORCE $(call if_changed,objcopy) $(call DO_STATIC_RELA,$<,$@,$(CONFIG_SYS_TEXT_BASE))
Answer by 3 months ago
Hi Simon, I've tested this patch series and I found some issues. When I use dtb build from latest 3.15-rc3 kernel I got during signing this errors: Couldn't create signature node: FDT_ERR_NOSPACE Failed to add verification data for 'signature [ at ] 1' signature node in 'conf [ at ] 1' image node which was fixed by those 2 small patches: - this one doesn't overwrite return value because upper layer then stop with no space error and doesn't allocate more space --- a/lib/rsa/rsa-sign.c +++ b/lib/rsa/rsa-sign.c @@ -405,7 +405,7 @@ int rsa_add_verify_data(struct image_sign_info *info, void *keydest) if (parent < 0) { fprintf(stderr, "Couldn't create signature node: %s\n", fdt_strerror(parent)); - return -EINVAL; + return parent; --- a/tools/image-host.c +++ b/tools/image-host.c @@ -612,7 +612,7 @@ static int fit_config_process_sig(const char *keydir, void *keydest, if (ret) { printf("Failed to add verification data for '%s' signature node in '%s' image node\n", node_name, conf_name); - return ret == FDT_ERR_NOSPACE ? -ENOSPC : -EIO; + return ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO; With this small changes I can create signed fit image. Other problem appear during booting. I'm using simple uEnv.txt to get fit image to ram and boot (setenv loadaddr '0x8050000'; run loadimage; bootm). Booting of kernel fails with data abort: Importing environment from mmc ... Running uenvcmd ... reading /uImage 4322274 bytes read in 585 ms (7 MiB/s) ## Loading kernel from FIT Image at 80500000 ... Using 'conf [ at ] 1' configuration Verifying Hash Integrity ... sha1,rsa2048:dev+ OK Trying 'kernel [ at ] 1' kernel subimage Description: Linux kernel Type: Kernel Image Compression: uncompressed Data Start: 0x805000e4 Data Size: 4289584 Bytes = 4.1 MiB Architecture: ARM OS: Linux Load Address: 0x80008000 Entry Point: 0x80008000 Hash algo: sha1 Hash value: 74d429a5c48d72ce3f569ba7eaa072c8c1eaab20 Verifying Hash Integrity ... sha1+ OK ## Loading fdt from FIT Image at 80500000 ... Using 'conf [ at ] 1' configuration Trying 'fdt [ at ] 1' fdt subimage Description: Flattened Device Tree blob Type: Flat Device Tree Compression: uncompressed Data Start: 0x80917608 Data Size: 29802 Bytes = 29.1 KiB Architecture: ARM Hash algo: sha1 Hash value: e86cfd55c3e869c6b3014c758825b2a1ade3991e Verifying Hash Integrity ... sha1+ OK Booting using the fdt blob at 0x80917608 Loading Kernel Image ... OK Using Device Tree in place at 80917608, end 80921a71 Starting kernel ... data abort pc : [<81a80020>] lr : [<80008008>] sp : 8e71b528 ip : 0000000c fp : 00000400 r10: 8f7a3d60 r9 : 8e723f28 r8 : 00000000 r7 : 00000000 r6 : 00000ffc r5 : 0ffc0004 r4 : 000000f7 r3 : fc7391ff r2 : 80917608 r1 : 00000e05 r0 : 80917608 Flags: Nzcv IRQs off FIQs on Mode SVC_32 Resetting CPU ... I wasn't able yet track down what is causing this issue but it happened when jumping to kernel image (kernel_entry(0, machid, r2);). Any ideas what to check? Thanks in advance. Ruska Nova Ves 219 | Presov, 08005 Slovak Republic Tel: +421 915 052 184 skype: marekwhite twitter: #opennandra web: http://open-nandra.com
Answer by 2 months ago
Hi Belisko, For this I'm not sure, perhaps your load address is wrong? I just sent out a new series (available in u-boot-x86.git branch 'bone') which adds some step-by-step documentation. It also collects all the fixes in one place. [snip] Regards, Simon
Answer by 2 months ago
Quoted message by Belisko Marek 3 months ago
Hi Simon, I've tested this patch series and I found some issues. When I use dtb build from latest 3.15-rc3 kernel I got during signing this errors: Couldn't create signature node: FDT_ERR_NOSPACE Failed to add verification data for 'signature [ at ] 1' signature node in 'conf [ at ] 1' image node which was fixed by those 2 small patches: - this one doesn't overwrite return value because upper layer then stop with no space error and doesn't allocate more space --- a/lib/rsa/rsa-sign.c +++ b/lib/rsa/rsa-sign.c @@ -405,7 +405,7 @@ int rsa_add_verify_data(struct image_sign_info *info, void *keydest) if (parent < 0) { fprintf(stderr, "Couldn't create signature node: %s\n", fdt_strerror(parent)); - return -EINVAL; + return parent; --- a/tools/image-host.c +++ b/tools/image-host.c @@ -612,7 +612,7 @@ static int fit_config_process_sig(const char *keydir, void *keydest, if (ret) { printf("Failed to add verification data for '%s' signature node in '%s' image node\n", node_name, conf_name); - return ret == FDT_ERR_NOSPACE ? -ENOSPC : -EIO; + return ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO; With this small changes I can create signed fit image. Other problem appear during booting. I'm using simple uEnv.txt to get fit image to ram and boot (setenv loadaddr '0x8050000'; run loadimage; bootm). Booting of kernel fails with data abort: Importing environment from mmc ... Running uenvcmd ... reading /uImage 4322274 bytes read in 585 ms (7 MiB/s) ## Loading kernel from FIT Image at 80500000 ... Using 'conf [ at ] 1' configuration Verifying Hash Integrity ... sha1,rsa2048:dev+ OK Trying 'kernel [ at ] 1' kernel subimage Description: Linux kernel Type: Kernel Image Compression: uncompressed Data Start: 0x805000e4 Data Size: 4289584 Bytes = 4.1 MiB Architecture: ARM OS: Linux Load Address: 0x80008000 Entry Point: 0x80008000 Hash algo: sha1 Hash value: 74d429a5c48d72ce3f569ba7eaa072c8c1eaab20 Verifying Hash Integrity ... sha1+ OK ## Loading fdt from FIT Image at 80500000 ... Using 'conf [ at ] 1' configuration Trying 'fdt [ at ] 1' fdt subimage Description: Flattened Device Tree blob Type: Flat Device Tree Compression: uncompressed Data Start: 0x80917608 Data Size: 29802 Bytes = 29.1 KiB Architecture: ARM Hash algo: sha1 Hash value: e86cfd55c3e869c6b3014c758825b2a1ade3991e Verifying Hash Integrity ... sha1+ OK Booting using the fdt blob at 0x80917608 Loading Kernel Image ... OK Using Device Tree in place at 80917608, end 80921a71 Starting kernel ... data abort pc : [<81a80020>] lr : [<80008008>] sp : 8e71b528 ip : 0000000c fp : 00000400 r10: 8f7a3d60 r9 : 8e723f28 r8 : 00000000 r7 : 00000000 r6 : 00000ffc r5 : 0ffc0004 r4 : 000000f7 r3 : fc7391ff r2 : 80917608 r1 : 00000e05 r0 : 80917608 Flags: Nzcv IRQs off FIQs on Mode SVC_32 Resetting CPU ... I wasn't able yet track down what is causing this issue but it happened when jumping to kernel image (kernel_entry(0, machid, r2);). Any ideas what to check? Thanks in advance. Ruska Nova Ves 219 | Presov, 08005 Slovak Republic Tel: +421 915 052 184 skype: marekwhite twitter: #opennandra web: http://open-nandra.com
Dear Simon Glass, Yes I've fixed this issue some time ago. What about those two mentioned patches? Without them I cannot create FIT properly. Thanks I'll look on that. BR, marek Ruska Nova Ves 219 | Presov, 08005 Slovak Republic Tel: +421 915 052 184 skype: marekwhite twitter: #opennandra web: http://open-nandra.com
Answer by 2 months ago
Quoted message by Simon Glass 2 months ago
Hi Belisko, For this I'm not sure, perhaps your load address is wrong? I just sent out a new series (available in u-boot-x86.git branch 'bone') which adds some step-by-step documentation. It also collects all the fixes in one place. [snip] Regards, Simon
Hi, Here we must return a value like -ENOSPC if we want to signal that. Also it should be done before printing the error. This looks right to me. [snip] I think your patches are useful - my code was aimed at fixing this problem for the FIT image, and adding the large signature block, but it can just as easily happen with the U-Boot fdt. I will take a look at rolling it into a new version, but if you come up with a patch on top of my latest series, please send it (u-boot-x86.git branch 'bone') Regards, Simon