aboutsummaryrefslogtreecommitdiff
path: root/bl1
diff options
context:
space:
mode:
authorAntonio Nino Diaz <antonio.ninodiaz@arm.com>2017-06-01 13:40:17 +0100
committerAntonio Nino Diaz <antonio.ninodiaz@arm.com>2017-06-01 14:52:11 +0100
commit128daee29868a8a4a7cf00508126ea68311fd1cc (patch)
treea92dead477d6afb662f0a366011b6a4821d6efd2 /bl1
parent79eb1aff7850f6b23a302835d7a08936d74e8ce2 (diff)
downloadtrusted-firmware-a-128daee29868a8a4a7cf00508126ea68311fd1cc.tar.gz
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>
Diffstat (limited to 'bl1')
-rw-r--r--bl1/bl1_fwu.c149
1 files changed, 149 insertions, 0 deletions
diff --git a/bl1/bl1_fwu.c b/bl1/bl1_fwu.c
index ace364d422..43bb4d2bae 100644
--- a/bl1/bl1_fwu.c
+++ b/bl1/bl1_fwu.c
@@ -91,6 +91,124 @@ register_t bl1_fwu_smc_handler(unsigned int smc_fid,
}
/*******************************************************************************
+ * 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 @@ static int bl1_fwu_image_copy(unsigned int image_id,
/* 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 @@ static int bl1_fwu_image_copy(unsigned int image_id,
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 @@ static int bl1_fwu_image_auth(unsigned int image_id,
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 @@ static int bl1_fwu_image_auth(unsigned int image_id,
/* 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 @@ static int bl1_fwu_sec_image_done(void **handle, unsigned int flags)
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;