FWU: Check for overlaps when loading images
Added checks to FWU_SMC_IMAGE_COPY to prevent loading data into a
memory region where another image data is already loaded.
Without this check, if two images are configured to be loaded in
overlapping memory regions, one of them can be loaded and
authenticated and the copy function is still able to load data from
the second image on top of the first one. Since the first image is
still in authenticated state, it can be executed, which could lead to
the execution of unauthenticated arbitrary code of the second image.
Firmware update documentation updated.
Change-Id: Ib6871e569794c8e610a5ea59fe162ff5dcec526c
Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>
diff --git a/bl1/bl1_fwu.c b/bl1/bl1_fwu.c
index ace364d..43bb4d2 100644
--- a/bl1/bl1_fwu.c
+++ b/bl1/bl1_fwu.c
@@ -91,6 +91,124 @@
}
/*******************************************************************************
+ * Utility functions to keep track of the images that are loaded at any time.
+ ******************************************************************************/
+
+#ifdef PLAT_FWU_MAX_SIMULTANEOUS_IMAGES
+#define FWU_MAX_SIMULTANEOUS_IMAGES PLAT_FWU_MAX_SIMULTANEOUS_IMAGES
+#else
+#define FWU_MAX_SIMULTANEOUS_IMAGES 10
+#endif
+
+static int bl1_fwu_loaded_ids[FWU_MAX_SIMULTANEOUS_IMAGES] = {
+ [0 ... FWU_MAX_SIMULTANEOUS_IMAGES-1] = INVALID_IMAGE_ID
+};
+
+/*
+ * Adds an image_id to the bl1_fwu_loaded_ids array.
+ * Returns 0 on success, 1 on error.
+ */
+static int bl1_fwu_add_loaded_id(int image_id)
+{
+ int i;
+
+ /* Check if the ID is already in the list */
+ for (i = 0; i < FWU_MAX_SIMULTANEOUS_IMAGES; i++) {
+ if (bl1_fwu_loaded_ids[i] == image_id)
+ return 0;
+ }
+
+ /* Find an empty slot */
+ for (i = 0; i < FWU_MAX_SIMULTANEOUS_IMAGES; i++) {
+ if (bl1_fwu_loaded_ids[i] == INVALID_IMAGE_ID) {
+ bl1_fwu_loaded_ids[i] = image_id;
+ return 0;
+ }
+ }
+
+ return 1;
+}
+
+/*
+ * Removes an image_id from the bl1_fwu_loaded_ids array.
+ * Returns 0 on success, 1 on error.
+ */
+static int bl1_fwu_remove_loaded_id(int image_id)
+{
+ int i;
+
+ /* Find the ID */
+ for (i = 0; i < FWU_MAX_SIMULTANEOUS_IMAGES; i++) {
+ if (bl1_fwu_loaded_ids[i] == image_id) {
+ bl1_fwu_loaded_ids[i] = INVALID_IMAGE_ID;
+ return 0;
+ }
+ }
+
+ return 1;
+}
+
+/*******************************************************************************
+ * This function checks if the specified image overlaps another image already
+ * loaded. It returns 0 if there is no overlap, a negative error code otherwise.
+ ******************************************************************************/
+static int bl1_fwu_image_check_overlaps(int image_id)
+{
+ const image_desc_t *image_desc, *checked_image_desc;
+ const image_info_t *info, *checked_info;
+
+ uintptr_t image_base, image_end;
+ uintptr_t checked_image_base, checked_image_end;
+
+ checked_image_desc = bl1_plat_get_image_desc(image_id);
+ checked_info = &checked_image_desc->image_info;
+
+ /* Image being checked mustn't be empty. */
+ assert(checked_info->image_size != 0);
+
+ checked_image_base = checked_info->image_base;
+ checked_image_end = checked_image_base + checked_info->image_size - 1;
+ /* No need to check for overlaps, it's done in bl1_fwu_image_copy(). */
+
+ for (int i = 0; i < FWU_MAX_SIMULTANEOUS_IMAGES; i++) {
+
+ /* Don't check image against itself. */
+ if (bl1_fwu_loaded_ids[i] == image_id)
+ continue;
+
+ image_desc = bl1_plat_get_image_desc(bl1_fwu_loaded_ids[i]);
+
+ /* Only check images that are loaded or being loaded. */
+ assert (image_desc->state != IMAGE_STATE_RESET);
+
+ info = &image_desc->image_info;
+
+ /* There cannot be overlaps with an empty image. */
+ if (info->image_size == 0)
+ continue;
+
+ image_base = info->image_base;
+ image_end = image_base + info->image_size - 1;
+ /*
+ * Overflows cannot happen. It is checked in
+ * bl1_fwu_image_copy() when the image goes from RESET to
+ * COPYING or COPIED.
+ */
+ assert (image_end > image_base);
+
+ /* Check if there are overlaps. */
+ if (!(image_end < checked_image_base ||
+ checked_image_end < image_base)) {
+ VERBOSE("Image with ID %d overlaps existing image with ID %d",
+ checked_image_desc->image_id, image_desc->image_id);
+ return -EPERM;
+ }
+ }
+
+ return 0;
+}
+
+/*******************************************************************************
* This function is responsible for copying secure images in AP Secure RAM.
******************************************************************************/
static int bl1_fwu_image_copy(unsigned int image_id,
@@ -189,6 +307,13 @@
/* Save the given image size. */
image_desc->image_info.image_size = image_size;
+ /* Make sure the image doesn't overlap other images. */
+ if (bl1_fwu_image_check_overlaps(image_id)) {
+ image_desc->image_info.image_size = 0;
+ WARN("BL1-FWU: This image overlaps another one\n");
+ return -EPERM;
+ }
+
/*
* copied_size must be explicitly initialized here because the
* FWU code doesn't necessarily do it when it resets the state
@@ -213,6 +338,11 @@
return -ENOMEM;
}
+ if (bl1_fwu_add_loaded_id(image_id)) {
+ WARN("BL1-FWU: Too many images loaded at the same time.\n");
+ return -ENOMEM;
+ }
+
/* Everything looks sane. Go ahead and copy the block of data. */
dest_addr = image_desc->image_info.image_base + image_desc->copied_size;
memcpy((void *) dest_addr, (const void *) image_src, block_size);
@@ -290,6 +420,11 @@
return -ENOMEM;
}
+ if (bl1_fwu_add_loaded_id(image_id)) {
+ WARN("BL1-FWU: Too many images loaded at the same time.\n");
+ return -ENOMEM;
+ }
+
base_addr = image_src;
total_size = image_size;
@@ -319,6 +454,13 @@
/* Indicate that image can be copied again*/
image_desc->state = IMAGE_STATE_RESET;
}
+
+ /*
+ * Even if this fails it's ok because the ID isn't in the array.
+ * The image cannot be in RESET state here, it is checked at the
+ * beginning of the function.
+ */
+ bl1_fwu_remove_loaded_id(image_id);
return -EAUTH;
}
@@ -475,6 +617,13 @@
assert(EP_GET_EXE(image_desc->ep_info.h.attr) == EXECUTABLE);
assert(image_desc->state == IMAGE_STATE_EXECUTED);
+#if ENABLE_ASSERTIONS
+ int rc = bl1_fwu_remove_loaded_id(sec_exec_image_id);
+ assert(rc == 0);
+#else
+ bl1_fwu_remove_loaded_id(sec_exec_image_id);
+#endif
+
/* Update the flags. */
image_desc->state = IMAGE_STATE_RESET;
sec_exec_image_id = INVALID_IMAGE_ID;
diff --git a/docs/firmware-update.md b/docs/firmware-update.md
index 21872fd..56ef15c 100644
--- a/docs/firmware-update.md
+++ b/docs/firmware-update.md
@@ -211,6 +211,7 @@
if (source block is in secure memory) return -ENOMEM
if (source block is not mapped into BL1) return -ENOMEM
if (image_size > free secure memory) return -ENOMEM
+ if (image overlaps another image) return -EPERM
This SMC copies the secure image indicated by `image_id` from non-secure memory
to secure memory for later authentication. The image may be copied in a single
diff --git a/docs/porting-guide.md b/docs/porting-guide.md
index 4d7a5ea..c7b9e89 100644
--- a/docs/porting-guide.md
+++ b/docs/porting-guide.md
@@ -354,6 +354,13 @@
NS_BL2U image identifier, used by BL1 to fetch an image descriptor
corresponding to NS_BL2U.
+For the the Firmware update capability of TRUSTED BOARD BOOT, the following
+macros may also be defined:
+
+* **#define : PLAT_FWU_MAX_SIMULTANEOUS_IMAGES**
+
+ Total number of images that can be loaded simultaneously. If the platform
+ doesn't specify any value, it defaults to 10.
If a SCP_BL2 image is supported by the platform, the following constants must
also be defined: