| From c5819701a3de61e2ba2ef7ad0b616565b32305e5 Mon Sep 17 00:00:00 2001 |
| From: Simon Glass <sjg@chromium.org> |
| 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 <sjg@chromium.org> |
| Reported-by: Bruce Monroe <bruce.monroe@intel.com> |
| Reported-by: Arie Haenel <arie.haenel@intel.com> |
| Reported-by: Julien Lenoir <julien.lenoir@intel.com> |
| |
| CVE: CVE-2021-27097 CVE-2021-27138 |
| Upstream-Status: Backport[https://github.com/u-boot/u-boot/commit/c5819701a3de61e2ba2ef7ad0b616565b32305e5] |
| Signed-off-by: Scott Murray <scott.murray@konsulko.com> |
| |
| --- |
| arch/arm/cpu/armv8/sec_firmware.c | 2 +- |
| cmd/bootm.c | 6 ++--- |
| cmd/disk.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 | 46 +++++++++++++++++--------------------- |
| common/splash_source.c | 6 ++--- |
| common/update.c | 4 ++-- |
| drivers/fpga/socfpga_arria10.c | 6 ++--- |
| 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, 65 insertions(+), 49 deletions(-) |
| |
| diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c |
| index bfc0fac3ef..0561f5efd1 100644 |
| --- a/arch/arm/cpu/armv8/sec_firmware.c |
| +++ b/arch/arm/cpu/armv8/sec_firmware.c |
| @@ -316,7 +316,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 e6b0e04413..a0f823f968 100644 |
| --- a/cmd/bootm.c |
| +++ b/cmd/bootm.c |
| @@ -291,7 +291,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"); |
| unmap_sysmem(hdr); |
| return 1; |
| @@ -368,7 +368,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); |
| @@ -448,7 +448,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 8060e753eb..3195db9127 100644 |
| --- a/cmd/disk.c |
| +++ b/cmd/disk.c |
| @@ -114,7 +114,7 @@ int common_diskboot(struct cmd_tbl *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/fpga.c b/cmd/fpga.c |
| index 8ae1c936fb..51410a8e42 100644 |
| --- a/cmd/fpga.c |
| +++ b/cmd/fpga.c |
| @@ -330,7 +330,7 @@ static int do_fpga_loadmk(struct cmd_tbl *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 92d039af8f..97e117a979 100644 |
| --- a/cmd/nand.c |
| +++ b/cmd/nand.c |
| @@ -917,7 +917,7 @@ static int nand_load_image(struct cmd_tbl *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 b6c709a3d2..71f71528ad 100644 |
| --- a/cmd/source.c |
| +++ b/cmd/source.c |
| @@ -107,7 +107,7 @@ int image_source_script(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 159ba51648..ef738ebfa2 100644 |
| --- a/cmd/ximg.c |
| +++ b/cmd/ximg.c |
| @@ -136,7 +136,7 @@ do_imgextract(struct cmd_tbl *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 327a8c4c39..4105259212 100644 |
| --- a/common/image-fdt.c |
| +++ b/common/image-fdt.c |
| @@ -399,7 +399,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 9637d747fb..402f08fc9d 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 <time.h> |
| @@ -1550,49 +1552,41 @@ 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; |
| } |
| |
| - |
| /** |
| * fit_conf_find_compat |
| * @fit: pointer to the FIT format image header |
| @@ -1929,7 +1923,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 f51ca5ddf3..bad9a7790a 100644 |
| --- a/common/splash_source.c |
| +++ b/common/splash_source.c |
| @@ -336,10 +336,10 @@ 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; |
| + return res; |
| } |
| |
| /* Get the splash image node */ |
| diff --git a/common/update.c b/common/update.c |
| index a5879cb52c..f0848954e5 100644 |
| --- a/common/update.c |
| +++ b/common/update.c |
| @@ -286,7 +286,7 @@ int update_tftp(ulong addr, char *interface, char *devstring) |
| got_update_file: |
| fit = map_sysmem(addr, 0); |
| |
| - 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; |
| @@ -363,7 +363,7 @@ int fit_update(const void *fit) |
| if (!fit) |
| return -EINVAL; |
| |
| - 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 -EINVAL; |
| } |
| diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c |
| index 44e1ac54c3..18f99676d2 100644 |
| --- a/drivers/fpga/socfpga_arria10.c |
| +++ b/drivers/fpga/socfpga_arria10.c |
| @@ -565,10 +565,10 @@ static int first_loading_rbf_to_buffer(struct udevice *dev, |
| if (ret < 0) |
| return ret; |
| |
| - ret = fit_check_format(buffer_p); |
| - if (!ret) { |
| + ret = fit_check_format(buffer_p, IMAGE_SIZE_INVAL); |
| + if (ret) { |
| debug("FPGA: No valid FIT image was found.\n"); |
| - return -EBADF; |
| + return ret; |
| } |
| |
| confs_noffset = fdt_path_offset(buffer_p, FIT_CONFS_PATH); |
| diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c |
| index 84db6be624..81265ee356 100644 |
| --- a/drivers/net/fsl-mc/mc.c |
| +++ b/drivers/net/fsl-mc/mc.c |
| @@ -141,7 +141,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 41999e176d..eee70a2e73 100644 |
| --- a/drivers/net/pfe_eth/pfe_firmware.c |
| +++ b/drivers/net/pfe_eth/pfe_firmware.c |
| @@ -160,7 +160,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 41473dbb9c..8c152c5c5f 100644 |
| --- a/include/image.h |
| +++ b/include/image.h |
| @@ -134,6 +134,9 @@ extern ulong image_load_addr; /* Default Load Address */ |
| extern ulong image_save_addr; /* Default Save Address */ |
| extern ulong image_save_size; /* Default Save Size */ |
| |
| +/* An invalid size, meaning that the image size is not known */ |
| +#define IMAGE_SIZE_INVAL (-1UL) |
| + |
| enum ih_category { |
| IH_ARCH, |
| IH_COMP, |
| @@ -1141,7 +1144,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 cdf987d3c1..52b63296f8 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 06faeda7c2..d440d143c6 100644 |
| --- a/tools/fit_image.c |
| +++ b/tools/fit_image.c |
| @@ -883,7 +883,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 5b096a545b..0d3148444c 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; |