diff --git a/common/Kconfig b/common/Kconfig index f04e79b836..4a1f5cf768 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -717,3 +717,12 @@ config CONSOLE_RECORD_IN_SIZE tstc() and getc() will use this in preference to real device input. The buffer is allocated immediately after the malloc() region is ready. + +config FIT_FULL_CHECK + bool "Do a full check of the FIT before using it" + default y + help + Enable this do a full check of the FIT to make sure it is valid. This + helps to protect against carefully crafted FITs which take advantage + of bugs or omissions in the code. This includes a bad structure, + multiple root nodes and the like. diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 48738ac605..73ffa2142c 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -279,7 +279,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; } @@ -352,7 +352,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); @@ -434,7 +434,7 @@ static int nand_imls_fitimage(nand_info_t *nand, 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/common/cmd_disk.c b/common/cmd_disk.c index 9dbe550fb4..bc2851cbbc 100644 --- a/common/cmd_disk.c +++ b/common/cmd_disk.c @@ -112,7 +112,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 (fit_hdr && 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/common/cmd_fdc.c b/common/cmd_fdc.c index 5766b5650b..9384ebf0e5 100644 --- a/common/cmd_fdc.c +++ b/common/cmd_fdc.c @@ -731,7 +731,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/common/cmd_fpga.c b/common/cmd_fpga.c index 7f99aabf8a..c97dc31b1a 100644 --- a/common/cmd_fpga.c +++ b/common/cmd_fpga.c @@ -248,7 +248,7 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) return 1; } - 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/cmd_nand.c b/common/cmd_nand.c index 517badc20d..2be767e3bf 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -889,7 +889,7 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand, #if defined(CONFIG_FIT) /* This cannot be done earlier, we need complete FIT image in RAM first */ if (fit_hdr && 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/common/cmd_source.c b/common/cmd_source.c index db7ab7e5f4..9a35cad2f1 100644 --- a/common/cmd_source.c +++ b/common/cmd_source.c @@ -97,7 +97,7 @@ source (ulong addr, const char *fit_uname) } 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/common/cmd_ximg.c b/common/cmd_ximg.c index a62c0d7a51..4b6f2a12e8 100644 --- a/common/cmd_ximg.c +++ b/common/cmd_ximg.c @@ -132,7 +132,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 5e4e5bd914..a8c24bafee 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -353,7 +353,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, */ #if defined(CONFIG_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 = fit_image_load(images, diff --git a/common/image-fit.c b/common/image-fit.c index afa7468e97..b5b6f961ce 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -8,7 +8,7 @@ * * SPDX-License-Identifier: GPL-2.0+ */ - +#define LOG_CATEGORY LOGC_BOOT #ifdef USE_HOSTCC #include "mkimage.h" #include @@ -991,11 +991,22 @@ int fit_image_verify(const void *fit, int image_noffset) { const void *data; size_t size; - int noffset = 0; char *err_msg = ""; int verify_all = 1; int ret; + int noffset = 0; +#if defined(CONFIG_FIT_SIGNATURE) + const char *name = fit_get_name(fit, image_noffset, NULL); + if (strchr(name, '@')) { + /* + * We don't support this since libfdt considers names with the + * name root but different @ suffix to be equal + */ + err_msg = "Node name contains @"; + goto error; + } +#endif /* Get image data and data length */ if (fit_image_get_data(fit, image_noffset, &data, &size)) { err_msg = "Can't get image data/size"; @@ -1192,6 +1203,33 @@ int fit_image_check_comp(const void *fit, int noffset, uint8_t comp) return (comp == image_comp); } +/** + * fdt_check_no_at() - Check for nodes whose names contain '@' + * This checks the parent node and all subnodes recursively + * @fit: FIT to check + * @parent: Parent node to check + * @return 0 if OK, -EADDRNOTAVAIL is a node has a name containing '@' + */ +#if defined(CONFIG_FIT_SIGNATURE) +static int fdt_check_no_at(const void *fit, int parent) +{ + const char *name; + int node; + int ret; + + name = fdt_get_name(fit, parent, NULL); + if (!name || strchr(name, '@')) + return -EADDRNOTAVAIL; + + fdt_for_each_subnode(fit, node, parent) { + ret = fdt_check_no_at(fit, node); + if (ret) + return ret; + } + + return 0; +} +#endif /** * fit_check_format - sanity check FIT image format * @fit: pointer to the FIT format image header @@ -1203,29 +1241,71 @@ int fit_image_check_comp(const void *fit, int noffset, uint8_t comp) * 1, on success * 0, on failure */ -int fit_check_format(const void *fit) +int fit_check_format(const void *fit, ulong size) { + int ret; + + ret = fdt_check_header(fit); + if (ret) { + debug("Wrong FIT format: not a flattened device tree\n"); + return -ENOEXEC; + } + +#if defined(CONFIG_FIT_FULL_CHECK) + /* + * If we are not given the size, make do wtih calculating it. + * This is not as secure, so we should consider a flag to + * control this. + */ + if (size == IMAGE_SIZE_INVAL) + size = fdt_totalsize(fit); + ret = fdt_check_full(fit, size); + if (ret) + ret = -EINVAL; + + /* + * U-Boot stopped using unit addressed in 2017. Since libfdt + * can match nodes ignoring any unit address, signature + * verification can see the wrong node if one is inserted with + * the same name as a valid node but with a unit address + * attached. Protect against this by disallowing unit addresses. + */ +#if defined(CONFIG_FIT_SIGNATURE) + if (!ret) + ret = fdt_check_no_at(fit, 0); + + if (ret) { + debug("FIT check error %d\n", ret); + return ret; + } + } +#endif + if (ret) { + debug("FIT check error %d\n", ret); + return ret; + } +#endif /* mandatory / node 'description' property */ - if (fdt_getprop(fit, 0, FIT_DESC_PROP, NULL) == NULL) { + if (!fdt_getprop(fit, 0, FIT_DESC_PROP, NULL)) { debug("Wrong FIT format: no description\n"); - return 0; + return -ENOMSG; } if (IMAGE_ENABLE_TIMESTAMP) { /* mandatory / node 'timestamp' property */ - if (fdt_getprop(fit, 0, FIT_TIMESTAMP_PROP, NULL) == NULL) { + if (!fdt_getprop(fit, 0, FIT_TIMESTAMP_PROP, NULL)) { debug("Wrong FIT format: no timestamp\n"); - return 0; + 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; + return -ENOENT; } - return 1; + return 0; } @@ -1581,10 +1661,15 @@ 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)) { - printf("Bad FIT %s image format!\n", prop_name); + ret = fit_check_format(fit, IMAGE_SIZE_INVAL); + if (ret) { + printf("Bad FIT %s image format! (err=%d)\n", prop_name, ret); +#if defined(CONFIG_FIT_SIGNATURE) + if (ret == -EADDRNOTAVAIL) + printf("Signature checking prevents use of unit addresses (@) in nodes\n"); +#endif bootstage_error(bootstage_id + BOOTSTAGE_SUB_FORMAT); - return -ENOEXEC; + return ret; } bootstage_mark(bootstage_id + BOOTSTAGE_SUB_FORMAT_OK); if (fit_uname) { diff --git a/common/update.c b/common/update.c index 1da80b70f2..521772c364 100644 --- a/common/update.c +++ b/common/update.c @@ -279,7 +279,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/misc/fsl_debug_server.c b/drivers/misc/fsl_debug_server.c index 98d9fbe534..25713316a1 100644 --- a/drivers/misc/fsl_debug_server.c +++ b/drivers/misc/fsl_debug_server.c @@ -63,7 +63,7 @@ int debug_server_parse_firmware_fit_image(const void **raw_image_addr, goto out_error; } - if (!fit_check_format(fit_hdr)) { + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { printf("Debug Server FW: Bad FIT image format\n"); goto out_error; } diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index bac4610fd9..47c9359397 100644 --- a/drivers/net/fsl-mc/mc.c +++ b/drivers/net/fsl-mc/mc.c @@ -123,7 +123,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/include/image.h b/include/image.h index 43e873d627..558ae7cbbe 100644 --- a/include/image.h +++ b/include/image.h @@ -792,6 +792,9 @@ int bootz_setup(ulong image, ulong *start, ulong *end); #define FIT_MAX_HASH_LEN HASH_MAX_DIGEST_SIZE +/* An invalid size, meaning that the image size is not known */ +#define IMAGE_SIZE_INVAL (-1UL) + /* cmdline argument format parsing */ int fit_parse_conf(const char *spec, ulong addr_curr, ulong *addr, const char **conf_name); @@ -888,10 +891,26 @@ 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); int fit_conf_get_node(const void *fit, const char *conf_uname); +int fdt_check_full(const void *fdt, size_t bufsize); /** * fit_conf_get_prop_node() - Get node refered to by a configuration diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c index 7b0777b67e..e6ac9bc693 100644 --- a/lib/libfdt/fdt_ro.c +++ b/lib/libfdt/fdt_ro.c @@ -14,6 +14,7 @@ #include "libfdt_internal.h" +#define INT_MAX ((int)(~0U>>1)) static int _fdt_nodename_eq(const void *fdt, int offset, const char *s, int len) { @@ -625,3 +626,83 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset, return offset; /* error from fdt_next_node() */ } + +int fdt_check_full(const void *fdt, size_t bufsize) +{ + int err; + int num_memrsv; + int offset, nextoffset = 0; + uint32_t tag; + unsigned depth = 0; + const void *prop; + const char *propname; + bool expect_end = false; + + if (bufsize < FDT_V1_SIZE) + return -FDT_ERR_TRUNCATED; + err = fdt_check_header(fdt); + if (err != 0) + return err; + if (bufsize < fdt_totalsize(fdt)) + return -FDT_ERR_TRUNCATED; + + num_memrsv = fdt_num_mem_rsv(fdt); + if (num_memrsv < 0) + return num_memrsv; + + while (1) { + offset = nextoffset; + tag = fdt_next_tag(fdt, offset, &nextoffset); + + if (nextoffset < 0) + return nextoffset; + + /* If we see two root nodes, something is wrong */ + if (expect_end && tag != FDT_END) + return -FDT_ERR_BADLAYOUT; + + switch (tag) { + case FDT_NOP: + break; + + case FDT_END: + if (depth != 0) + return -FDT_ERR_BADSTRUCTURE; + return 0; + + case FDT_BEGIN_NODE: + depth++; + if (depth > INT_MAX) + return -FDT_ERR_BADSTRUCTURE; + + /* The root node must have an empty name */ + if (depth == 1) { + const char *name; + int len; + + name = fdt_get_name(fdt, offset, &len); + if (*name || len) + return -FDT_ERR_BADLAYOUT; + } + break; + + case FDT_END_NODE: + if (depth == 0) + return -FDT_ERR_BADSTRUCTURE; + depth--; + if (depth == 0) + expect_end = true; + break; + + case FDT_PROP: + prop = fdt_getprop_by_offset(fdt, offset, &propname, + &err); + if (!prop) + return err; + break; + + default: + return -FDT_ERR_INTERNAL; + } + } +} diff --git a/lib/libfdt/fdt_wip.c b/lib/libfdt/fdt_wip.c index 3f2dfa573b..6f5cd1700e 100644 --- a/lib/libfdt/fdt_wip.c +++ b/lib/libfdt/fdt_wip.c @@ -103,6 +103,7 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, int depth = -1; int want = 0; int base = fdt_off_dt_struct(fdt); + bool expect_end = false; end = path; *end = '\0'; @@ -119,6 +120,10 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, tag = fdt_next_tag(fdt, offset, &nextoffset); stop_at = nextoffset; + /* If we see two root nodes, something is wrong */ + if (expect_end && tag != FDT_END) + return -FDT_ERR_BADLAYOUT; + switch (tag) { case FDT_PROP: include = want >= 2; @@ -139,6 +144,9 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, if (depth == FDT_MAX_DEPTH) return -FDT_ERR_BADSTRUCTURE; name = fdt_get_name(fdt, offset, &len); + /* The root node must have an empty name */ + if (!depth && *name) + return -FDT_ERR_BADLAYOUT; if (end - path + 2 + len >= path_len) return -FDT_ERR_NOSPACE; if (end != path + 1) @@ -163,6 +171,8 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, while (end > path && *--end != '/') ; *end = '\0'; + if (depth == -1) + expect_end = true; break; case FDT_END: diff --git a/tools/fit_common.c b/tools/fit_common.c index 81ba698abd..6824df382a 100644 --- a/tools/fit_common.c +++ b/tools/fit_common.c @@ -27,7 +27,11 @@ int fit_verify_header(unsigned char *ptr, int image_size, struct image_tool_params *params) { - return fdt_check_header(ptr); + if (fdt_check_header(ptr) != EXIT_SUCCESS || + fit_check_format(ptr, IMAGE_SIZE_INVAL)) + return EXIT_FAILURE; + + return EXIT_SUCCESS; } int fit_check_image_types(uint8_t type) diff --git a/tools/fit_image.c b/tools/fit_image.c index 87804094f2..e5dd0c780d 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -200,7 +200,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; }