From d676a491fb465f11271c47185f1eb3e479c5c738 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 15 Feb 2021 17:08:09 -0700 Subject: [PATCH] image: Adjust the workings of fit_check_format() At present this function does not accept a size for the FIT. This means that it must be read from the FIT itself, introducing potential security risk. Update the function to include a size parameter, which can be invalid, in which case fit_check_format() calculates it. For now no callers pass the size, but this can be updated later. Also adjust the return value to an error code so that all the different types of problems can be distinguished by the user. Signed-off-by: Simon Glass Reported-by: Bruce Monroe Reported-by: Arie Haenel Reported-by: Julien Lenoir --- arch/arm/cpu/armv8/sec_firmware.c | 2 +- cmd/bootm.c | 6 ++-- cmd/disk.c | 2 +- cmd/fdc.c | 2 +- cmd/fpga.c | 2 +- cmd/nand.c | 2 +- cmd/source.c | 2 +- cmd/ximg.c | 2 +- common/image-fdt.c | 2 +- common/image-fit.c | 45 +++++++++++++----------------- common/splash_source.c | 4 +-- common/update.c | 2 +- drivers/net/fsl-mc/mc.c | 2 +- drivers/net/pfe_eth/pfe_firmware.c | 2 +- include/image.h | 21 +++++++++++++- tools/fit_common.c | 3 +- tools/fit_image.c | 2 +- tools/mkimage.h | 2 ++ 18 files changed, 61 insertions(+), 44 deletions(-) diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c index 8dc0ac92668f..3c5249541222 100644 --- a/arch/arm/cpu/armv8/sec_firmware.c +++ b/arch/arm/cpu/armv8/sec_firmware.c @@ -310,7 +310,7 @@ __weak bool sec_firmware_is_valid(const void *sec_firmware_img) return false; } - if (!fit_check_format(sec_firmware_img)) { + if (fit_check_format(sec_firmware_img, IMAGE_SIZE_INVAL)) { printf("SEC Firmware: Bad firmware image (bad FIT header)\n"); return false; } diff --git a/cmd/bootm.c b/cmd/bootm.c index c3a063474ac6..1d6ec0d4cacc 100644 --- a/cmd/bootm.c +++ b/cmd/bootm.c @@ -282,7 +282,7 @@ static int image_info(ulong addr) case IMAGE_FORMAT_FIT: puts(" FIT image found\n"); - if (!fit_check_format(hdr)) { + if (fit_check_format(hdr, IMAGE_SIZE_INVAL)) { puts("Bad FIT image format!\n"); return 1; } @@ -355,7 +355,7 @@ static int do_imls_nor(void) #endif #if defined(CONFIG_FIT) case IMAGE_FORMAT_FIT: - if (!fit_check_format(hdr)) + if (fit_check_format(hdr), IMAGE_SIZE_INVAL) goto next_sector; printf("FIT Image at %08lX:\n", (ulong)hdr); @@ -435,7 +435,7 @@ static int nand_imls_fitimage(struct mtd_info *mtd, int nand_dev, loff_t off, return ret; } - if (!fit_check_format(imgdata)) { + if (fit_check_format(imgdata), IMAGE_SIZE_INVAL) { free(imgdata); return 0; } diff --git a/cmd/disk.c b/cmd/disk.c index dcc36a6c2cb7..294fc111023a 100644 --- a/cmd/disk.c +++ b/cmd/disk.c @@ -110,7 +110,7 @@ int common_diskboot(cmd_tbl_t *cmdtp, const char *intf, int argc, /* This cannot be done earlier, * we need complete FIT image in RAM first */ if (genimg_get_format((void *) addr) == IMAGE_FORMAT_FIT) { - if (!fit_check_format(fit_hdr)) { + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { bootstage_error(BOOTSTAGE_ID_IDE_FIT_READ); puts("** Bad FIT image format\n"); return 1; diff --git a/cmd/fdc.c b/cmd/fdc.c index 906845d4049b..37e557a1e7d7 100644 --- a/cmd/fdc.c +++ b/cmd/fdc.c @@ -730,7 +730,7 @@ int do_fdcboot (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) #if defined(CONFIG_FIT) /* This cannot be done earlier, we need complete FIT image in RAM first */ if (genimg_get_format ((void *)addr) == IMAGE_FORMAT_FIT) { - if (!fit_check_format (fit_hdr)) { + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { puts ("** Bad FIT image format\n"); return 1; } diff --git a/cmd/fpga.c b/cmd/fpga.c index 88a8e3f3186b..9093026ff6ce 100644 --- a/cmd/fpga.c +++ b/cmd/fpga.c @@ -325,7 +325,7 @@ static int do_fpga_loadmk(cmd_tbl_t *cmdtp, int flag, int argc, return CMD_RET_FAILURE; } - if (!fit_check_format(fit_hdr)) { + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { puts("Bad FIT image format\n"); return CMD_RET_FAILURE; } diff --git a/cmd/nand.c b/cmd/nand.c index a22945d144b3..536a11be9605 100644 --- a/cmd/nand.c +++ b/cmd/nand.c @@ -911,7 +911,7 @@ static int nand_load_image(cmd_tbl_t *cmdtp, struct mtd_info *mtd, #if defined(CONFIG_FIT) /* This cannot be done earlier, we need complete FIT image in RAM first */ if (genimg_get_format ((void *)addr) == IMAGE_FORMAT_FIT) { - if (!fit_check_format (fit_hdr)) { + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { bootstage_error(BOOTSTAGE_ID_NAND_FIT_READ); puts ("** Bad FIT image format\n"); return 1; diff --git a/cmd/source.c b/cmd/source.c index 6d98a1cfd32b..897b97057d85 100644 --- a/cmd/source.c +++ b/cmd/source.c @@ -106,7 +106,7 @@ source (ulong addr, const char *fit_uname) #if defined(CONFIG_FIT) case IMAGE_FORMAT_FIT: fit_hdr = buf; - if (!fit_check_format (fit_hdr)) { + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { puts ("Bad FIT image format\n"); return 1; } diff --git a/cmd/ximg.c b/cmd/ximg.c index 8572a67a0063..51af741c827b 100644 --- a/cmd/ximg.c +++ b/cmd/ximg.c @@ -131,7 +131,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) "at %08lx ...\n", uname, addr); fit_hdr = (const void *)addr; - if (!fit_check_format(fit_hdr)) { + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { puts("Bad FIT image format\n"); return 1; } diff --git a/common/image-fdt.c b/common/image-fdt.c index 52ada56fc17b..3aa6c427362c 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -394,7 +394,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, */ #if CONFIG_IS_ENABLED(FIT) /* check FDT blob vs FIT blob */ - if (fit_check_format(buf)) { + if (!fit_check_format(buf, IMAGE_SIZE_INVAL)) { ulong load, len; fdt_noffset = boot_get_fdt_fit(images, diff --git a/common/image-fit.c b/common/image-fit.c index 6894384b47b9..124d8895cffd 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -8,6 +8,8 @@ * Wolfgang Denk, DENX Software Engineering, wd@denx.de. */ +#define LOG_CATEGORY LOGC_BOOT + #ifdef USE_HOSTCC #include "mkimage.h" #include @@ -1460,46 +1462,39 @@ int fit_image_check_comp(const void *fit, int noffset, uint8_t comp) return (comp == image_comp); } -/** - * fit_check_format - sanity check FIT image format - * @fit: pointer to the FIT format image header - * - * fit_check_format() runs a basic sanity FIT image verification. - * Routine checks for mandatory properties, nodes, etc. - * - * returns: - * 1, on success - * 0, on failure - */ -int fit_check_format(const void *fit) +int fit_check_format(const void *fit, ulong size) { + int ret; + /* A FIT image must be a valid FDT */ - if (fdt_check_header(fit)) { - debug("Wrong FIT format: not a flattened device tree\n"); - return 0; + ret = fdt_check_header(fit); + if (ret) { + log_debug("Wrong FIT format: not a flattened device tree (err=%d)\n", + ret); + return -ENOEXEC; } /* mandatory / node 'description' property */ - if (fdt_getprop(fit, 0, FIT_DESC_PROP, NULL) == NULL) { - debug("Wrong FIT format: no description\n"); - return 0; + if (!fdt_getprop(fit, 0, FIT_DESC_PROP, NULL)) { + log_debug("Wrong FIT format: no description\n"); + return -ENOMSG; } if (IMAGE_ENABLE_TIMESTAMP) { /* mandatory / node 'timestamp' property */ - if (fdt_getprop(fit, 0, FIT_TIMESTAMP_PROP, NULL) == NULL) { - debug("Wrong FIT format: no timestamp\n"); - return 0; + if (!fdt_getprop(fit, 0, FIT_TIMESTAMP_PROP, NULL)) { + log_debug("Wrong FIT format: no timestamp\n"); + return -ENODATA; } } /* mandatory subimages parent '/images' node */ if (fdt_path_offset(fit, FIT_IMAGES_PATH) < 0) { - debug("Wrong FIT format: no images parent node\n"); - return 0; + log_debug("Wrong FIT format: no images parent node\n"); + return -ENOENT; } - return 1; + return 0; } @@ -1813,7 +1808,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr, printf("## Loading %s from FIT Image at %08lx ...\n", prop_name, addr); bootstage_mark(bootstage_id + BOOTSTAGE_SUB_FORMAT); - if (!fit_check_format(fit)) { + if (fit_check_format(fit, IMAGE_SIZE_INVAL)) { printf("Bad FIT %s image format!\n", prop_name); bootstage_error(bootstage_id + BOOTSTAGE_SUB_FORMAT); return -ENOEXEC; diff --git a/common/splash_source.c b/common/splash_source.c index 62763b9ebd56..d43dd0b2cd98 100644 --- a/common/splash_source.c +++ b/common/splash_source.c @@ -329,8 +329,8 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr) if (res < 0) return res; - res = fit_check_format(fit_header); - if (!res) { + res = fit_check_format(fit_header, IMAGE_SIZE_INVAL); + if (res) { debug("Could not find valid FIT image\n"); return -EINVAL; } diff --git a/common/update.c b/common/update.c index f237ea53bb2a..42950edbbf22 100644 --- a/common/update.c +++ b/common/update.c @@ -280,7 +280,7 @@ int update_tftp(ulong addr, char *interface, char *devstring) got_update_file: fit = (void *)addr; - if (!fit_check_format((void *)fit)) { + if (fit_check_format((void *)fit, IMAGE_SIZE_INVAL)) { printf("Bad FIT format of the update file, aborting " "auto-update\n"); return 1; diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index cc59b21f9f48..c4f35e7325b2 100644 --- a/drivers/net/fsl-mc/mc.c +++ b/drivers/net/fsl-mc/mc.c @@ -130,7 +130,7 @@ int parse_mc_firmware_fit_image(u64 mc_fw_addr, return -EINVAL; } - if (!fit_check_format(fit_hdr)) { + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { printf("fsl-mc: ERR: Bad firmware image (bad FIT header)\n"); return -EINVAL; } diff --git a/drivers/net/pfe_eth/pfe_firmware.c b/drivers/net/pfe_eth/pfe_firmware.c index adb2d06010ce..7b930ecc2a02 100644 --- a/drivers/net/pfe_eth/pfe_firmware.c +++ b/drivers/net/pfe_eth/pfe_firmware.c @@ -150,7 +150,7 @@ static int pfe_fit_check(void) return ret; } - if (!fit_check_format(pfe_fit_addr)) { + if (fit_check_format(pfe_fit_addr, IMAGE_SIZE_INVAL)) { printf("PFE Firmware: Bad firmware image (bad FIT header)\n"); ret = -1; return ret; diff --git a/include/image.h b/include/image.h index ea4c05ca2586..b73f739c1585 100644 --- a/include/image.h +++ b/include/image.h @@ -453,6 +453,9 @@ extern bootm_headers_t images; #define uimage_to_cpu(x) be32_to_cpu(x) #define cpu_to_uimage(x) cpu_to_be32(x) +/* An invalid size, meaning that the image size is not known */ +#define IMAGE_SIZE_INVAL (-1UL) + /* * Translation table for entries of a specific type; used by * get_table_entry_id() and get_table_entry_name(). @@ -1062,7 +1065,23 @@ int fit_image_check_os(const void *fit, int noffset, uint8_t os); int fit_image_check_arch(const void *fit, int noffset, uint8_t arch); int fit_image_check_type(const void *fit, int noffset, uint8_t type); int fit_image_check_comp(const void *fit, int noffset, uint8_t comp); -int fit_check_format(const void *fit); + +/** + * fit_check_format() - Check that the FIT is valid + * + * This performs various checks on the FIT to make sure it is suitable for + * use, looking for mandatory properties, nodes, etc. + * + * If FIT_FULL_CHECK is enabled, it also runs it through libfdt to make + * sure that there are no strange tags or broken nodes in the FIT. + * + * @fit: pointer to the FIT format image header + * @return 0 if OK, -ENOEXEC if not an FDT file, -EINVAL if the full FDT check + * failed (e.g. due to bad structure), -ENOMSG if the description is + * missing, -ENODATA if the timestamp is missing, -ENOENT if the /images + * path is missing + */ +int fit_check_format(const void *fit, ulong size); int fit_conf_find_compat(const void *fit, const void *fdt); diff --git a/tools/fit_common.c b/tools/fit_common.c index 9506390214ce..5e85ca221ac9 100644 --- a/tools/fit_common.c +++ b/tools/fit_common.c @@ -26,7 +26,8 @@ int fit_verify_header(unsigned char *ptr, int image_size, struct image_tool_params *params) { - if (fdt_check_header(ptr) != EXIT_SUCCESS || !fit_check_format(ptr)) + if (fdt_check_header(ptr) != EXIT_SUCCESS || + fit_check_format(ptr, IMAGE_SIZE_INVAL)) return EXIT_FAILURE; return EXIT_SUCCESS; diff --git a/tools/fit_image.c b/tools/fit_image.c index 3b867e06564e..21fc11c084c9 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -764,7 +764,7 @@ static int fit_extract_contents(void *ptr, struct image_tool_params *params) /* Indent string is defined in header image.h */ p = IMAGE_INDENT_STRING; - if (!fit_check_format(fit)) { + if (fit_check_format(fit, IMAGE_SIZE_INVAL)) { printf("Bad FIT image format\n"); return -1; } diff --git a/tools/mkimage.h b/tools/mkimage.h index 0254af59fbed..d32625f0a234 100644 --- a/tools/mkimage.h +++ b/tools/mkimage.h @@ -29,6 +29,8 @@ #define debug(fmt,args...) #endif /* MKIMAGE_DEBUG */ +#define log_debug(fmt, args...) debug(fmt, ##args) + static inline void *map_sysmem(ulong paddr, unsigned long len) { return (void *)(uintptr_t)paddr; -- 2.17.1