summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorGabe Black <gabeblack@chromium.org>2011-12-20 01:46:46 -0800
committerGabe Black <gabeblack@chromium.org>2011-12-20 17:54:47 -0800
commitfe623bdec87124735ae59fce52aea05782f688ca (patch)
tree69f6762884e1ef96490415d9e6e09704f7f4f139 /lib
parent63b3a649ddf4df0bb5b010b0a8d1e3c6c31f85f2 (diff)
Security: Make sure not to overflow the in memory version of the GBB
This is a revised version of this patch which fixes an ARM bug. This change plumbs the size of the GBB specified in the device tree to the functions that read it from the flash into memory, and adds checks to those functions to make sure they don't spill out of the in memory GBB. From a security standpoint this is a largely theoretical problem since the GBB is in the read only portion of flash and if that can be modified the machine is totally compromised, but it's possible somehow an attacker could force vboot to read the GBB from the wrong place. From a practical perspective it's not a bad idea to check this to avoid accidental memory corruption. BUG=chromium-os:24223 TEST=Built and booted on Lumpy. Built for Kaen. Change-Id: I90d23fd6e055db595af12b1bd63d9932cbffe7ae Signed-off-by: Gabe Black <gabeblack@google.com> Reviewed-on: https://gerrit.chromium.org/gerrit/13279 Tested-by: Simon Glass <sjg@chromium.org> Reviewed-by: Gabe Black <gabeblack@chromium.org>
Diffstat (limited to 'lib')
-rw-r--r--lib/chromeos/gbb.c52
1 files changed, 48 insertions, 4 deletions
diff --git a/lib/chromeos/gbb.c b/lib/chromeos/gbb.c
index 25d57413185..77e524b2c1c 100644
--- a/lib/chromeos/gbb.c
+++ b/lib/chromeos/gbb.c
@@ -16,16 +16,39 @@
#define PREFIX "gbb: "
-int gbb_init(read_buf_type gbb, firmware_storage_t *file, uint32_t gbb_offset)
+int gbb_init(read_buf_type gbb, firmware_storage_t *file, uint32_t gbb_offset,
+ size_t gbb_size)
{
#ifndef CONFIG_HARDWARE_MAPPED_SPI
GoogleBinaryBlockHeader *gbbh = (GoogleBinaryBlockHeader *)gbb;
+ uint32_t hwid_end;
+ uint32_t rootkey_end;
if (file->read(file, gbb_offset, sizeof(*gbbh), gbbh)) {
VBDEBUG(PREFIX "failed to read GBB header\n");
return 1;
}
+ hwid_end = gbbh->hwid_offset + gbbh->hwid_size;
+ rootkey_end = gbbh->rootkey_offset + gbbh->rootkey_size;
+ if (hwid_end < gbbh->hwid_offset || hwid_end > gbb_size ||
+ rootkey_end < gbbh->rootkey_offset ||
+ rootkey_end > gbb_size) {
+ VBDEBUG(PREFIX "%s: invalid gbb header entries\n", __func__);
+ VBDEBUG(PREFIX " hwid_end=%x\n", hwid_end);
+ VBDEBUG(PREFIX " gbbh->hwid_offset=%x\n", gbbh->hwid_offset);
+ VBDEBUG(PREFIX " gbb_size=%x\n", gbb_size);
+ VBDEBUG(PREFIX " rootkey_end=%x\n", rootkey_end);
+ VBDEBUG(PREFIX " gbbh->rootkey_offset=%x\n",
+ gbbh->rootkey_offset);
+ VBDEBUG(PREFIX " %d, %d, %d, %d\n",
+ hwid_end < gbbh->hwid_offset,
+ hwid_end >= gbb_size,
+ rootkey_end < gbbh->rootkey_offset,
+ rootkey_end >= gbb_size);
+ return 1;
+ }
+
if (file->read(file, gbb_offset + gbbh->hwid_offset,
gbbh->hwid_size,
gbb + gbbh->hwid_offset)) {
@@ -40,6 +63,7 @@ int gbb_init(read_buf_type gbb, firmware_storage_t *file, uint32_t gbb_offset)
return 1;
}
#else
+ /* No data is actually moved in this case so no bounds checks. */
if (file->read(file, gbb_offset,
sizeof(GoogleBinaryBlockHeader), gbb)) {
VBDEBUG(PREFIX "failed to read GBB header\n");
@@ -51,9 +75,16 @@ int gbb_init(read_buf_type gbb, firmware_storage_t *file, uint32_t gbb_offset)
}
#ifndef CONFIG_HARDWARE_MAPPED_SPI
-int gbb_read_bmp_block(void *gbb, firmware_storage_t *file, uint32_t gbb_offset)
+int gbb_read_bmp_block(void *gbb, firmware_storage_t *file, uint32_t gbb_offset,
+ size_t gbb_size)
{
GoogleBinaryBlockHeader *gbbh = (GoogleBinaryBlockHeader *)gbb;
+ uint32_t bmpfv_end = gbbh->bmpfv_offset + gbbh->bmpfv_size;
+
+ if (bmpfv_end < gbbh->bmpfv_offset || bmpfv_end > gbb_size) {
+ VBDEBUG(PREFIX "%s: invalid gbb header entries\n", __func__);
+ return 1;
+ }
if (file->read(file, gbb_offset + gbbh->bmpfv_offset,
gbbh->bmpfv_size,
@@ -65,10 +96,23 @@ int gbb_read_bmp_block(void *gbb, firmware_storage_t *file, uint32_t gbb_offset)
return 0;
}
-int gbb_read_recovery_key(void *gbb,
- firmware_storage_t *file, uint32_t gbb_offset)
+int gbb_read_recovery_key(void *gbb, firmware_storage_t *file,
+ uint32_t gbb_offset, size_t gbb_size)
{
GoogleBinaryBlockHeader *gbbh = (GoogleBinaryBlockHeader *)gbb;
+ uint32_t rkey_end = gbbh->recovery_key_offset +
+ gbbh->recovery_key_size;
+
+ if (rkey_end < gbbh->recovery_key_offset || rkey_end > gbb_size) {
+ VBDEBUG(PREFIX "%s: invalid gbb header entries\n", __func__);
+ VBDEBUG(PREFIX " gbbh->recovery_key_offset=%x\n",
+ gbbh->recovery_key_offset);
+ VBDEBUG(PREFIX " gbbh->recovery_key_size=%x\n",
+ gbbh->recovery_key_size);
+ VBDEBUG(PREFIX " rkey_end=%x\n", rkey_end);
+ VBDEBUG(PREFIX " gbb_size=%x\n", gbb_size);
+ return 1;
+ }
if (file->read(file, gbb_offset + gbbh->recovery_key_offset,
gbbh->recovery_key_size,