Andrew Geissler | 95ac1b8 | 2021-03-31 14:34:31 -0500 | [diff] [blame^] | 1 | From c5819701a3de61e2ba2ef7ad0b616565b32305e5 Mon Sep 17 00:00:00 2001 |
| 2 | From: Simon Glass <sjg@chromium.org> |
| 3 | Date: Mon, 15 Feb 2021 17:08:09 -0700 |
| 4 | Subject: [PATCH] image: Adjust the workings of fit_check_format() |
| 5 | |
| 6 | At present this function does not accept a size for the FIT. This means |
| 7 | that it must be read from the FIT itself, introducing potential security |
| 8 | risk. Update the function to include a size parameter, which can be |
| 9 | invalid, in which case fit_check_format() calculates it. |
| 10 | |
| 11 | For now no callers pass the size, but this can be updated later. |
| 12 | |
| 13 | Also adjust the return value to an error code so that all the different |
| 14 | types of problems can be distinguished by the user. |
| 15 | |
| 16 | Signed-off-by: Simon Glass <sjg@chromium.org> |
| 17 | Reported-by: Bruce Monroe <bruce.monroe@intel.com> |
| 18 | Reported-by: Arie Haenel <arie.haenel@intel.com> |
| 19 | Reported-by: Julien Lenoir <julien.lenoir@intel.com> |
| 20 | |
| 21 | CVE: CVE-2021-27097 CVE-2021-27138 |
| 22 | Upstream-Status: Backport[https://github.com/u-boot/u-boot/commit/c5819701a3de61e2ba2ef7ad0b616565b32305e5] |
| 23 | Signed-off-by: Scott Murray <scott.murray@konsulko.com> |
| 24 | |
| 25 | --- |
| 26 | arch/arm/cpu/armv8/sec_firmware.c | 2 +- |
| 27 | cmd/bootm.c | 6 ++--- |
| 28 | cmd/disk.c | 2 +- |
| 29 | cmd/fpga.c | 2 +- |
| 30 | cmd/nand.c | 2 +- |
| 31 | cmd/source.c | 2 +- |
| 32 | cmd/ximg.c | 2 +- |
| 33 | common/image-fdt.c | 2 +- |
| 34 | common/image-fit.c | 46 +++++++++++++++++--------------------- |
| 35 | common/splash_source.c | 6 ++--- |
| 36 | common/update.c | 4 ++-- |
| 37 | drivers/fpga/socfpga_arria10.c | 6 ++--- |
| 38 | drivers/net/fsl-mc/mc.c | 2 +- |
| 39 | drivers/net/pfe_eth/pfe_firmware.c | 2 +- |
| 40 | include/image.h | 21 ++++++++++++++++- |
| 41 | tools/fit_common.c | 3 ++- |
| 42 | tools/fit_image.c | 2 +- |
| 43 | tools/mkimage.h | 2 ++ |
| 44 | 18 files changed, 65 insertions(+), 49 deletions(-) |
| 45 | |
| 46 | diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c |
| 47 | index bfc0fac3ef..0561f5efd1 100644 |
| 48 | --- a/arch/arm/cpu/armv8/sec_firmware.c |
| 49 | +++ b/arch/arm/cpu/armv8/sec_firmware.c |
| 50 | @@ -316,7 +316,7 @@ __weak bool sec_firmware_is_valid(const void *sec_firmware_img) |
| 51 | return false; |
| 52 | } |
| 53 | |
| 54 | - if (!fit_check_format(sec_firmware_img)) { |
| 55 | + if (fit_check_format(sec_firmware_img, IMAGE_SIZE_INVAL)) { |
| 56 | printf("SEC Firmware: Bad firmware image (bad FIT header)\n"); |
| 57 | return false; |
| 58 | } |
| 59 | diff --git a/cmd/bootm.c b/cmd/bootm.c |
| 60 | index e6b0e04413..a0f823f968 100644 |
| 61 | --- a/cmd/bootm.c |
| 62 | +++ b/cmd/bootm.c |
| 63 | @@ -291,7 +291,7 @@ static int image_info(ulong addr) |
| 64 | case IMAGE_FORMAT_FIT: |
| 65 | puts(" FIT image found\n"); |
| 66 | |
| 67 | - if (!fit_check_format(hdr)) { |
| 68 | + if (fit_check_format(hdr, IMAGE_SIZE_INVAL)) { |
| 69 | puts("Bad FIT image format!\n"); |
| 70 | unmap_sysmem(hdr); |
| 71 | return 1; |
| 72 | @@ -368,7 +368,7 @@ static int do_imls_nor(void) |
| 73 | #endif |
| 74 | #if defined(CONFIG_FIT) |
| 75 | case IMAGE_FORMAT_FIT: |
| 76 | - if (!fit_check_format(hdr)) |
| 77 | + if (fit_check_format(hdr, IMAGE_SIZE_INVAL)) |
| 78 | goto next_sector; |
| 79 | |
| 80 | printf("FIT Image at %08lX:\n", (ulong)hdr); |
| 81 | @@ -448,7 +448,7 @@ static int nand_imls_fitimage(struct mtd_info *mtd, int nand_dev, loff_t off, |
| 82 | return ret; |
| 83 | } |
| 84 | |
| 85 | - if (!fit_check_format(imgdata)) { |
| 86 | + if (fit_check_format(imgdata, IMAGE_SIZE_INVAL)) { |
| 87 | free(imgdata); |
| 88 | return 0; |
| 89 | } |
| 90 | diff --git a/cmd/disk.c b/cmd/disk.c |
| 91 | index 8060e753eb..3195db9127 100644 |
| 92 | --- a/cmd/disk.c |
| 93 | +++ b/cmd/disk.c |
| 94 | @@ -114,7 +114,7 @@ int common_diskboot(struct cmd_tbl *cmdtp, const char *intf, int argc, |
| 95 | /* This cannot be done earlier, |
| 96 | * we need complete FIT image in RAM first */ |
| 97 | if (genimg_get_format((void *) addr) == IMAGE_FORMAT_FIT) { |
| 98 | - if (!fit_check_format(fit_hdr)) { |
| 99 | + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { |
| 100 | bootstage_error(BOOTSTAGE_ID_IDE_FIT_READ); |
| 101 | puts("** Bad FIT image format\n"); |
| 102 | return 1; |
| 103 | diff --git a/cmd/fpga.c b/cmd/fpga.c |
| 104 | index 8ae1c936fb..51410a8e42 100644 |
| 105 | --- a/cmd/fpga.c |
| 106 | +++ b/cmd/fpga.c |
| 107 | @@ -330,7 +330,7 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc, |
| 108 | return CMD_RET_FAILURE; |
| 109 | } |
| 110 | |
| 111 | - if (!fit_check_format(fit_hdr)) { |
| 112 | + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { |
| 113 | puts("Bad FIT image format\n"); |
| 114 | return CMD_RET_FAILURE; |
| 115 | } |
| 116 | diff --git a/cmd/nand.c b/cmd/nand.c |
| 117 | index 92d039af8f..97e117a979 100644 |
| 118 | --- a/cmd/nand.c |
| 119 | +++ b/cmd/nand.c |
| 120 | @@ -917,7 +917,7 @@ static int nand_load_image(struct cmd_tbl *cmdtp, struct mtd_info *mtd, |
| 121 | #if defined(CONFIG_FIT) |
| 122 | /* This cannot be done earlier, we need complete FIT image in RAM first */ |
| 123 | if (genimg_get_format ((void *)addr) == IMAGE_FORMAT_FIT) { |
| 124 | - if (!fit_check_format (fit_hdr)) { |
| 125 | + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { |
| 126 | bootstage_error(BOOTSTAGE_ID_NAND_FIT_READ); |
| 127 | puts ("** Bad FIT image format\n"); |
| 128 | return 1; |
| 129 | diff --git a/cmd/source.c b/cmd/source.c |
| 130 | index b6c709a3d2..71f71528ad 100644 |
| 131 | --- a/cmd/source.c |
| 132 | +++ b/cmd/source.c |
| 133 | @@ -107,7 +107,7 @@ int image_source_script(ulong addr, const char *fit_uname) |
| 134 | #if defined(CONFIG_FIT) |
| 135 | case IMAGE_FORMAT_FIT: |
| 136 | fit_hdr = buf; |
| 137 | - if (!fit_check_format (fit_hdr)) { |
| 138 | + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { |
| 139 | puts ("Bad FIT image format\n"); |
| 140 | return 1; |
| 141 | } |
| 142 | diff --git a/cmd/ximg.c b/cmd/ximg.c |
| 143 | index 159ba51648..ef738ebfa2 100644 |
| 144 | --- a/cmd/ximg.c |
| 145 | +++ b/cmd/ximg.c |
| 146 | @@ -136,7 +136,7 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) |
| 147 | "at %08lx ...\n", uname, addr); |
| 148 | |
| 149 | fit_hdr = (const void *)addr; |
| 150 | - if (!fit_check_format(fit_hdr)) { |
| 151 | + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { |
| 152 | puts("Bad FIT image format\n"); |
| 153 | return 1; |
| 154 | } |
| 155 | diff --git a/common/image-fdt.c b/common/image-fdt.c |
| 156 | index 327a8c4c39..4105259212 100644 |
| 157 | --- a/common/image-fdt.c |
| 158 | +++ b/common/image-fdt.c |
| 159 | @@ -399,7 +399,7 @@ int boot_get_fdt(int flag, int argc, char *const argv[], uint8_t arch, |
| 160 | */ |
| 161 | #if CONFIG_IS_ENABLED(FIT) |
| 162 | /* check FDT blob vs FIT blob */ |
| 163 | - if (fit_check_format(buf)) { |
| 164 | + if (!fit_check_format(buf, IMAGE_SIZE_INVAL)) { |
| 165 | ulong load, len; |
| 166 | |
| 167 | fdt_noffset = boot_get_fdt_fit(images, |
| 168 | diff --git a/common/image-fit.c b/common/image-fit.c |
| 169 | index 9637d747fb..402f08fc9d 100644 |
| 170 | --- a/common/image-fit.c |
| 171 | +++ b/common/image-fit.c |
| 172 | @@ -8,6 +8,8 @@ |
| 173 | * Wolfgang Denk, DENX Software Engineering, wd@denx.de. |
| 174 | */ |
| 175 | |
| 176 | +#define LOG_CATEGORY LOGC_BOOT |
| 177 | + |
| 178 | #ifdef USE_HOSTCC |
| 179 | #include "mkimage.h" |
| 180 | #include <time.h> |
| 181 | @@ -1550,49 +1552,41 @@ int fit_image_check_comp(const void *fit, int noffset, uint8_t comp) |
| 182 | return (comp == image_comp); |
| 183 | } |
| 184 | |
| 185 | -/** |
| 186 | - * fit_check_format - sanity check FIT image format |
| 187 | - * @fit: pointer to the FIT format image header |
| 188 | - * |
| 189 | - * fit_check_format() runs a basic sanity FIT image verification. |
| 190 | - * Routine checks for mandatory properties, nodes, etc. |
| 191 | - * |
| 192 | - * returns: |
| 193 | - * 1, on success |
| 194 | - * 0, on failure |
| 195 | - */ |
| 196 | -int fit_check_format(const void *fit) |
| 197 | +int fit_check_format(const void *fit, ulong size) |
| 198 | { |
| 199 | + int ret; |
| 200 | + |
| 201 | /* A FIT image must be a valid FDT */ |
| 202 | - if (fdt_check_header(fit)) { |
| 203 | - debug("Wrong FIT format: not a flattened device tree\n"); |
| 204 | - return 0; |
| 205 | + ret = fdt_check_header(fit); |
| 206 | + if (ret) { |
| 207 | + log_debug("Wrong FIT format: not a flattened device tree (err=%d)\n", |
| 208 | + ret); |
| 209 | + return -ENOEXEC; |
| 210 | } |
| 211 | |
| 212 | /* mandatory / node 'description' property */ |
| 213 | - if (fdt_getprop(fit, 0, FIT_DESC_PROP, NULL) == NULL) { |
| 214 | - debug("Wrong FIT format: no description\n"); |
| 215 | - return 0; |
| 216 | + if (!fdt_getprop(fit, 0, FIT_DESC_PROP, NULL)) { |
| 217 | + log_debug("Wrong FIT format: no description\n"); |
| 218 | + return -ENOMSG; |
| 219 | } |
| 220 | |
| 221 | if (IMAGE_ENABLE_TIMESTAMP) { |
| 222 | /* mandatory / node 'timestamp' property */ |
| 223 | - if (fdt_getprop(fit, 0, FIT_TIMESTAMP_PROP, NULL) == NULL) { |
| 224 | - debug("Wrong FIT format: no timestamp\n"); |
| 225 | - return 0; |
| 226 | + if (!fdt_getprop(fit, 0, FIT_TIMESTAMP_PROP, NULL)) { |
| 227 | + log_debug("Wrong FIT format: no timestamp\n"); |
| 228 | + return -ENODATA; |
| 229 | } |
| 230 | } |
| 231 | |
| 232 | /* mandatory subimages parent '/images' node */ |
| 233 | if (fdt_path_offset(fit, FIT_IMAGES_PATH) < 0) { |
| 234 | - debug("Wrong FIT format: no images parent node\n"); |
| 235 | - return 0; |
| 236 | + log_debug("Wrong FIT format: no images parent node\n"); |
| 237 | + return -ENOENT; |
| 238 | } |
| 239 | |
| 240 | - return 1; |
| 241 | + return 0; |
| 242 | } |
| 243 | |
| 244 | - |
| 245 | /** |
| 246 | * fit_conf_find_compat |
| 247 | * @fit: pointer to the FIT format image header |
| 248 | @@ -1929,7 +1923,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr, |
| 249 | printf("## Loading %s from FIT Image at %08lx ...\n", prop_name, addr); |
| 250 | |
| 251 | bootstage_mark(bootstage_id + BOOTSTAGE_SUB_FORMAT); |
| 252 | - if (!fit_check_format(fit)) { |
| 253 | + if (fit_check_format(fit, IMAGE_SIZE_INVAL)) { |
| 254 | printf("Bad FIT %s image format!\n", prop_name); |
| 255 | bootstage_error(bootstage_id + BOOTSTAGE_SUB_FORMAT); |
| 256 | return -ENOEXEC; |
| 257 | diff --git a/common/splash_source.c b/common/splash_source.c |
| 258 | index f51ca5ddf3..bad9a7790a 100644 |
| 259 | --- a/common/splash_source.c |
| 260 | +++ b/common/splash_source.c |
| 261 | @@ -336,10 +336,10 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr) |
| 262 | if (res < 0) |
| 263 | return res; |
| 264 | |
| 265 | - res = fit_check_format(fit_header); |
| 266 | - if (!res) { |
| 267 | + res = fit_check_format(fit_header, IMAGE_SIZE_INVAL); |
| 268 | + if (res) { |
| 269 | debug("Could not find valid FIT image\n"); |
| 270 | - return -EINVAL; |
| 271 | + return res; |
| 272 | } |
| 273 | |
| 274 | /* Get the splash image node */ |
| 275 | diff --git a/common/update.c b/common/update.c |
| 276 | index a5879cb52c..f0848954e5 100644 |
| 277 | --- a/common/update.c |
| 278 | +++ b/common/update.c |
| 279 | @@ -286,7 +286,7 @@ int update_tftp(ulong addr, char *interface, char *devstring) |
| 280 | got_update_file: |
| 281 | fit = map_sysmem(addr, 0); |
| 282 | |
| 283 | - if (!fit_check_format((void *)fit)) { |
| 284 | + if (fit_check_format((void *)fit, IMAGE_SIZE_INVAL)) { |
| 285 | printf("Bad FIT format of the update file, aborting " |
| 286 | "auto-update\n"); |
| 287 | return 1; |
| 288 | @@ -363,7 +363,7 @@ int fit_update(const void *fit) |
| 289 | if (!fit) |
| 290 | return -EINVAL; |
| 291 | |
| 292 | - if (!fit_check_format((void *)fit)) { |
| 293 | + if (fit_check_format((void *)fit, IMAGE_SIZE_INVAL)) { |
| 294 | printf("Bad FIT format of the update file, aborting auto-update\n"); |
| 295 | return -EINVAL; |
| 296 | } |
| 297 | diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c |
| 298 | index 44e1ac54c3..18f99676d2 100644 |
| 299 | --- a/drivers/fpga/socfpga_arria10.c |
| 300 | +++ b/drivers/fpga/socfpga_arria10.c |
| 301 | @@ -565,10 +565,10 @@ static int first_loading_rbf_to_buffer(struct udevice *dev, |
| 302 | if (ret < 0) |
| 303 | return ret; |
| 304 | |
| 305 | - ret = fit_check_format(buffer_p); |
| 306 | - if (!ret) { |
| 307 | + ret = fit_check_format(buffer_p, IMAGE_SIZE_INVAL); |
| 308 | + if (ret) { |
| 309 | debug("FPGA: No valid FIT image was found.\n"); |
| 310 | - return -EBADF; |
| 311 | + return ret; |
| 312 | } |
| 313 | |
| 314 | confs_noffset = fdt_path_offset(buffer_p, FIT_CONFS_PATH); |
| 315 | diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c |
| 316 | index 84db6be624..81265ee356 100644 |
| 317 | --- a/drivers/net/fsl-mc/mc.c |
| 318 | +++ b/drivers/net/fsl-mc/mc.c |
| 319 | @@ -141,7 +141,7 @@ int parse_mc_firmware_fit_image(u64 mc_fw_addr, |
| 320 | return -EINVAL; |
| 321 | } |
| 322 | |
| 323 | - if (!fit_check_format(fit_hdr)) { |
| 324 | + if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) { |
| 325 | printf("fsl-mc: ERR: Bad firmware image (bad FIT header)\n"); |
| 326 | return -EINVAL; |
| 327 | } |
| 328 | diff --git a/drivers/net/pfe_eth/pfe_firmware.c b/drivers/net/pfe_eth/pfe_firmware.c |
| 329 | index 41999e176d..eee70a2e73 100644 |
| 330 | --- a/drivers/net/pfe_eth/pfe_firmware.c |
| 331 | +++ b/drivers/net/pfe_eth/pfe_firmware.c |
| 332 | @@ -160,7 +160,7 @@ static int pfe_fit_check(void) |
| 333 | return ret; |
| 334 | } |
| 335 | |
| 336 | - if (!fit_check_format(pfe_fit_addr)) { |
| 337 | + if (fit_check_format(pfe_fit_addr, IMAGE_SIZE_INVAL)) { |
| 338 | printf("PFE Firmware: Bad firmware image (bad FIT header)\n"); |
| 339 | ret = -1; |
| 340 | return ret; |
| 341 | diff --git a/include/image.h b/include/image.h |
| 342 | index 41473dbb9c..8c152c5c5f 100644 |
| 343 | --- a/include/image.h |
| 344 | +++ b/include/image.h |
| 345 | @@ -134,6 +134,9 @@ extern ulong image_load_addr; /* Default Load Address */ |
| 346 | extern ulong image_save_addr; /* Default Save Address */ |
| 347 | extern ulong image_save_size; /* Default Save Size */ |
| 348 | |
| 349 | +/* An invalid size, meaning that the image size is not known */ |
| 350 | +#define IMAGE_SIZE_INVAL (-1UL) |
| 351 | + |
| 352 | enum ih_category { |
| 353 | IH_ARCH, |
| 354 | IH_COMP, |
| 355 | @@ -1141,7 +1144,23 @@ int fit_image_check_os(const void *fit, int noffset, uint8_t os); |
| 356 | int fit_image_check_arch(const void *fit, int noffset, uint8_t arch); |
| 357 | int fit_image_check_type(const void *fit, int noffset, uint8_t type); |
| 358 | int fit_image_check_comp(const void *fit, int noffset, uint8_t comp); |
| 359 | -int fit_check_format(const void *fit); |
| 360 | + |
| 361 | +/** |
| 362 | + * fit_check_format() - Check that the FIT is valid |
| 363 | + * |
| 364 | + * This performs various checks on the FIT to make sure it is suitable for |
| 365 | + * use, looking for mandatory properties, nodes, etc. |
| 366 | + * |
| 367 | + * If FIT_FULL_CHECK is enabled, it also runs it through libfdt to make |
| 368 | + * sure that there are no strange tags or broken nodes in the FIT. |
| 369 | + * |
| 370 | + * @fit: pointer to the FIT format image header |
| 371 | + * @return 0 if OK, -ENOEXEC if not an FDT file, -EINVAL if the full FDT check |
| 372 | + * failed (e.g. due to bad structure), -ENOMSG if the description is |
| 373 | + * missing, -ENODATA if the timestamp is missing, -ENOENT if the /images |
| 374 | + * path is missing |
| 375 | + */ |
| 376 | +int fit_check_format(const void *fit, ulong size); |
| 377 | |
| 378 | int fit_conf_find_compat(const void *fit, const void *fdt); |
| 379 | |
| 380 | diff --git a/tools/fit_common.c b/tools/fit_common.c |
| 381 | index cdf987d3c1..52b63296f8 100644 |
| 382 | --- a/tools/fit_common.c |
| 383 | +++ b/tools/fit_common.c |
| 384 | @@ -26,7 +26,8 @@ |
| 385 | int fit_verify_header(unsigned char *ptr, int image_size, |
| 386 | struct image_tool_params *params) |
| 387 | { |
| 388 | - if (fdt_check_header(ptr) != EXIT_SUCCESS || !fit_check_format(ptr)) |
| 389 | + if (fdt_check_header(ptr) != EXIT_SUCCESS || |
| 390 | + fit_check_format(ptr, IMAGE_SIZE_INVAL)) |
| 391 | return EXIT_FAILURE; |
| 392 | |
| 393 | return EXIT_SUCCESS; |
| 394 | diff --git a/tools/fit_image.c b/tools/fit_image.c |
| 395 | index 06faeda7c2..d440d143c6 100644 |
| 396 | --- a/tools/fit_image.c |
| 397 | +++ b/tools/fit_image.c |
| 398 | @@ -883,7 +883,7 @@ static int fit_extract_contents(void *ptr, struct image_tool_params *params) |
| 399 | /* Indent string is defined in header image.h */ |
| 400 | p = IMAGE_INDENT_STRING; |
| 401 | |
| 402 | - if (!fit_check_format(fit)) { |
| 403 | + if (fit_check_format(fit, IMAGE_SIZE_INVAL)) { |
| 404 | printf("Bad FIT image format\n"); |
| 405 | return -1; |
| 406 | } |
| 407 | diff --git a/tools/mkimage.h b/tools/mkimage.h |
| 408 | index 5b096a545b..0d3148444c 100644 |
| 409 | --- a/tools/mkimage.h |
| 410 | +++ b/tools/mkimage.h |
| 411 | @@ -29,6 +29,8 @@ |
| 412 | #define debug(fmt,args...) |
| 413 | #endif /* MKIMAGE_DEBUG */ |
| 414 | |
| 415 | +#define log_debug(fmt, args...) debug(fmt, ##args) |
| 416 | + |
| 417 | static inline void *map_sysmem(ulong paddr, unsigned long len) |
| 418 | { |
| 419 | return (void *)(uintptr_t)paddr; |