* [U-Boot] [PATCH 0/4] mpc8313: ids8313 board updates
@ 2014-05-08 11:05 Heiko Schocher
2014-05-08 11:05 ` [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format Heiko Schocher
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Heiko Schocher @ 2014-05-08 11:05 UTC (permalink / raw)
To: u-boot
- introduce CONFIG_DISABLE_IMAGE_FORMAT_LEGACY for disabling
booting legacy image format, as this board use only signed
FIT images, and activiate it for the ids8313 board.
- add CONFIG_SYS_GENERIC_BOARD to the ids8313 board,
therefore fdtdec_get_int() is moved out of lib/fdtdec.c
as lib/fdtdec.c is only compiled if CONFIG_OF_CONTROL
is defined, but defining this for the ids8313 board
leads in conjunction with CONFIG_SYS_GENERIC_BOARD
in booting error:
No valid FDT found - please append one to U-Boot binary, use u-boot-dtb.bin or define CONFIG_OF_EMBED. For sandbox, use -d <file.dtb>
So move the common used function fdtdec_get_int()
out of lib/fdtdec.c into lib/fdtdec_common.c
Cc: Simon Glass <sjg@chromium.org>
Cc: Kim Phillips <kim.phillips@freescale.com>
Cc: Wolfgang Denk <wd@denx.de>
Heiko Schocher (4):
bootm: allow to disable legacy image format
mpc8313, signed fit: disable legacy image format on ids8313 board
lib, fdt: move fdtdec_get_int() out of lib/fdtdec.c
mpc8313: add CONFIG_SYS_GENERIC_BOARD to ids8313 board
README | 5 +++++
common/cmd_bootm.c | 14 ++++++++++++
common/cmd_disk.c | 4 ++++
common/cmd_fdc.c | 4 ++++
common/cmd_fpga.c | 2 ++
common/cmd_nand.c | 4 ++++
common/cmd_source.c | 4 ++++
common/cmd_ximg.c | 7 +++++-
common/image-fdt.c | 10 +++++++--
common/image.c | 23 +++++++++++++------
doc/uImage.FIT/signature.txt | 3 +++
include/configs/ids8313.h | 4 +++-
include/image.h | 2 ++
lib/Makefile | 1 +
lib/fdtdec.c | 18 ---------------
lib/fdtdec_common.c | 53 ++++++++++++++++++++++++++++++++++++++++++++
tools/fdtdec.c | 1 +
17 files changed, 130 insertions(+), 29 deletions(-)
create mode 100644 lib/fdtdec_common.c
--
1.8.3.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format
2014-05-08 11:05 [U-Boot] [PATCH 0/4] mpc8313: ids8313 board updates Heiko Schocher
@ 2014-05-08 11:05 ` Heiko Schocher
2014-05-08 13:02 ` mike
2014-05-08 11:05 ` [U-Boot] [PATCH 2/4] mpc8313, signed fit: disable legacy image format on ids8313 board Heiko Schocher
` (2 subsequent siblings)
3 siblings, 1 reply; 18+ messages in thread
From: Heiko Schocher @ 2014-05-08 11:05 UTC (permalink / raw)
To: u-boot
Disabling legacy image format is useful when using
signed FIT images with required signature check.
Use CONFIG_DISABLE_IMAGE_FORMAT_LEGACY in the board
config file.
Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Lars Steubesand <lars.steubesand@philips.com>
Cc: Mike Pearce <mike@kaew.be>
Cc: Wolfgang Denk <wd@denx.de>
---
README | 5 +++++
common/cmd_bootm.c | 14 ++++++++++++++
common/cmd_disk.c | 4 ++++
common/cmd_fdc.c | 4 ++++
common/cmd_fpga.c | 2 ++
common/cmd_nand.c | 4 ++++
common/cmd_source.c | 4 ++++
common/cmd_ximg.c | 7 ++++++-
common/image-fdt.c | 10 ++++++++--
common/image.c | 23 ++++++++++++++++-------
doc/uImage.FIT/signature.txt | 3 +++
include/image.h | 2 ++
12 files changed, 72 insertions(+), 10 deletions(-)
diff --git a/README b/README
index b973344..b1f0ea3 100644
--- a/README
+++ b/README
@@ -3152,6 +3152,11 @@ FIT uImage format:
using a hash signed and verified using RSA. See
doc/uImage.FIT/signature.txt for more details.
+ CONFIG_DISABLE_IMAGE_FORMAT_LEGACY
+ WARNING: When relying on signed FIT images with required
+ signature check the legacy image format should be disabled by
+ using this define.
+
- Standalone program support:
CONFIG_STANDALONE_LOAD_ADDR
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index c243a5b..69b86ee 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -233,6 +233,7 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc,
/* get image parameters */
switch (genimg_get_format(os_hdr)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
images.os.type = image_get_type(os_hdr);
images.os.comp = image_get_comp(os_hdr);
@@ -241,6 +242,7 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc,
images.os.end = image_get_image_end(os_hdr);
images.os.load = image_get_load(os_hdr);
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
if (fit_image_get_type(images.fit_hdr_os,
@@ -838,6 +840,7 @@ int bootm_maybe_autostart(cmd_tbl_t *cmdtp, const char *cmd)
return 0;
}
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
/**
* image_get_kernel - verify legacy format kernel image
* @img_addr: in RAM address of the legacy format image to be verified
@@ -888,6 +891,7 @@ static image_header_t *image_get_kernel(ulong img_addr, int verify)
}
return hdr;
}
+#endif
/**
* boot_get_kernel - find kernel image
@@ -905,7 +909,9 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[], bootm_headers_t *images, ulong *os_data,
ulong *os_len)
{
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
image_header_t *hdr;
+#endif
ulong img_addr;
const void *buf;
#if defined(CONFIG_FIT)
@@ -943,6 +949,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
*os_data = *os_len = 0;
buf = map_sysmem(img_addr, 0);
switch (genimg_get_format(buf)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
printf("## Booting kernel from Legacy Image at %08lx ...\n",
img_addr);
@@ -985,6 +992,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
images->legacy_hdr_valid = 1;
bootstage_mark(BOOTSTAGE_ID_DECOMP_IMAGE);
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
os_noffset = fit_image_load(images, FIT_KERNEL_PROP,
@@ -1114,6 +1122,7 @@ static int image_info(ulong addr)
printf("\n## Checking Image at %08lx ...\n", addr);
switch (genimg_get_format(hdr)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
puts(" Legacy image found\n");
if (!image_check_magic(hdr)) {
@@ -1135,6 +1144,7 @@ static int image_info(ulong addr)
}
puts("OK\n");
return 0;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
puts(" FIT image found\n");
@@ -1194,6 +1204,7 @@ static int do_imls_nor(void)
goto next_sector;
switch (genimg_get_format(hdr)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
if (!image_check_hcrc(hdr))
goto next_sector;
@@ -1208,6 +1219,7 @@ static int do_imls_nor(void)
puts("OK\n");
}
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
if (!fit_check_format(hdr))
@@ -1342,12 +1354,14 @@ static int do_imls_nand(void)
}
switch (genimg_get_format(buffer)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
header = (const image_header_t *)buffer;
len = image_get_image_size(header);
nand_imls_legacyimage(nand, nand_dev, off, len);
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
len = fit_get_size(buffer);
diff --git a/common/cmd_disk.c b/common/cmd_disk.c
index 3e457f6..9b1ba7f 100644
--- a/common/cmd_disk.c
+++ b/common/cmd_disk.c
@@ -17,7 +17,9 @@ int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
ulong addr = CONFIG_SYS_LOAD_ADDR;
ulong cnt;
disk_partition_t info;
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
image_header_t *hdr;
+#endif
block_dev_desc_t *dev_desc;
#if defined(CONFIG_FIT)
@@ -62,6 +64,7 @@ int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
bootstage_mark(BOOTSTAGE_ID_IDE_PART_READ);
switch (genimg_get_format((void *) addr)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
hdr = (image_header_t *) addr;
@@ -78,6 +81,7 @@ int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
cnt = image_get_image_size(hdr);
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
fit_hdr = (const void *) addr;
diff --git a/common/cmd_fdc.c b/common/cmd_fdc.c
index 1cfb656..409a216 100644
--- a/common/cmd_fdc.c
+++ b/common/cmd_fdc.c
@@ -635,7 +635,9 @@ int do_fdcboot (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
FD_GEO_STRUCT *pFG = (FD_GEO_STRUCT *)floppy_type;
FDC_COMMAND_STRUCT *pCMD = &cmd;
unsigned long addr,imsize;
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
image_header_t *hdr; /* used for fdc boot */
+#endif
unsigned char boot_drive;
int i,nrofblk;
#if defined(CONFIG_FIT)
@@ -689,12 +691,14 @@ int do_fdcboot (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
}
switch (genimg_get_format ((void *)addr)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
hdr = (image_header_t *)addr;
image_print_contents (hdr);
imsize = image_get_image_size (hdr);
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
fit_hdr = (const void *)addr;
diff --git a/common/cmd_fpga.c b/common/cmd_fpga.c
index 010cd24..34eb3f0 100644
--- a/common/cmd_fpga.c
+++ b/common/cmd_fpga.c
@@ -155,6 +155,7 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
case FPGA_LOADMK:
switch (genimg_get_format(fpga_data)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
{
image_header_t *hdr =
@@ -182,6 +183,7 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
rc = fpga_load(dev, (void *)data, data_size);
}
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
{
diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 04ab0f1..67d0796 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -904,7 +904,9 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
int r;
char *s;
size_t cnt;
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
image_header_t *hdr;
+#endif
#if defined(CONFIG_FIT)
const void *fit_hdr = NULL;
#endif
@@ -930,6 +932,7 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
bootstage_mark(BOOTSTAGE_ID_NAND_HDR_READ);
switch (genimg_get_format ((void *)addr)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
hdr = (image_header_t *)addr;
@@ -938,6 +941,7 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
cnt = image_get_image_size (hdr);
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
fit_hdr = (const void *)addr;
diff --git a/common/cmd_source.c b/common/cmd_source.c
index 54ffd16..51043a1 100644
--- a/common/cmd_source.c
+++ b/common/cmd_source.c
@@ -29,7 +29,9 @@ int
source (ulong addr, const char *fit_uname)
{
ulong len;
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
const image_header_t *hdr;
+#endif
ulong *data;
int verify;
void *buf;
@@ -44,6 +46,7 @@ source (ulong addr, const char *fit_uname)
buf = map_sysmem(addr, 0);
switch (genimg_get_format(buf)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
hdr = buf;
@@ -84,6 +87,7 @@ source (ulong addr, const char *fit_uname)
*/
while (*data++);
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
if (fit_uname == NULL) {
diff --git a/common/cmd_ximg.c b/common/cmd_ximg.c
index 65a8319..527b1bd 100644
--- a/common/cmd_ximg.c
+++ b/common/cmd_ximg.c
@@ -32,10 +32,13 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
{
ulong addr = load_addr;
ulong dest = 0;
- ulong data, len, count;
+ ulong data, len;
int verify;
int part = 0;
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
+ ulong count;
image_header_t *hdr = NULL;
+#endif
#if defined(CONFIG_FIT)
const char *uname = NULL;
const void* fit_hdr;
@@ -64,6 +67,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
}
switch (genimg_get_format((void *)addr)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
printf("## Copying part %d from legacy image "
@@ -114,6 +118,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
image_multi_getimg(hdr, part, &data, &len);
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
if (uname == NULL) {
diff --git a/common/image-fdt.c b/common/image-fdt.c
index a54a919..2a3262d 100644
--- a/common/image-fdt.c
+++ b/common/image-fdt.c
@@ -29,6 +29,7 @@ static void fdt_error(const char *msg)
puts(" - must RESET the board to recover.\n");
}
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
static const image_header_t *image_get_fdt(ulong fdt_addr)
{
const image_header_t *fdt_hdr = map_sysmem(fdt_addr, 0);
@@ -61,6 +62,7 @@ static const image_header_t *image_get_fdt(ulong fdt_addr)
}
return fdt_hdr;
}
+#endif
/**
* boot_fdt_add_mem_rsv_regions - Mark the memreserve sections as unusable
@@ -220,11 +222,13 @@ error:
int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
bootm_headers_t *images, char **of_flat_tree, ulong *of_size)
{
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
const image_header_t *fdt_hdr;
+ ulong load, load_end;
+ ulong image_start, image_data, image_end;
+#endif
ulong fdt_addr;
char *fdt_blob = NULL;
- ulong image_start, image_data, image_end;
- ulong load, load_end;
void *buf;
#if defined(CONFIG_FIT)
const char *fit_uname_config = images->fit_uname_cfg;
@@ -298,6 +302,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
*/
buf = map_sysmem(fdt_addr, 0);
switch (genimg_get_format(buf)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
/* verify fdt_addr points to a valid image header */
printf("## Flattened Device Tree from Legacy Image at %08lx\n",
@@ -337,6 +342,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
fdt_addr = load;
break;
+#endif
case IMAGE_FORMAT_FIT:
/*
* This case will catch both: new uImage format
diff --git a/common/image.c b/common/image.c
index 9c6bec5..ae7105e 100644
--- a/common/image.c
+++ b/common/image.c
@@ -44,8 +44,10 @@ extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
DECLARE_GLOBAL_DATA_PTR;
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch,
int verify);
+#endif
#else
#include "mkimage.h"
#include <u-boot/md5.h>
@@ -328,6 +330,7 @@ void image_print_contents(const void *ptr)
#ifndef USE_HOSTCC
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
/**
* image_get_ramdisk - get and verify ramdisk image
* @rd_addr: ramdisk image start address
@@ -389,6 +392,7 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch,
return rd_hdr;
}
+#endif
#endif /* !USE_HOSTCC */
/*****************************************************************************/
@@ -652,20 +656,19 @@ int genimg_get_comp_id(const char *name)
*/
int genimg_get_format(const void *img_addr)
{
- ulong format = IMAGE_FORMAT_INVALID;
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
const image_header_t *hdr;
hdr = (const image_header_t *)img_addr;
if (image_check_magic(hdr))
- format = IMAGE_FORMAT_LEGACY;
+ return IMAGE_FORMAT_LEGACY;
+#endif
#if defined(CONFIG_FIT) || defined(CONFIG_OF_LIBFDT)
- else {
- if (fdt_check_header(img_addr) == 0)
- format = IMAGE_FORMAT_FIT;
- }
+ if (fdt_check_header(img_addr) == 0)
+ return IMAGE_FORMAT_FIT;
#endif
- return format;
+ return IMAGE_FORMAT_INVALID;
}
/**
@@ -707,12 +710,14 @@ ulong genimg_get_image(ulong img_addr)
/* get data size */
switch (genimg_get_format(buf)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
d_size = image_get_data_size(buf);
debug(" Legacy format image found at 0x%08lx, "
"size 0x%08lx\n",
ram_addr, d_size);
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
d_size = fit_get_size(buf) - h_size;
@@ -788,7 +793,9 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
{
ulong rd_addr, rd_load;
ulong rd_data, rd_len;
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
const image_header_t *rd_hdr;
+#endif
void *buf;
#ifdef CONFIG_SUPPORT_RAW_INITRD
char *end;
@@ -871,6 +878,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
*/
buf = map_sysmem(rd_addr, 0);
switch (genimg_get_format(buf)) {
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
case IMAGE_FORMAT_LEGACY:
printf("## Loading init Ramdisk from Legacy "
"Image at %08lx ...\n", rd_addr);
@@ -886,6 +894,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
rd_len = image_get_data_size(rd_hdr);
rd_load = image_get_load(rd_hdr);
break;
+#endif
#if defined(CONFIG_FIT)
case IMAGE_FORMAT_FIT:
rd_noffset = fit_image_load(images, FIT_RAMDISK_PROP,
diff --git a/doc/uImage.FIT/signature.txt b/doc/uImage.FIT/signature.txt
index 9502037..4e32aa0 100644
--- a/doc/uImage.FIT/signature.txt
+++ b/doc/uImage.FIT/signature.txt
@@ -328,6 +328,9 @@ be enabled:
CONFIG_FIT_SIGNATURE - enable signing and verfication in FITs
CONFIG_RSA - enable RSA algorithm for signing
+WARNING: When relying on signed FIT images with required signature check
+the legacy image format should be disabled by using
+CONFIG_DISABLE_IMAGE_FORMAT_LEGACY
Testing
-------
diff --git a/include/image.h b/include/image.h
index 2508d7d..c46b1e0 100644
--- a/include/image.h
+++ b/include/image.h
@@ -410,7 +410,9 @@ enum fit_load_op {
#ifndef USE_HOSTCC
/* Image format types, returned by _get_format() routine */
#define IMAGE_FORMAT_INVALID 0x00
+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
#define IMAGE_FORMAT_LEGACY 0x01 /* legacy image_header based format */
+#endif
#define IMAGE_FORMAT_FIT 0x02 /* new, libfdt based format */
int genimg_get_format(const void *img_addr);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/4] mpc8313, signed fit: disable legacy image format on ids8313 board
2014-05-08 11:05 [U-Boot] [PATCH 0/4] mpc8313: ids8313 board updates Heiko Schocher
2014-05-08 11:05 ` [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format Heiko Schocher
@ 2014-05-08 11:05 ` Heiko Schocher
2014-05-08 20:19 ` Kim Phillips
2014-05-08 11:05 ` [U-Boot] [PATCH 3/4] lib, fdt: move fdtdec_get_int() out of lib/fdtdec.c Heiko Schocher
2014-05-08 11:05 ` [U-Boot] [PATCH 4/4] mpc8313: add CONFIG_SYS_GENERIC_BOARD to ids8313 board Heiko Schocher
3 siblings, 1 reply; 18+ messages in thread
From: Heiko Schocher @ 2014-05-08 11:05 UTC (permalink / raw)
To: u-boot
Disable legacy image format with CONFIG_DISABLE_IMAGE_FORMAT_LEGACY
on the ids8313 board, as it uses signed FIT images for booting
Linux.
Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Kim Phillips <kim.phillips@freescale.com>
Cc: Michael Conrad <Michael.Conrad@ids.de>
---
include/configs/ids8313.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/configs/ids8313.h b/include/configs/ids8313.h
index 613f7e1..a6b861b 100644
--- a/include/configs/ids8313.h
+++ b/include/configs/ids8313.h
@@ -577,6 +577,7 @@
#define CONFIG_FIT
#define CONFIG_FIT_SIGNATURE
+#define CONFIG_DISABLE_IMAGE_FORMAT_LEGACY
#define CONFIG_CMD_FDT
#define CONFIG_CMD_HASH
#define CONFIG_RSA
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 3/4] lib, fdt: move fdtdec_get_int() out of lib/fdtdec.c
2014-05-08 11:05 [U-Boot] [PATCH 0/4] mpc8313: ids8313 board updates Heiko Schocher
2014-05-08 11:05 ` [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format Heiko Schocher
2014-05-08 11:05 ` [U-Boot] [PATCH 2/4] mpc8313, signed fit: disable legacy image format on ids8313 board Heiko Schocher
@ 2014-05-08 11:05 ` Heiko Schocher
2014-05-09 19:59 ` Simon Glass
2014-05-08 11:05 ` [U-Boot] [PATCH 4/4] mpc8313: add CONFIG_SYS_GENERIC_BOARD to ids8313 board Heiko Schocher
3 siblings, 1 reply; 18+ messages in thread
From: Heiko Schocher @ 2014-05-08 11:05 UTC (permalink / raw)
To: u-boot
move fdtdec_get_int() out of lib/fdtdec.c into lib/fdtdec_common.c
as this function is also used, if CONFIG_OF_CONTROL is not
used. Poped up on the ids8313 board using signed FIT images,
and activating CONFIG_SYS_GENERIC_BOARD. Without this patch
it shows on boot:
No valid FDT found - please append one to U-Boot binary, use u-boot-dtb.bin or define CONFIG_OF_EMBED. For sandbox, use -d <file.dtb>
With this patch, it boots again with CONFIG_SYS_GENERIC_BOARD
enabled.
Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@ti.com>
---
lib/Makefile | 1 +
lib/fdtdec.c | 18 ------------------
lib/fdtdec_common.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/fdtdec.c | 1 +
4 files changed, 55 insertions(+), 18 deletions(-)
create mode 100644 lib/fdtdec_common.c
diff --git a/lib/Makefile b/lib/Makefile
index 27e4f78..a3a237f 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_USB_TTY) += circbuf.o
obj-y += crc7.o
obj-y += crc8.o
obj-y += crc16.o
+obj-y += fdtdec_common.o
obj-$(CONFIG_OF_CONTROL) += fdtdec.o
obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
obj-$(CONFIG_GZIP) += gunzip.o
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 8ecb80f..397bcc2 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -646,22 +646,4 @@ int fdtdec_read_fmap_entry(const void *blob, int node, const char *name,
return 0;
}
-#else
-#include "libfdt.h"
-#include "fdt_support.h"
-
-int fdtdec_get_int(const void *blob, int node, const char *prop_name,
- int default_val)
-{
- const int *cell;
- int len;
-
- cell = fdt_getprop_w((void *)blob, node, prop_name, &len);
- if (cell && len >= sizeof(int)) {
- int val = fdt32_to_cpu(cell[0]);
-
- return val;
- }
- return default_val;
-}
#endif
diff --git a/lib/fdtdec_common.c b/lib/fdtdec_common.c
new file mode 100644
index 0000000..7123554
--- /dev/null
+++ b/lib/fdtdec_common.c
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2014
+ * Heiko Schocher, DENX Software Engineering, hs at denx.de.
+ *
+ * Based on lib/fdtdec.c:
+ * Copyright (c) 2011 The Chromium OS Authors.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#ifndef USE_HOSTCC
+#include <common.h>
+#include <libfdt.h>
+#include <fdtdec.h>
+
+s32 fdtdec_get_int(const void *blob, int node, const char *prop_name,
+ s32 default_val)
+{
+ const s32 *cell;
+ int len;
+
+ debug("%s: %s: ", __func__, prop_name);
+ cell = fdt_getprop(blob, node, prop_name, &len);
+ if (cell && len >= sizeof(s32)) {
+ s32 val = fdt32_to_cpu(cell[0]);
+
+ debug("%#x (%d)\n", val, val);
+ return val;
+ }
+ debug("(not found)\n");
+ return default_val;
+}
+
+
+#else
+#include "libfdt.h"
+#include "fdt_support.h"
+
+int fdtdec_get_int(const void *blob, int node, const char *prop_name,
+ int default_val)
+{
+ const int *cell;
+ int len;
+
+ cell = fdt_getprop_w((void *)blob, node, prop_name, &len);
+ if (cell && len >= sizeof(int)) {
+ int val = fdt32_to_cpu(cell[0]);
+
+ return val;
+ }
+ return default_val;
+}
+#endif
diff --git a/tools/fdtdec.c b/tools/fdtdec.c
index f1c2256..9987f83 100644
--- a/tools/fdtdec.c
+++ b/tools/fdtdec.c
@@ -1 +1,2 @@
+#include "../lib/fdtdec_common.c"
#include "../lib/fdtdec.c"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 4/4] mpc8313: add CONFIG_SYS_GENERIC_BOARD to ids8313 board
2014-05-08 11:05 [U-Boot] [PATCH 0/4] mpc8313: ids8313 board updates Heiko Schocher
` (2 preceding siblings ...)
2014-05-08 11:05 ` [U-Boot] [PATCH 3/4] lib, fdt: move fdtdec_get_int() out of lib/fdtdec.c Heiko Schocher
@ 2014-05-08 11:05 ` Heiko Schocher
2014-05-08 20:19 ` Kim Phillips
3 siblings, 1 reply; 18+ messages in thread
From: Heiko Schocher @ 2014-05-08 11:05 UTC (permalink / raw)
To: u-boot
- add CONFIG_SYS_GENERIC_BOARD
- remove CONFIG_OF_CONTROL to boot again
Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Kim Phillips <kim.phillips@freescale.com>
---
include/configs/ids8313.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/configs/ids8313.h b/include/configs/ids8313.h
index a6b861b..a2d95af 100644
--- a/include/configs/ids8313.h
+++ b/include/configs/ids8313.h
@@ -19,6 +19,8 @@
#define CONFIG_MPC8313
#define CONFIG_IDS8313
+#define CONFIG_SYS_GENERIC_BOARD
+
#define CONFIG_FSL_ELBC
#define CONFIG_MISC_INIT_R
@@ -583,6 +585,5 @@
#define CONFIG_RSA
#define CONFIG_SHA1
#define CONFIG_SHA256
-#define CONFIG_OF_CONTROL
#endif /* __CONFIG_H */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format
2014-05-08 11:05 ` [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format Heiko Schocher
@ 2014-05-08 13:02 ` mike
2014-05-09 4:29 ` Wolfgang Denk
2014-05-09 5:12 ` Heiko Schocher
0 siblings, 2 replies; 18+ messages in thread
From: mike @ 2014-05-08 13:02 UTC (permalink / raw)
To: u-boot
Hi Heiko,
Did you see my last email? The one that bounced with a mime header and
where I attached a patch file.
I just wonder if its not better to switch the define to be
if (CONFIG_SIGNATURE_VERIFICATION_WITH_LEGACY_SIDE_DOOR). It can become
mutually exclusive with the existing signature verification define.
That way the legacy stuff is removed automatically upon requesting
verification unless defined otherwise. When you fail to boot an unsigned
legacy kernel then its kind of obvious that you have to solve something
but if you implement verified boot and forget this new variable then you
leave a security hole.
In my last email I also discussed my confusion regard the 'required'
variable. Similar argument to the above plus some other thoughts.
Thanks,
Mike.
On 05/08/2014 01:05 PM, Heiko Schocher wrote:
> Disabling legacy image format is useful when using
> signed FIT images with required signature check.
> Use CONFIG_DISABLE_IMAGE_FORMAT_LEGACY in the board
> config file.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Lars Steubesand <lars.steubesand@philips.com>
> Cc: Mike Pearce <mike@kaew.be>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
> README | 5 +++++
> common/cmd_bootm.c | 14 ++++++++++++++
> common/cmd_disk.c | 4 ++++
> common/cmd_fdc.c | 4 ++++
> common/cmd_fpga.c | 2 ++
> common/cmd_nand.c | 4 ++++
> common/cmd_source.c | 4 ++++
> common/cmd_ximg.c | 7 ++++++-
> common/image-fdt.c | 10 ++++++++--
> common/image.c | 23 ++++++++++++++++-------
> doc/uImage.FIT/signature.txt | 3 +++
> include/image.h | 2 ++
> 12 files changed, 72 insertions(+), 10 deletions(-)
>
> diff --git a/README b/README
> index b973344..b1f0ea3 100644
> --- a/README
> +++ b/README
> @@ -3152,6 +3152,11 @@ FIT uImage format:
> using a hash signed and verified using RSA. See
> doc/uImage.FIT/signature.txt for more details.
>
> + CONFIG_DISABLE_IMAGE_FORMAT_LEGACY
> + WARNING: When relying on signed FIT images with required
> + signature check the legacy image format should be disabled by
> + using this define.
> +
> - Standalone program support:
> CONFIG_STANDALONE_LOAD_ADDR
>
> diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
> index c243a5b..69b86ee 100644
> --- a/common/cmd_bootm.c
> +++ b/common/cmd_bootm.c
> @@ -233,6 +233,7 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc,
>
> /* get image parameters */
> switch (genimg_get_format(os_hdr)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> images.os.type = image_get_type(os_hdr);
> images.os.comp = image_get_comp(os_hdr);
> @@ -241,6 +242,7 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc,
> images.os.end = image_get_image_end(os_hdr);
> images.os.load = image_get_load(os_hdr);
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> if (fit_image_get_type(images.fit_hdr_os,
> @@ -838,6 +840,7 @@ int bootm_maybe_autostart(cmd_tbl_t *cmdtp, const char *cmd)
> return 0;
> }
>
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> /**
> * image_get_kernel - verify legacy format kernel image
> * @img_addr: in RAM address of the legacy format image to be verified
> @@ -888,6 +891,7 @@ static image_header_t *image_get_kernel(ulong img_addr, int verify)
> }
> return hdr;
> }
> +#endif
>
> /**
> * boot_get_kernel - find kernel image
> @@ -905,7 +909,9 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
> char * const argv[], bootm_headers_t *images, ulong *os_data,
> ulong *os_len)
> {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> image_header_t *hdr;
> +#endif
> ulong img_addr;
> const void *buf;
> #if defined(CONFIG_FIT)
> @@ -943,6 +949,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
> *os_data = *os_len = 0;
> buf = map_sysmem(img_addr, 0);
> switch (genimg_get_format(buf)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> printf("## Booting kernel from Legacy Image at %08lx ...\n",
> img_addr);
> @@ -985,6 +992,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
> images->legacy_hdr_valid = 1;
> bootstage_mark(BOOTSTAGE_ID_DECOMP_IMAGE);
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> os_noffset = fit_image_load(images, FIT_KERNEL_PROP,
> @@ -1114,6 +1122,7 @@ static int image_info(ulong addr)
> printf("\n## Checking Image at %08lx ...\n", addr);
>
> switch (genimg_get_format(hdr)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> puts(" Legacy image found\n");
> if (!image_check_magic(hdr)) {
> @@ -1135,6 +1144,7 @@ static int image_info(ulong addr)
> }
> puts("OK\n");
> return 0;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> puts(" FIT image found\n");
> @@ -1194,6 +1204,7 @@ static int do_imls_nor(void)
> goto next_sector;
>
> switch (genimg_get_format(hdr)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> if (!image_check_hcrc(hdr))
> goto next_sector;
> @@ -1208,6 +1219,7 @@ static int do_imls_nor(void)
> puts("OK\n");
> }
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> if (!fit_check_format(hdr))
> @@ -1342,12 +1354,14 @@ static int do_imls_nand(void)
> }
>
> switch (genimg_get_format(buffer)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> header = (const image_header_t *)buffer;
>
> len = image_get_image_size(header);
> nand_imls_legacyimage(nand, nand_dev, off, len);
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> len = fit_get_size(buffer);
> diff --git a/common/cmd_disk.c b/common/cmd_disk.c
> index 3e457f6..9b1ba7f 100644
> --- a/common/cmd_disk.c
> +++ b/common/cmd_disk.c
> @@ -17,7 +17,9 @@ int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
> ulong addr = CONFIG_SYS_LOAD_ADDR;
> ulong cnt;
> disk_partition_t info;
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> image_header_t *hdr;
> +#endif
> block_dev_desc_t *dev_desc;
>
> #if defined(CONFIG_FIT)
> @@ -62,6 +64,7 @@ int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
> bootstage_mark(BOOTSTAGE_ID_IDE_PART_READ);
>
> switch (genimg_get_format((void *) addr)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> hdr = (image_header_t *) addr;
>
> @@ -78,6 +81,7 @@ int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc,
>
> cnt = image_get_image_size(hdr);
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> fit_hdr = (const void *) addr;
> diff --git a/common/cmd_fdc.c b/common/cmd_fdc.c
> index 1cfb656..409a216 100644
> --- a/common/cmd_fdc.c
> +++ b/common/cmd_fdc.c
> @@ -635,7 +635,9 @@ int do_fdcboot (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> FD_GEO_STRUCT *pFG = (FD_GEO_STRUCT *)floppy_type;
> FDC_COMMAND_STRUCT *pCMD = &cmd;
> unsigned long addr,imsize;
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> image_header_t *hdr; /* used for fdc boot */
> +#endif
> unsigned char boot_drive;
> int i,nrofblk;
> #if defined(CONFIG_FIT)
> @@ -689,12 +691,14 @@ int do_fdcboot (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> }
>
> switch (genimg_get_format ((void *)addr)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> hdr = (image_header_t *)addr;
> image_print_contents (hdr);
>
> imsize = image_get_image_size (hdr);
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> fit_hdr = (const void *)addr;
> diff --git a/common/cmd_fpga.c b/common/cmd_fpga.c
> index 010cd24..34eb3f0 100644
> --- a/common/cmd_fpga.c
> +++ b/common/cmd_fpga.c
> @@ -155,6 +155,7 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>
> case FPGA_LOADMK:
> switch (genimg_get_format(fpga_data)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> {
> image_header_t *hdr =
> @@ -182,6 +183,7 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
> rc = fpga_load(dev, (void *)data, data_size);
> }
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> {
> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
> index 04ab0f1..67d0796 100644
> --- a/common/cmd_nand.c
> +++ b/common/cmd_nand.c
> @@ -904,7 +904,9 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
> int r;
> char *s;
> size_t cnt;
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> image_header_t *hdr;
> +#endif
> #if defined(CONFIG_FIT)
> const void *fit_hdr = NULL;
> #endif
> @@ -930,6 +932,7 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
> bootstage_mark(BOOTSTAGE_ID_NAND_HDR_READ);
>
> switch (genimg_get_format ((void *)addr)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> hdr = (image_header_t *)addr;
>
> @@ -938,6 +941,7 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
>
> cnt = image_get_image_size (hdr);
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> fit_hdr = (const void *)addr;
> diff --git a/common/cmd_source.c b/common/cmd_source.c
> index 54ffd16..51043a1 100644
> --- a/common/cmd_source.c
> +++ b/common/cmd_source.c
> @@ -29,7 +29,9 @@ int
> source (ulong addr, const char *fit_uname)
> {
> ulong len;
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> const image_header_t *hdr;
> +#endif
> ulong *data;
> int verify;
> void *buf;
> @@ -44,6 +46,7 @@ source (ulong addr, const char *fit_uname)
>
> buf = map_sysmem(addr, 0);
> switch (genimg_get_format(buf)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> hdr = buf;
>
> @@ -84,6 +87,7 @@ source (ulong addr, const char *fit_uname)
> */
> while (*data++);
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> if (fit_uname == NULL) {
> diff --git a/common/cmd_ximg.c b/common/cmd_ximg.c
> index 65a8319..527b1bd 100644
> --- a/common/cmd_ximg.c
> +++ b/common/cmd_ximg.c
> @@ -32,10 +32,13 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
> {
> ulong addr = load_addr;
> ulong dest = 0;
> - ulong data, len, count;
> + ulong data, len;
> int verify;
> int part = 0;
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> + ulong count;
> image_header_t *hdr = NULL;
> +#endif
> #if defined(CONFIG_FIT)
> const char *uname = NULL;
> const void* fit_hdr;
> @@ -64,6 +67,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
> }
>
> switch (genimg_get_format((void *)addr)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
>
> printf("## Copying part %d from legacy image "
> @@ -114,6 +118,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>
> image_multi_getimg(hdr, part, &data, &len);
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> if (uname == NULL) {
> diff --git a/common/image-fdt.c b/common/image-fdt.c
> index a54a919..2a3262d 100644
> --- a/common/image-fdt.c
> +++ b/common/image-fdt.c
> @@ -29,6 +29,7 @@ static void fdt_error(const char *msg)
> puts(" - must RESET the board to recover.\n");
> }
>
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> static const image_header_t *image_get_fdt(ulong fdt_addr)
> {
> const image_header_t *fdt_hdr = map_sysmem(fdt_addr, 0);
> @@ -61,6 +62,7 @@ static const image_header_t *image_get_fdt(ulong fdt_addr)
> }
> return fdt_hdr;
> }
> +#endif
>
> /**
> * boot_fdt_add_mem_rsv_regions - Mark the memreserve sections as unusable
> @@ -220,11 +222,13 @@ error:
> int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
> bootm_headers_t *images, char **of_flat_tree, ulong *of_size)
> {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> const image_header_t *fdt_hdr;
> + ulong load, load_end;
> + ulong image_start, image_data, image_end;
> +#endif
> ulong fdt_addr;
> char *fdt_blob = NULL;
> - ulong image_start, image_data, image_end;
> - ulong load, load_end;
> void *buf;
> #if defined(CONFIG_FIT)
> const char *fit_uname_config = images->fit_uname_cfg;
> @@ -298,6 +302,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
> */
> buf = map_sysmem(fdt_addr, 0);
> switch (genimg_get_format(buf)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> /* verify fdt_addr points to a valid image header */
> printf("## Flattened Device Tree from Legacy Image at %08lx\n",
> @@ -337,6 +342,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
>
> fdt_addr = load;
> break;
> +#endif
> case IMAGE_FORMAT_FIT:
> /*
> * This case will catch both: new uImage format
> diff --git a/common/image.c b/common/image.c
> index 9c6bec5..ae7105e 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -44,8 +44,10 @@ extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>
> DECLARE_GLOBAL_DATA_PTR;
>
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch,
> int verify);
> +#endif
> #else
> #include "mkimage.h"
> #include <u-boot/md5.h>
> @@ -328,6 +330,7 @@ void image_print_contents(const void *ptr)
>
>
> #ifndef USE_HOSTCC
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> /**
> * image_get_ramdisk - get and verify ramdisk image
> * @rd_addr: ramdisk image start address
> @@ -389,6 +392,7 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch,
>
> return rd_hdr;
> }
> +#endif
> #endif /* !USE_HOSTCC */
>
> /*****************************************************************************/
> @@ -652,20 +656,19 @@ int genimg_get_comp_id(const char *name)
> */
> int genimg_get_format(const void *img_addr)
> {
> - ulong format = IMAGE_FORMAT_INVALID;
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> const image_header_t *hdr;
>
> hdr = (const image_header_t *)img_addr;
> if (image_check_magic(hdr))
> - format = IMAGE_FORMAT_LEGACY;
> + return IMAGE_FORMAT_LEGACY;
> +#endif
> #if defined(CONFIG_FIT) || defined(CONFIG_OF_LIBFDT)
> - else {
> - if (fdt_check_header(img_addr) == 0)
> - format = IMAGE_FORMAT_FIT;
> - }
> + if (fdt_check_header(img_addr) == 0)
> + return IMAGE_FORMAT_FIT;
> #endif
>
> - return format;
> + return IMAGE_FORMAT_INVALID;
> }
>
> /**
> @@ -707,12 +710,14 @@ ulong genimg_get_image(ulong img_addr)
>
> /* get data size */
> switch (genimg_get_format(buf)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> d_size = image_get_data_size(buf);
> debug(" Legacy format image found at 0x%08lx, "
> "size 0x%08lx\n",
> ram_addr, d_size);
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> d_size = fit_get_size(buf) - h_size;
> @@ -788,7 +793,9 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
> {
> ulong rd_addr, rd_load;
> ulong rd_data, rd_len;
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> const image_header_t *rd_hdr;
> +#endif
> void *buf;
> #ifdef CONFIG_SUPPORT_RAW_INITRD
> char *end;
> @@ -871,6 +878,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
> */
> buf = map_sysmem(rd_addr, 0);
> switch (genimg_get_format(buf)) {
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> case IMAGE_FORMAT_LEGACY:
> printf("## Loading init Ramdisk from Legacy "
> "Image at %08lx ...\n", rd_addr);
> @@ -886,6 +894,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
> rd_len = image_get_data_size(rd_hdr);
> rd_load = image_get_load(rd_hdr);
> break;
> +#endif
> #if defined(CONFIG_FIT)
> case IMAGE_FORMAT_FIT:
> rd_noffset = fit_image_load(images, FIT_RAMDISK_PROP,
> diff --git a/doc/uImage.FIT/signature.txt b/doc/uImage.FIT/signature.txt
> index 9502037..4e32aa0 100644
> --- a/doc/uImage.FIT/signature.txt
> +++ b/doc/uImage.FIT/signature.txt
> @@ -328,6 +328,9 @@ be enabled:
> CONFIG_FIT_SIGNATURE - enable signing and verfication in FITs
> CONFIG_RSA - enable RSA algorithm for signing
>
> +WARNING: When relying on signed FIT images with required signature check
> +the legacy image format should be disabled by using
> +CONFIG_DISABLE_IMAGE_FORMAT_LEGACY
>
> Testing
> -------
> diff --git a/include/image.h b/include/image.h
> index 2508d7d..c46b1e0 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -410,7 +410,9 @@ enum fit_load_op {
> #ifndef USE_HOSTCC
> /* Image format types, returned by _get_format() routine */
> #define IMAGE_FORMAT_INVALID 0x00
> +#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)
> #define IMAGE_FORMAT_LEGACY 0x01 /* legacy image_header based format */
> +#endif
> #define IMAGE_FORMAT_FIT 0x02 /* new, libfdt based format */
>
> int genimg_get_format(const void *img_addr);
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/4] mpc8313, signed fit: disable legacy image format on ids8313 board
2014-05-08 11:05 ` [U-Boot] [PATCH 2/4] mpc8313, signed fit: disable legacy image format on ids8313 board Heiko Schocher
@ 2014-05-08 20:19 ` Kim Phillips
0 siblings, 0 replies; 18+ messages in thread
From: Kim Phillips @ 2014-05-08 20:19 UTC (permalink / raw)
To: u-boot
On Thu, 8 May 2014 13:05:16 +0200
Heiko Schocher <hs@denx.de> wrote:
> Disable legacy image format with CONFIG_DISABLE_IMAGE_FORMAT_LEGACY
> on the ids8313 board, as it uses signed FIT images for booting
> Linux.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Kim Phillips <kim.phillips@freescale.com>
> Cc: Michael Conrad <Michael.Conrad@ids.de>
> ---
on behalf of mpc83xx:
Acked-by: Kim Phillips <kim.phillips@freescale.com>
Kim
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 4/4] mpc8313: add CONFIG_SYS_GENERIC_BOARD to ids8313 board
2014-05-08 11:05 ` [U-Boot] [PATCH 4/4] mpc8313: add CONFIG_SYS_GENERIC_BOARD to ids8313 board Heiko Schocher
@ 2014-05-08 20:19 ` Kim Phillips
0 siblings, 0 replies; 18+ messages in thread
From: Kim Phillips @ 2014-05-08 20:19 UTC (permalink / raw)
To: u-boot
On Thu, 8 May 2014 13:05:18 +0200
Heiko Schocher <hs@denx.de> wrote:
> - add CONFIG_SYS_GENERIC_BOARD
> - remove CONFIG_OF_CONTROL to boot again
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Kim Phillips <kim.phillips@freescale.com>
> ---
on behalf of mpc83xx:
Acked-by: Kim Phillips <kim.phillips@freescale.com>
Kim
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format
2014-05-08 13:02 ` mike
@ 2014-05-09 4:29 ` Wolfgang Denk
2014-05-09 5:12 ` Heiko Schocher
1 sibling, 0 replies; 18+ messages in thread
From: Wolfgang Denk @ 2014-05-09 4:29 UTC (permalink / raw)
To: u-boot
Dear mike,
In message <536B8062.6030209@kaew.be> you wrote:
> Hi Heiko,
>
> Did you see my last email? The one that bounced with a mime header and
> where I attached a patch file.
>
> I just wonder if its not better to switch the define to be
>
> if (CONFIG_SIGNATURE_VERIFICATION_WITH_LEGACY_SIDE_DOOR). It can become
> mutually exclusive with the existing signature verification define.
Can we please come up with somewhat longer macro names? :-(
Please try and find some more reasonable (short!) names!
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Intel's new motto: United we stand. Divided we fall!
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format
2014-05-08 13:02 ` mike
2014-05-09 4:29 ` Wolfgang Denk
@ 2014-05-09 5:12 ` Heiko Schocher
2014-05-09 13:13 ` Simon Glass
1 sibling, 1 reply; 18+ messages in thread
From: Heiko Schocher @ 2014-05-09 5:12 UTC (permalink / raw)
To: u-boot
Hello Mike,
Am 08.05.2014 15:02, schrieb mike:
> Hi Heiko,
>
> Did you see my last email? The one that bounced with a mime header and where I attached a patch file.
Seems I missed this EMail ...
> I just wonder if its not better to switch the define to be
>
> if (CONFIG_SIGNATURE_VERIFICATION_WITH_LEGACY_SIDE_DOOR). It can become mutually exclusive with the existing signature verification define.
The define length seems a little long, but this is also an option.
I just prepared my patch after Simons comment, see:
http://lists.denx.de/pipermail/u-boot/2014-May/179139.html
> That way the legacy stuff is removed automatically upon requesting verification unless defined otherwise. When you fail to boot an unsigned legacy kernel then its kind of obvious that you have to solve something but if you implement verified boot and
> forget this new variable then you leave a security hole.
>
> In my last email I also discussed my confusion regard the 'required' variable. Similar argument to the above plus some other thoughts.
Was this EMail on the U-Boot ML? I could not find it...
Can you send a link?
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format
2014-05-09 5:12 ` Heiko Schocher
@ 2014-05-09 13:13 ` Simon Glass
2014-05-09 13:35 ` Wolfgang Denk
0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2014-05-09 13:13 UTC (permalink / raw)
To: u-boot
Hi,
On 8 May 2014 23:12, Heiko Schocher <hs@denx.de> wrote:
> Hello Mike,
>
> Am 08.05.2014 15:02, schrieb mike:
>
>> Hi Heiko,
>>
>> Did you see my last email? The one that bounced with a mime header and
>> where I attached a patch file.
>
>
> Seems I missed this EMail ...
>
>
>> I just wonder if its not better to switch the define to be
>>
>> if (CONFIG_SIGNATURE_VERIFICATION_WITH_LEGACY_SIDE_DOOR). It can become
>> mutually exclusive with the existing signature verification define.
>
>
> The define length seems a little long, but this is also an option.
> I just prepared my patch after Simons comment, see:
>
> http://lists.denx.de/pipermail/u-boot/2014-May/179139.html
I agree that it might be dangerous to allow legacy boot when signature
verification is used. It would be nice to fix that.
Ideally we could have something like CONFIG_IMAGE_LEGACY to enable the
legacy feature. We would need to enable it for most boards though.
Also it breaks the rule of changing the behaviour of existing
features. CONFIG_IMAGE_NO_LEGACY?
A rather more convoluted approach might be something awful like this
in config_defaults:
#ifdef CONFIG_FIT_SIGNATURE_VERIFICATION
# ifdef CONFIG_IMAGE_LEGACY_SIDE_DOOR
# define CONFIG_IMAGE_LEGACY
# endif
#else
#define CONFIG_IMAGE_LEGACY
#endif
This means that legacy is on by default, unless signature verification
is enabled, in which case the default flips. But I worry that it might
only confuse people. This seems like a Wolfgang / Tom question :-)
>
>
>> That way the legacy stuff is removed automatically upon requesting
>> verification unless defined otherwise. When you fail to boot an unsigned
>> legacy kernel then its kind of obvious that you have to solve something but
>> if you implement verified boot and
>> forget this new variable then you leave a security hole.
>>
>> In my last email I also discussed my confusion regard the 'required'
>> variable. Similar argument to the above plus some other thoughts.
>
>
> Was this EMail on the U-Boot ML? I could not find it...
> Can you send a link?
>
> bye,
> Heiko
Regards,
SImon
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format
2014-05-09 13:13 ` Simon Glass
@ 2014-05-09 13:35 ` Wolfgang Denk
2014-05-09 18:47 ` Simon Glass
0 siblings, 1 reply; 18+ messages in thread
From: Wolfgang Denk @ 2014-05-09 13:35 UTC (permalink / raw)
To: u-boot
Dear Simon,
In message <CAPnjgZ1_Cf-eu592YqF0=th7MT1da6Gh7Pv1Lxaf79kV8Lw9OQ@mail.gmail.com> you wrote:
>
> I agree that it might be dangerous to allow legacy boot when signature
> verification is used. It would be nice to fix that.
I think there is general agreement on this point.
> This means that legacy is on by default, unless signature verification
> is enabled, in which case the default flips. But I worry that it might
> only confuse people. This seems like a Wolfgang / Tom question :-)
OK, here is my 0.02? to it:
I think, no matter how we implement it, this should exactly the
behaviour. Average users tend to avoid reading documentation, so if
they enable signature verification the most likely want a secure
system, so we should give them just that. Only if someone really
knows what he is doing he should be able to enable support for
(insecure) legacy images.
As for the implementation - yes, the
#ifdef CONFIG_FIT_SIGNATURE_VERIFICATION
approach indeed does not look very nice, but then, it appears to be
the straightforward implementation of what we want to do?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Time is an illusion perpetrated by the manufacturers of space.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format
2014-05-09 13:35 ` Wolfgang Denk
@ 2014-05-09 18:47 ` Simon Glass
2014-05-09 19:12 ` Tom Rini
0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2014-05-09 18:47 UTC (permalink / raw)
To: u-boot
Hi Wolfgang,
On 9 May 2014 07:35, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon,
>
> In message <CAPnjgZ1_Cf-eu592YqF0=th7MT1da6Gh7Pv1Lxaf79kV8Lw9OQ@mail.gmail.com> you wrote:
>>
>> I agree that it might be dangerous to allow legacy boot when signature
>> verification is used. It would be nice to fix that.
>
> I think there is general agreement on this point.
>
>> This means that legacy is on by default, unless signature verification
>> is enabled, in which case the default flips. But I worry that it might
>> only confuse people. This seems like a Wolfgang / Tom question :-)
>
> OK, here is my 0.02? to it:
>
> I think, no matter how we implement it, this should exactly the
> behaviour. Average users tend to avoid reading documentation, so if
> they enable signature verification the most likely want a secure
> system, so we should give them just that. Only if someone really
> knows what he is doing he should be able to enable support for
> (insecure) legacy images.
>
> As for the implementation - yes, the
> #ifdef CONFIG_FIT_SIGNATURE_VERIFICATION
> approach indeed does not look very nice, but then, it appears to be
> the straightforward implementation of what we want to do?
OK, well in that case, let's do it that way.
Regards,
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format
2014-05-09 18:47 ` Simon Glass
@ 2014-05-09 19:12 ` Tom Rini
2014-05-12 7:36 ` Heiko Schocher
0 siblings, 1 reply; 18+ messages in thread
From: Tom Rini @ 2014-05-09 19:12 UTC (permalink / raw)
To: u-boot
On Fri, May 09, 2014 at 12:47:44PM -0600, Simon Glass wrote:
> Hi Wolfgang,
>
> On 9 May 2014 07:35, Wolfgang Denk <wd@denx.de> wrote:
> > Dear Simon,
> >
> > In message <CAPnjgZ1_Cf-eu592YqF0=th7MT1da6Gh7Pv1Lxaf79kV8Lw9OQ@mail.gmail.com> you wrote:
> >>
> >> I agree that it might be dangerous to allow legacy boot when signature
> >> verification is used. It would be nice to fix that.
> >
> > I think there is general agreement on this point.
> >
> >> This means that legacy is on by default, unless signature verification
> >> is enabled, in which case the default flips. But I worry that it might
> >> only confuse people. This seems like a Wolfgang / Tom question :-)
> >
> > OK, here is my 0.02? to it:
> >
> > I think, no matter how we implement it, this should exactly the
> > behaviour. Average users tend to avoid reading documentation, so if
> > they enable signature verification the most likely want a secure
> > system, so we should give them just that. Only if someone really
> > knows what he is doing he should be able to enable support for
> > (insecure) legacy images.
> >
> > As for the implementation - yes, the
> > #ifdef CONFIG_FIT_SIGNATURE_VERIFICATION
> > approach indeed does not look very nice, but then, it appears to be
> > the straightforward implementation of what we want to do?
>
> OK, well in that case, let's do it that way.
Agreed, then we can look for clever ways to refactor the code after.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140509/86af0d67/attachment.pgp>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 3/4] lib, fdt: move fdtdec_get_int() out of lib/fdtdec.c
2014-05-08 11:05 ` [U-Boot] [PATCH 3/4] lib, fdt: move fdtdec_get_int() out of lib/fdtdec.c Heiko Schocher
@ 2014-05-09 19:59 ` Simon Glass
2014-05-12 7:09 ` Heiko Schocher
0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2014-05-09 19:59 UTC (permalink / raw)
To: u-boot
Hi Heiko,
On 8 May 2014 05:05, Heiko Schocher <hs@denx.de> wrote:
> move fdtdec_get_int() out of lib/fdtdec.c into lib/fdtdec_common.c
> as this function is also used, if CONFIG_OF_CONTROL is not
> used. Poped up on the ids8313 board using signed FIT images,
> and activating CONFIG_SYS_GENERIC_BOARD. Without this patch
> it shows on boot:
>
> No valid FDT found - please append one to U-Boot binary, use u-boot-dtb.bin or define CONFIG_OF_EMBED. For sandbox, use -d <file.dtb>
>
> With this patch, it boots again with CONFIG_SYS_GENERIC_BOARD
> enabled.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@ti.com>
What is the reason why we can't have a common function? Is it because
of the s32 type?
Regards,
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 3/4] lib, fdt: move fdtdec_get_int() out of lib/fdtdec.c
2014-05-09 19:59 ` Simon Glass
@ 2014-05-12 7:09 ` Heiko Schocher
0 siblings, 0 replies; 18+ messages in thread
From: Heiko Schocher @ 2014-05-12 7:09 UTC (permalink / raw)
To: u-boot
Hello Simon,
Am 09.05.2014 21:59, schrieb Simon Glass:
> Hi Heiko,
>
> On 8 May 2014 05:05, Heiko Schocher<hs@denx.de> wrote:
>> move fdtdec_get_int() out of lib/fdtdec.c into lib/fdtdec_common.c
>> as this function is also used, if CONFIG_OF_CONTROL is not
>> used. Poped up on the ids8313 board using signed FIT images,
>> and activating CONFIG_SYS_GENERIC_BOARD. Without this patch
>> it shows on boot:
>>
>> No valid FDT found - please append one to U-Boot binary, use u-boot-dtb.bin or define CONFIG_OF_EMBED. For sandbox, use -d<file.dtb>
>>
>> With this patch, it boots again with CONFIG_SYS_GENERIC_BOARD
>> enabled.
>>
>> Signed-off-by: Heiko Schocher<hs@denx.de>
>> Cc: Simon Glass<sjg@chromium.org>
>> Cc: Tom Rini<trini@ti.com>
>
> What is the reason why we can't have a common function? Is it because
> of the s32 type?
I removed the two implementations and used now only:
int fdtdec_get_int(const void *blob, int node, const char *prop_name,
int default_val)
{
const int *cell;
int len;
cell = fdt_getprop_w((void *)blob, node, prop_name, &len);
if (cell && len >= sizeof(int)) {
int val = fdt32_to_cpu(cell[0]);
return val;
}
return default_val;
}
in lib/fdtdec_common.c. I see no compiler error/warnings for the
ids8313 board and the tools for host and target side ... so if this
is OK for you, I can send a v2.
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format
2014-05-09 19:12 ` Tom Rini
@ 2014-05-12 7:36 ` Heiko Schocher
2014-05-12 15:00 ` Tom Rini
0 siblings, 1 reply; 18+ messages in thread
From: Heiko Schocher @ 2014-05-12 7:36 UTC (permalink / raw)
To: u-boot
Hello Tom, Simon, Wolfgang, Lars,
Am 09.05.2014 21:12, schrieb Tom Rini:
> On Fri, May 09, 2014 at 12:47:44PM -0600, Simon Glass wrote:
>> Hi Wolfgang,
>>
>> On 9 May 2014 07:35, Wolfgang Denk<wd@denx.de> wrote:
>>> Dear Simon,
>>>
>>> In message<CAPnjgZ1_Cf-eu592YqF0=th7MT1da6Gh7Pv1Lxaf79kV8Lw9OQ@mail.gmail.com> you wrote:
>>>>
>>>> I agree that it might be dangerous to allow legacy boot when signature
>>>> verification is used. It would be nice to fix that.
>>>
>>> I think there is general agreement on this point.
>>>
>>>> This means that legacy is on by default, unless signature verification
>>>> is enabled, in which case the default flips. But I worry that it might
>>>> only confuse people. This seems like a Wolfgang / Tom question :-)
>>>
>>> OK, here is my 0.02? to it:
>>>
>>> I think, no matter how we implement it, this should exactly the
>>> behaviour. Average users tend to avoid reading documentation, so if
>>> they enable signature verification the most likely want a secure
>>> system, so we should give them just that. Only if someone really
>>> knows what he is doing he should be able to enable support for
>>> (insecure) legacy images.
>>>
>>> As for the implementation - yes, the
>>> #ifdef CONFIG_FIT_SIGNATURE_VERIFICATION
>>> approach indeed does not look very nice, but then, it appears to be
>>> the straightforward implementation of what we want to do?
>>
>> OK, well in that case, let's do it that way.
>
> Agreed, then we can look for clever ways to refactor the code after.
Ok, summary for one first step (I can do):
- introduce CONFIG_IMAGE_FORMAT_LEGACY based on patch [1]
(rename "+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)"
to "+#if defined(CONFIG_IMAGE_FORMAT_LEGACY)")
- set CONFIG_IMAGE_FORMAT_LEGACY as default:
(little bit adapted towards simons CONFIG_FIT_SIGNATURE_VERIFICATION
proposal ... I dont want to introduce a new define ...)
in config_defaults:
+#ifndef CONFIG_FIT_SIGNATURE
+#define CONFIG_IMAGE_LEGACY
+#endif
so, if boards not define CONFIG_FIT_SIGNATURE, they
have default CONFIG_IMAGE_FORMAT_LEGACY enabled (as currently).
If CONFIG_FIT_SIGNATURE is enabled, legacy image format is default
disabled (change current behaviour of boards, which use this
feature! This is only the case for:
$ grep -lr CONFIG_FIT_SIGNATURE include/
include/configs/zynq-common.h -> Michal, add Michal therefore to Cc
include/configs/sandbox.h -> Simon
include/configs/ids8313.h -> me
include/image.h
$
), but boards can enable it if needed (as ids8313 board needs
it ... yes not nice ...)
If boards which have not enabled CONFIG_FIT_SIGNATURE
and want to disable legacy image format ... we can add this
case if we want like:
in config_defaults:
+#ifndef CONFIG_FIT_SIGNATURE
+#define CONFIG_IMAGE_LEGACY
+#endif
+
+#ifdef CONFIG_DISABLE_IMAGE_LEGACY
+#undef CONFIG_IMAGE_LEGACY
+#endif
Is this a way to go?
bye,
Heiko
[1]:
[U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format
http://lists.denx.de/pipermail/u-boot/2014-May/179190.html
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format
2014-05-12 7:36 ` Heiko Schocher
@ 2014-05-12 15:00 ` Tom Rini
0 siblings, 0 replies; 18+ messages in thread
From: Tom Rini @ 2014-05-12 15:00 UTC (permalink / raw)
To: u-boot
On Mon, May 12, 2014 at 09:36:54AM +0200, Heiko Schocher wrote:
> Hello Tom, Simon, Wolfgang, Lars,
[snip]
> Ok, summary for one first step (I can do):
>
> - introduce CONFIG_IMAGE_FORMAT_LEGACY based on patch [1]
> (rename "+#if !defined(CONFIG_DISABLE_IMAGE_FORMAT_LEGACY)"
> to "+#if defined(CONFIG_IMAGE_FORMAT_LEGACY)")
>
> - set CONFIG_IMAGE_FORMAT_LEGACY as default:
> (little bit adapted towards simons CONFIG_FIT_SIGNATURE_VERIFICATION
> proposal ... I dont want to introduce a new define ...)
>
> in config_defaults:
> +#ifndef CONFIG_FIT_SIGNATURE
> +#define CONFIG_IMAGE_LEGACY
> +#endif
>
> so, if boards not define CONFIG_FIT_SIGNATURE, they
> have default CONFIG_IMAGE_FORMAT_LEGACY enabled (as currently).
>
> If CONFIG_FIT_SIGNATURE is enabled, legacy image format is default
> disabled (change current behaviour of boards, which use this
> feature! This is only the case for:
>
> $ grep -lr CONFIG_FIT_SIGNATURE include/
> include/configs/zynq-common.h -> Michal, add Michal therefore to Cc
> include/configs/sandbox.h -> Simon
> include/configs/ids8313.h -> me
> include/image.h
> $
>
> ), but boards can enable it if needed (as ids8313 board needs
> it ... yes not nice ...)
>
> If boards which have not enabled CONFIG_FIT_SIGNATURE
> and want to disable legacy image format ... we can add this
> case if we want like:
>
> in config_defaults:
> +#ifndef CONFIG_FIT_SIGNATURE
> +#define CONFIG_IMAGE_LEGACY
> +#endif
> +
> +#ifdef CONFIG_DISABLE_IMAGE_LEGACY
> +#undef CONFIG_IMAGE_LEGACY
> +#endif
>
> Is this a way to go?
Sounds right to me, thanks.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140512/9c2b61a8/attachment.pgp>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2014-05-12 15:00 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-08 11:05 [U-Boot] [PATCH 0/4] mpc8313: ids8313 board updates Heiko Schocher
2014-05-08 11:05 ` [U-Boot] [PATCH 1/4] bootm: allow to disable legacy image format Heiko Schocher
2014-05-08 13:02 ` mike
2014-05-09 4:29 ` Wolfgang Denk
2014-05-09 5:12 ` Heiko Schocher
2014-05-09 13:13 ` Simon Glass
2014-05-09 13:35 ` Wolfgang Denk
2014-05-09 18:47 ` Simon Glass
2014-05-09 19:12 ` Tom Rini
2014-05-12 7:36 ` Heiko Schocher
2014-05-12 15:00 ` Tom Rini
2014-05-08 11:05 ` [U-Boot] [PATCH 2/4] mpc8313, signed fit: disable legacy image format on ids8313 board Heiko Schocher
2014-05-08 20:19 ` Kim Phillips
2014-05-08 11:05 ` [U-Boot] [PATCH 3/4] lib, fdt: move fdtdec_get_int() out of lib/fdtdec.c Heiko Schocher
2014-05-09 19:59 ` Simon Glass
2014-05-12 7:09 ` Heiko Schocher
2014-05-08 11:05 ` [U-Boot] [PATCH 4/4] mpc8313: add CONFIG_SYS_GENERIC_BOARD to ids8313 board Heiko Schocher
2014-05-08 20:19 ` Kim Phillips
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox