[PATCH] armv8: sec_firmware: check secure memory bounds

[Josh Law]

sec_firmware_load_image() copies the firmware image into the reserved
secure memory area after the MMU tables. The FIT image size was used
without checking that the copy still fits inside that reservation.

Loadable images use the same secure memory area and had the same issue.

Check the destination range before copying either image type. Keep the
copy size as size_t so the FIT image size is not truncated before
memcpy().

Signed-off-by: Josh Law <josh2 at disroot.org>
---
 arch/arm/cpu/armv8/sec_firmware.c | 50 ++++++++++++++++++++++++++-----
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
index 8c31fd19399..5b15739c9b7 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -26,6 +26,7 @@ extern void c_runtime_cpu_setup(void);
 #define SEC_FIRMWARE_LOADED	0x1
 #define SEC_FIRMWARE_RUNNING	0x2
 #define SEC_FIRMWARE_ADDR_MASK	(~0x3)
+#define SEC_FIRMWARE_PHYS_ADDR_MAX	((phys_addr_t)~0ULL)
 /*
  * Secure firmware load addr
  * Flags used: 0x1 secure firmware has been loaded to secure memory
@@ -47,6 +48,24 @@ static int sec_firmware_get_data(const void *sec_firmware_img,
 				      data, size);
 }
 
+static int sec_firmware_check_copy_range(phys_addr_t dest, size_t size)
+{
+	phys_addr_t secure_start;
+	phys_addr_t secure_end;
+
+	secure_start = gd->arch.secure_ram & MEM_RESERVE_SECURE_ADDR_MASK;
+	if (CFG_SYS_MEM_RESERVE_SECURE >
+	    SEC_FIRMWARE_PHYS_ADDR_MAX - secure_start)
+		return -EOVERFLOW;
+
+	secure_end = secure_start + CFG_SYS_MEM_RESERVE_SECURE;
+	if (dest < secure_start || dest > secure_end ||
+	    size > secure_end - dest)
+		return -ENOSPC;
+
+	return 0;
+}
+
 /*
  * SEC Firmware FIT image parser checks if the image is in FIT
  * format, verifies integrity of the image and calculates raw
@@ -87,7 +106,7 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
 	size_t size;
 	ulong load;
 	const char *str, *type;
-	int count, i, len;
+	int count, i, len, ret;
 
 	conf_node_off = fit_conf_get_node(sec_firmware_img, NULL);
 	if (conf_node_off < 0) {
@@ -153,8 +172,20 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
 		}
 
 		/* Compute load address for loadable in secure memory */
-		sec_firmware_loadable_addr = (sec_firmware_addr -
-						gd->arch.tlb_size) + load;
+		sec_firmware_loadable_addr = sec_firmware_addr -
+					     gd->arch.tlb_size;
+		if (load > SEC_FIRMWARE_PHYS_ADDR_MAX -
+		    sec_firmware_loadable_addr) {
+			printf("SEC Loadable: invalid load address\n");
+			return -EOVERFLOW;
+		}
+		sec_firmware_loadable_addr += load;
+		ret = sec_firmware_check_copy_range(sec_firmware_loadable_addr,
+						    size);
+		if (ret) {
+			printf("SEC Loadable: image exceeds secure memory\n");
+			return ret;
+		}
 
 		/* Copy loadable to secure memory and flush dcache */
 		debug("%s copied to address 0x%p\n",
@@ -180,7 +211,7 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
 }
 
 static int sec_firmware_copy_image(const char *title,
-			 u64 image_addr, u32 image_size, u64 sec_firmware)
+			 u64 image_addr, size_t image_size, u64 sec_firmware)
 {
 	debug("%s copied to address 0x%p\n", title, (void *)sec_firmware);
 	memcpy((void *)sec_firmware, (void *)image_addr, image_size);
@@ -236,10 +267,13 @@ static int sec_firmware_load_image(const void *sec_firmware_img,
 	if (ret)
 		goto out;
 
-	/* TODO:
-	 * Check if the end addr of SEC Firmware has been extend the secure
-	 * memory.
-	 */
+	ret = sec_firmware_check_copy_range(sec_firmware_addr &
+					    SEC_FIRMWARE_ADDR_MASK,
+					    raw_image_size);
+	if (ret) {
+		printf("SEC Firmware: image exceeds secure memory\n");
+		goto out;
+	}
 
 	/* Copy the secure firmware to secure memory */
 	ret = sec_firmware_copy_image("SEC Firmware", (u64)raw_image_addr,
-- 
2.47.3