[U-Boot] [PATCH v2] spl: implement CRC check on U-Boot uImage

* [U-Boot] [PATCH v2] spl: implement CRC check on U-Boot uImage
@ 2018-11-28 20:52 Simon Goldschmidt
  2018-12-11  1:07 ` Simon Glass
  2019-02-08 19:46 ` [U-Boot] [U-Boot,v2] " Tom Rini
  0 siblings, 2 replies; 12+ messages in thread
From: Simon Goldschmidt @ 2018-11-28 20:52 UTC (permalink / raw)
  To: u-boot

SPL currently does not check uImage CRCs when loading U-Boot.

This patch adds checking the uImage CRC when SPL loads U-Boot. It does
this by reusing the existing config option SPL_CRC32_SUPPORT to allow
leaving out the CRC check on boards where the additional code size or
boot time is a problem (adding the CRC check currently adds ~1.4 kByte
to flash).

The SPL_CRC32_SUPPORT config option now gets enabled by default if SPL
support for legacy images is enabled to check the CRC on all boards
that don't actively take countermeasures.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

Changes in v2:
- added Kconfig option SPL_LEGACY_IMAGE_CRC_CHECK to enable/disable
  checking CRC on legacy images

 common/spl/Kconfig | 21 +++++++++++++++------
 common/spl/spl.c   | 30 +++++++++++++++++++++++++++++-
 include/spl.h      |  5 +++++
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index d0564621d4..5c6c437ef0 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -80,6 +80,15 @@ config SPL_LEGACY_IMAGE_SUPPORT
 	  is y. If this is not set, SPL will move on to other available
 	  boot media to find a suitable image.
 
+config SPL_LEGACY_IMAGE_CRC_CHECK
+	bool "Check CRC of Legacy images"
+	depends on SPL_LEGACY_IMAGE_SUPPORT
+	default y
+	select SPL_CRC32_SUPPORT
+	help
+	  Enable this to check the CRC of Legacy images. While this increases
+	  reliability, it affects both code size and boot duration.
+
 config SPL_SYS_MALLOC_SIMPLE
 	bool
 	prompt "Only use malloc_simple functions in the SPL"
@@ -207,13 +216,13 @@ config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE
 
 config SPL_CRC32_SUPPORT
 	bool "Support CRC32"
-	depends on SPL_FIT
+	default y if SPL_LEGACY_IMAGE_SUPPORT
 	help
-	  Enable this to support CRC32 in FIT images within SPL. This is a
-	  32-bit checksum value that can be used to verify images. This is
-	  the least secure type of checksum, suitable for detected
-	  accidental image corruption. For secure applications you should
-	  consider SHA1 or SHA256.
+	  Enable this to support CRC32 in uImages or FIT images within SPL.
+	  This is a 32-bit checksum value that can be used to verify images.
+	  For FIT images, this is the least secure type of checksum, suitable
+	  for detected accidental image corruption. For secure applications you
+	  should consider SHA1 or SHA256.
 
 config SPL_MD5_SUPPORT
 	bool "Support MD5"
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 12f9359c0a..f2ea87a0ec 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -236,6 +236,14 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
 #ifdef CONFIG_SPL_LEGACY_IMAGE_SUPPORT
 		u32 header_size = sizeof(struct image_header);
 
+		if (CONFIG_IS_ENABLED(LEGACY_IMAGE_CRC_CHECK)) {
+			/* check uImage header CRC */
+			if (!image_check_hcrc(header)) {
+				puts("SPL: Image header CRC check failed!\n");
+				return -EINVAL;
+			}
+		}
+
 		if (spl_image->flags & SPL_COPY_PAYLOAD_ONLY) {
 			/*
 			 * On some system (e.g. powerpc), the load-address and
@@ -253,6 +261,13 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
 			spl_image->size = image_get_data_size(header) +
 				header_size;
 		}
+#ifdef CONFIG_SPL_LEGACY_IMAGE_CRC_CHECK
+		/* store uImage data length and CRC to check later */
+		spl_image->dcrc_data = image_get_load(header);
+		spl_image->dcrc_length = image_get_data_size(header);
+		spl_image->dcrc = image_get_dcrc(header);
+#endif
+
 		spl_image->os = image_get_os(header);
 		spl_image->name = image_get_name(header);
 		debug("spl: payload image: %32s load addr: 0x%lx size: %d\n",
@@ -424,12 +439,25 @@ static struct spl_image_loader *spl_ll_find_loader(uint boot_device)
 static int spl_load_image(struct spl_image_info *spl_image,
 			  struct spl_image_loader *loader)
 {
+	int ret;
 	struct spl_boot_device bootdev;
 
 	bootdev.boot_device = loader->boot_device;
 	bootdev.boot_device_name = NULL;
 
-	return loader->load_image(spl_image, &bootdev);
+	ret = loader->load_image(spl_image, &bootdev);
+#ifdef CONFIG_SPL_LEGACY_IMAGE_CRC_CHECK
+	if (!ret && spl_image->dcrc_length) {
+		/* check data crc */
+		ulong dcrc = crc32_wd(0, (unsigned char *)spl_image->dcrc_data,
+				      spl_image->dcrc_length, CHUNKSZ_CRC32);
+		if (dcrc != spl_image->dcrc) {
+			puts("SPL: Image data CRC check failed!\n");
+			ret = -EINVAL;
+		}
+	}
+#endif
+	return ret;
 }
 
 /**
diff --git a/include/spl.h b/include/spl.h
index 9a439f468b..1b1d9ebbfb 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -33,6 +33,11 @@ struct spl_image_info {
 	u32 size;
 	u32 flags;
 	void *arg;
+#if CONFIG_IS_ENABLED(LEGACY_IMAGE_CRC_CHECK)
+	ulong dcrc_data;
+	ulong dcrc_length;
+	ulong dcrc;
+#endif
 };
 
 /*
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread