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>
State: upstream (c5819701a3de61e2ba2ef7ad0b616565b32305e5)

diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
index bfc0fac3eff2b76b92eb20b3484a86995a308ae9..0561f5efd1e083bfbd33c16b2cd0a700d64f94e2 100644 (file)
--- 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/bootefi.c b/cmd/bootefi.c
index 81dd8e0284f7084cae37dac32e69e541c2094548..63abc23d92e6c9e877c56a76eacbc4152a476f52 100644 (file)
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -72,7 +72,7 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
        /* Remember only PE-COFF and FIT images */
        if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
 #ifdef CONFIG_FIT
-               if (!fit_check_format(buffer))
+               if (fit_check_format(buffer, IMAGE_SIZE_INVAL))
                        return;
                /*
                 * FIT images of type EFI_OS are started via command bootm.

diff --git a/cmd/bootm.c b/cmd/bootm.c
index e6b0e04413cd3807e6da0364e8c566857c4bb045..a0f823f968b6bd84d2084b26cc3ad0bfce3677fd 100644 (file)
--- 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 8060e753ebda7bec8d58c4c67c00e8578998cb26..3195db91277e29a7c1bda3867572b598c7acdcbe 100644 (file)
--- 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 8ae1c936fbb5bc7b1161cecbc36e4edc13869c32..51410a8e424cf3a9689e4adb3b2d920eadd3a665 100644 (file)
--- 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 92d039af8f52c6bf9ceb61bffbc58d713e52c1e8..97e117a979a0da97b43d3076bfba20746cf43a77 100644 (file)
--- 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 b6c709a3d25865f3340f8febd6656d918c3020f8..71f71528ad2a218a8cf53613522692ed584e4a28 100644 (file)
--- 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 159ba516489896d81fbde48aeeaf4d96edc43e97..ef738ebfa2f34424ac57e65f4d9ff8e41afe31e1 100644 (file)
--- 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 327a8c4c39591ba5867bd79d610e91fed3c81332..410525921282fa89f3c4550c41ee57294b703626 100644 (file)
--- 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 9637d747fb1c5608a06dd5a805b1c93e7d94cc4a..402f08fc9d910c90152e565c26d32e956c362cb2 100644 (file)
--- 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 f51ca5ddf37cd8ca3488b134097c21ca508e3dd5..bad9a7790a922677b67652dc03736296db48228c 100644 (file)
--- 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 a5879cb52c4112702c914ca83c7892abb85931cc..f0848954e5dfb0ae3083ca4238e9bc54644ca035 100644 (file)
--- 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 44e1ac54c3f1149bb31bff1c1ae07ac78e96a659..18f99676d2c74090586cad082a5e78d7e670a073 100644 (file)
--- 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 84db6be624a4e0fe9de2e90fe127cf5a712bb126..81265ee3562a1219000e93c03c56fd222977abf7 100644 (file)
--- 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 41999e176d411a08d3d369e039b7254eff845e38..eee70a2e73a2091720ac1655152f7e82fc1002d5 100644 (file)
--- 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 41473dbb9cdf6b52362c07ebe21c9d1400b97573..8c152c5c5f15779339b7a89ba14c02fcfe272b24 100644 (file)
--- 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 cdf987d3c1389004ccf98135cd1d88aa54e174ff..52b63296f8918c59f181b1d49d2371fc2b46a88b 100644 (file)
--- 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 06faeda7c26f090142ae590fd523ff5c62948456..d440d143c6781b5ae2744f9a01b3470e0aac193a 100644 (file)
--- 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 5b096a545b79e96dee9475eac5cd54eb53576dd7..0d3148444c3f878d20e98fab851cccd54cea41d9 100644 (file)
--- 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;