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>

2 files changed, 26 insertions, 21 deletions

diff --git a/common/cmd_vbexport_test.c b/common/cmd_vbexport_test.c
index c267e12f47..cf7b9f95ac 100644
--- a/common/cmd_vbexport_test.c
+++ b/common/cmd_vbexport_test.c
@@ -403,10 +403,10 @@ static uint8_t *read_gbb_from_firmware(void)
 	firmware_storage_t file;
 	struct twostop_fmap fmap;
 	void *gbb;
-	size_t size;
+	size_t gbb_size;
 
 	gbb = fdt_decode_chromeos_alloc_region(gd->blob,
-			"google-binary-block", &size);
+			"google-binary-block", &gbb_size);
 	if (!gbb) {
 		VbExDebug("Failed to find gbb region!\n");
 		return NULL;
@@ -423,12 +423,13 @@ static uint8_t *read_gbb_from_firmware(void)
 		return NULL;
 	}
 
-	if (gbb_init(gbb, &file, fmap.readonly.gbb.offset)) {
+	if (gbb_init(gbb, &file, fmap.readonly.gbb.offset, gbb_size)) {
 		VbExDebug("Failed to read GBB!\n");
 		return NULL;
 	}
 
-	if (gbb_read_bmp_block(gbb, &file, fmap.readonly.gbb.offset)) {
+	if (gbb_read_bmp_block(gbb, &file, fmap.readonly.gbb.offset,
+			       gbb_size)) {
 		VbExDebug("Failed to load BMP Block in GBB!\n");
 		return NULL;
 	}
diff --git a/common/cmd_vboot_twostop.c b/common/cmd_vboot_twostop.c
index 8659b66b3a..2d8bf37e45 100644
--- a/common/cmd_vboot_twostop.c
+++ b/common/cmd_vboot_twostop.c
@@ -268,8 +268,8 @@ static void wipe_unused_memory(crossystem_data_t *cdata,
 
 static VbError_t
 twostop_init_vboot_library(firmware_storage_t *file, void *gbb,
-			   uint32_t gbb_offset, crossystem_data_t *cdata,
-			   VbCommonParams *cparams)
+			   uint32_t gbb_offset, size_t gbb_size,
+			   crossystem_data_t *cdata, VbCommonParams *cparams)
 {
 	VbError_t err;
 	VbInitParams iparams;
@@ -297,11 +297,11 @@ twostop_init_vboot_library(firmware_storage_t *file, void *gbb,
 
 	/* Load required information of GBB */
 	if (iparams.out_flags & VB_INIT_OUT_ENABLE_DISPLAY)
-		if (gbb_read_bmp_block(gbb, file, gbb_offset))
+		if (gbb_read_bmp_block(gbb, file, gbb_offset, gbb_size))
 			return 1;
 	if (cdata->boot_developer_switch ||
 			iparams.out_flags & VB_INIT_OUT_ENABLE_RECOVERY) {
-		if (gbb_read_recovery_key(gbb, file, gbb_offset))
+		if (gbb_read_recovery_key(gbb, file, gbb_offset, gbb_size))
 			return 1;
 	}
 
@@ -402,8 +402,8 @@ out:
 
 static uint32_t
 twostop_select_and_set_main_firmware(struct twostop_fmap *fmap,
-				     firmware_storage_t *file,
-				     void *gbb, crossystem_data_t *cdata,
+				     firmware_storage_t *file, void *gbb,
+				     size_t gbb_size, crossystem_data_t *cdata,
 				     void *vb_shared_data, int *boot_mode,
 				     void **fw_blob_ptr, uint32_t *fw_size_ptr)
 {
@@ -425,7 +425,7 @@ twostop_select_and_set_main_firmware(struct twostop_fmap *fmap,
 	}
 
 	if (twostop_init_vboot_library(file, gbb, fmap->readonly.gbb.offset,
-				       cdata, &cparams)
+				       gbb_size, cdata, &cparams)
 			!= VBERROR_SUCCESS) {
 		VBDEBUG(PREFIX "failed to init vboot library\n");
 		return VB_SELECT_ERROR;
@@ -513,7 +513,8 @@ twostop_jump(crossystem_data_t *cdata, void *fw_blob, uint32_t fw_size)
 
 static int
 twostop_init(struct twostop_fmap *fmap, firmware_storage_t *file,
-	     void **gbbp, crossystem_data_t *cdata, void *vb_shared_data)
+	     void **gbbp, size_t gbb_size, crossystem_data_t *cdata,
+	     void *vb_shared_data)
 {
 	cros_gpio_t wpsw, recsw, devsw;
 	GoogleBinaryBlockHeader *gbbh;
@@ -561,14 +562,14 @@ twostop_init(struct twostop_fmap *fmap, firmware_storage_t *file,
 
 	/* Load basic parts of gbb blob */
 #ifdef CONFIG_HARDWARE_MAPPED_SPI
-	if (gbb_init(gbbp, file, fmap->readonly.gbb.offset)) {
+	if (gbb_init(gbbp, file, fmap->readonly.gbb.offset, gbb_size)) {
 		VBDEBUG(PREFIX "failed to read gbb\n");
 		goto out;
 	}
 	gbb = *gbbp;
 #else
 	gbb = *gbbp;
-	if (gbb_init(gbb, file, fmap->readonly.gbb.offset)) {
+	if (gbb_init(gbb, file, fmap->readonly.gbb.offset, gbb_size)) {
 		VBDEBUG(PREFIX "failed to read gbb\n");
 		goto out;
 	}
@@ -670,14 +671,14 @@ twostop_main_firmware(struct twostop_fmap *fmap, void *gbb,
  * @param verify	1 to verify data, 0 to skip this step
  * @return 0 if ok, -1 on error
  */
-static int setup_gbb_and_cdata(void **gbb, crossystem_data_t **cdata,
-			       int verify)
+static int setup_gbb_and_cdata(void **gbb, size_t *gbb_size,
+			       crossystem_data_t **cdata, int verify)
 {
 	size_t size;
 
 #ifndef CONFIG_HARDWARE_MAPPED_SPI
 	*gbb = fdt_decode_chromeos_alloc_region(gd->blob,
-			"google-binary-block", &size);
+			"google-binary-block", gbb_size);
 
 	if (!*gbb) {
 		VBDEBUG(PREFIX "google-binary-block missing "
@@ -717,23 +718,25 @@ twostop_boot(void)
 	firmware_storage_t file;
 	crossystem_data_t *cdata;
 	void *gbb;
+	size_t gbb_size = 0;
 	void *vb_shared_data;
 	void *fw_blob = NULL;
 	uint32_t fw_size = 0;
 	uint32_t selection;
 	int boot_mode = FIRMWARE_TYPE_NORMAL;
 
-	if (setup_gbb_and_cdata(&gbb, &cdata, 0))
+	if (setup_gbb_and_cdata(&gbb, &gbb_size, &cdata, 0))
 		return VB_SELECT_ERROR;
 
 	vb_shared_data = cdata->vb_shared_data;
-	if (twostop_init(&fmap, &file, &gbb, cdata, vb_shared_data)) {
+	if (twostop_init(&fmap, &file, &gbb, gbb_size, cdata,
+			 vb_shared_data)) {
 		VBDEBUG(PREFIX "failed to init twostop boot\n");
 		return VB_SELECT_ERROR;
 	}
 
 	selection = twostop_select_and_set_main_firmware(&fmap, &file,
-			gbb, cdata, vb_shared_data,
+			gbb, gbb_size, cdata, vb_shared_data,
 			&boot_mode, &fw_blob, &fw_size);
 	VBDEBUG(PREFIX "selection of bootstub: %s\n", str_selection(selection));
 
@@ -786,6 +789,7 @@ twostop_readwrite_main_firmware(void)
 	struct twostop_fmap fmap;
 	crossystem_data_t *cdata;
 	void *gbb;
+	size_t gbb_size;
 
 	if (fdt_decode_twostop_fmap(gd->blob, &fmap)) {
 		VBDEBUG(PREFIX "failed to decode fmap\n");
@@ -796,7 +800,7 @@ twostop_readwrite_main_firmware(void)
 #ifdef CONFIG_HARDWARE_MAPPED_SPI
 	gbb = (void *) (fmap.readonly.gbb.offset + fmap.flash_base);
 #endif
-	if (setup_gbb_and_cdata(&gbb, &cdata, 1))
+	if (setup_gbb_and_cdata(&gbb, &gbb_size, &cdata, 1))
 		return VB_SELECT_ERROR;
 
 	/*