Revert "Security: Make sure not to overflow the in memory version of the GBB"

This breaks recovery mode on Kaen - the bitmaps are not displayed.

This reverts commit e1153e1f56ebebff188f3693e534f10bd68e6f07

Change-Id: I300ae39382dc1960bb0375ad660a88b65181edc9
Reviewed-on: https://gerrit.chromium.org/gerrit/13274
Reviewed-by: Gabe Black (Do Not Use) <gabeblack@google.com>
Commit-Ready: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>

4 files changed, 31 insertions, 68 deletions

diff --git a/common/cmd_vbexport_test.c b/common/cmd_vbexport_test.c
index cf7b9f95ac..c267e12f47 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 gbb_size;
+	size_t size;
 
 	gbb = fdt_decode_chromeos_alloc_region(gd->blob,
-			"google-binary-block", &gbb_size);
+			"google-binary-block", &size);
 	if (!gbb) {
 		VbExDebug("Failed to find gbb region!\n");
 		return NULL;
@@ -423,13 +423,12 @@ static uint8_t *read_gbb_from_firmware(void)
 		return NULL;
 	}
 
-	if (gbb_init(gbb, &file, fmap.readonly.gbb.offset, gbb_size)) {
+	if (gbb_init(gbb, &file, fmap.readonly.gbb.offset)) {
 		VbExDebug("Failed to read GBB!\n");
 		return NULL;
 	}
 
-	if (gbb_read_bmp_block(gbb, &file, fmap.readonly.gbb.offset,
-			       gbb_size)) {
+	if (gbb_read_bmp_block(gbb, &file, fmap.readonly.gbb.offset)) {
 		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 9336572fb5..8659b66b3a 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, size_t gbb_size,
-			   crossystem_data_t *cdata, VbCommonParams *cparams)
+			   uint32_t gbb_offset, 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, gbb_size))
+		if (gbb_read_bmp_block(gbb, file, gbb_offset))
 			return 1;
 	if (cdata->boot_developer_switch ||
 			iparams.out_flags & VB_INIT_OUT_ENABLE_RECOVERY) {
-		if (gbb_read_recovery_key(gbb, file, gbb_offset, gbb_size))
+		if (gbb_read_recovery_key(gbb, file, gbb_offset))
 			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,
-				     size_t gbb_size, crossystem_data_t *cdata,
+				     firmware_storage_t *file,
+				     void *gbb, crossystem_data_t *cdata,
 				     void *vb_shared_data, int *boot_mode,
 				     void **fw_blob_ptr, uint32_t *fw_size_ptr)
 {
@@ -424,9 +424,8 @@ twostop_select_and_set_main_firmware(struct twostop_fmap *fmap,
 		return VB_SELECT_ERROR;
 	}
 
-	if (twostop_init_vboot_library(file, gbb, gbb_size,
-				       fmap->readonly.gbb.offset, cdata,
-				       &cparams)
+	if (twostop_init_vboot_library(file, gbb, fmap->readonly.gbb.offset,
+				       cdata, &cparams)
 			!= VBERROR_SUCCESS) {
 		VBDEBUG(PREFIX "failed to init vboot library\n");
 		return VB_SELECT_ERROR;
@@ -514,8 +513,7 @@ 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, size_t gbb_size, crossystem_data_t *cdata,
-	     void *vb_shared_data)
+	     void **gbbp, crossystem_data_t *cdata, void *vb_shared_data)
 {
 	cros_gpio_t wpsw, recsw, devsw;
 	GoogleBinaryBlockHeader *gbbh;
@@ -563,14 +561,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, gbb_size)) {
+	if (gbb_init(gbbp, file, fmap->readonly.gbb.offset)) {
 		VBDEBUG(PREFIX "failed to read gbb\n");
 		goto out;
 	}
 	gbb = *gbbp;
 #else
 	gbb = *gbbp;
-	if (gbb_init(gbb, file, fmap->readonly.gbb.offset, gbb_size)) {
+	if (gbb_init(gbb, file, fmap->readonly.gbb.offset)) {
 		VBDEBUG(PREFIX "failed to read gbb\n");
 		goto out;
 	}
@@ -672,14 +670,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, size_t *gbb_size,
-			       crossystem_data_t **cdata, int verify)
+static int setup_gbb_and_cdata(void **gbb, 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", gbb_size);
+			"google-binary-block", &size);
 
 	if (!*gbb) {
 		VBDEBUG(PREFIX "google-binary-block missing "
@@ -719,25 +717,23 @@ 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, &gbb_size, &cdata, 0))
+	if (setup_gbb_and_cdata(&gbb, &cdata, 0))
 		return VB_SELECT_ERROR;
 
 	vb_shared_data = cdata->vb_shared_data;
-	if (twostop_init(&fmap, &file, &gbb, gbb_size, cdata,
-			 vb_shared_data)) {
+	if (twostop_init(&fmap, &file, &gbb, 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, gbb_size, cdata, vb_shared_data,
+			gbb, cdata, vb_shared_data,
 			&boot_mode, &fw_blob, &fw_size);
 	VBDEBUG(PREFIX "selection of bootstub: %s\n", str_selection(selection));
 
@@ -790,7 +786,6 @@ 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");
@@ -801,7 +796,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, &gbb_size, &cdata, 1))
+	if (setup_gbb_and_cdata(&gbb, &cdata, 1))
 		return VB_SELECT_ERROR;
 
 	/*
diff --git a/include/chromeos/gbb.h b/include/chromeos/gbb.h
index d3770f51b1..c3bc304b38 100644
--- a/include/chromeos/gbb.h
+++ b/include/chromeos/gbb.h
@@ -22,11 +22,9 @@
  * @param gbb		Buffer for holding GBB
  * @param file		Flashrom device handle
  * @param gbb_offset	Offset of GBB in flashrom device
- * @param gbb_size	Size of the buffer holding GBB
  * @return zero if this succeeds, non-zero if this fails
  */
-int gbb_init(read_buf_type gbb, firmware_storage_t *file, uint32_t gbb_offset,
-	     size_t gbb_size);
+int gbb_init(read_buf_type gbb, firmware_storage_t *file, uint32_t gbb_offset);
 
 #ifndef CONFIG_HARDWARE_MAPPED_SPI
 /**
@@ -35,11 +33,10 @@ int gbb_init(read_buf_type gbb, firmware_storage_t *file, uint32_t gbb_offset,
  * @param gbb		Buffer for holding GBB
  * @param file		Flashrom device handle
  * @param gbb_offset	Offset of GBB in flashrom device
- * @param gbb_size	Size of the buffer holding GBB
  * @return zero if this succeeds, non-zero if this fails
  */
-int gbb_read_bmp_block(read_buf_type gbb, firmware_storage_t *file,
-		       uint32_t gbb_offset, size_t gbb_size);
+int gbb_read_bmp_block(read_buf_type gbb,
+		firmware_storage_t *file, uint32_t gbb_offset);
 
 /*
  * This loads the recovery key of GBB from flashrom.
@@ -47,11 +44,10 @@ int gbb_read_bmp_block(read_buf_type gbb, firmware_storage_t *file,
  * @param gbb		Buffer for holding GBB
  * @param file		Flashrom device handle
  * @param gbb_offset	Offset of GBB in flashrom device
- * @param gbb_size	Size of the buffer holding GBB
  * @return zero if this succeeds, non-zero if this fails
  */
-int gbb_read_recovery_key(read_buf_type gbb, firmware_storage_t *file,
-			  uint32_t gbb_offset, size_t gbb_size);
+int gbb_read_recovery_key(read_buf_type gbb,
+		firmware_storage_t *file, uint32_t gbb_offset);
 
 #else
 
diff --git a/lib/chromeos/gbb.c b/lib/chromeos/gbb.c
index b2ab343b6f..25d5741318 100644
--- a/lib/chromeos/gbb.c
+++ b/lib/chromeos/gbb.c
@@ -16,28 +16,16 @@
 
 #define PREFIX "gbb: "
 
-int gbb_init(read_buf_type gbb, firmware_storage_t *file, uint32_t gbb_offset,
-	     size_t gbb_size)
+int gbb_init(read_buf_type gbb, firmware_storage_t *file, uint32_t gbb_offset)
 {
 #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 "invalid gbb header entries");
-		return 1;
-	}
-
 	if (file->read(file, gbb_offset + gbbh->hwid_offset,
 				gbbh->hwid_size,
 				gbb + gbbh->hwid_offset)) {
@@ -52,7 +40,6 @@ 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");
@@ -64,16 +51,9 @@ 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,
-		       size_t gbb_size)
+int gbb_read_bmp_block(void *gbb, firmware_storage_t *file, uint32_t gbb_offset)
 {
 	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 "invalid gbb header entries");
-		return 1;
-	}
 
 	if (file->read(file, gbb_offset + gbbh->bmpfv_offset,
 				gbbh->bmpfv_size,
@@ -85,17 +65,10 @@ 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, size_t gbb_size)
+int gbb_read_recovery_key(void *gbb,
+		firmware_storage_t *file, uint32_t gbb_offset)
 {
 	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 "invalid gbb header entries");
-		return 1;
-	}
 
 	if (file->read(file, gbb_offset + gbbh->recovery_key_offset,
 				gbbh->recovery_key_size,