region: Introduce region_create() functions

We introduce two new functions to create region objects. They allow us
to check for integer overflows (region_create_untrusted()) or assert
their absence (region_create()).

This fixes potential overflows in region_overlap() checks in SMI
handlers, where we would wrongfully report MMIO as *not* overlapping
SMRAM.

Also, two cases of strtol() in parse_region() (cbfstool),  where the
results were implicitly converted to `size_t`, are replaced with the
unsigned strtoul().

FIT payload support is left out, as it doesn't use the region API
(only the struct).

Change-Id: I4ae3e6274c981c9ab4fb1263c2a72fa68ef1c32b
Ticket: https://ticket.coreboot.org/issues/522
Found-by: Vadim Zaliva <lord@digamma.ai>
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/79905
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>

4 files changed, 19 insertions, 26 deletions

diff --git a/src/drivers/intel/fsp1_1/fsp_report.c b/src/drivers/intel/fsp1_1/fsp_report.c
index 884218d7f7..f5c7b2f05b 100644
--- a/src/drivers/intel/fsp1_1/fsp_report.c
+++ b/src/drivers/intel/fsp1_1/fsp_report.c
@@ -10,14 +10,11 @@ uintptr_t temp_memory_end;
 
 void report_fsp_output(void)
 {
-	const struct region fsp_car_region = {
-		.offset = temp_memory_start,
-		.size = temp_memory_end - temp_memory_start,
-	};
-	const struct region coreboot_car_region = {
-		.offset = (uintptr_t)_car_region_start,
-		.size = (uintptr_t)_car_region_size,
-	};
+	const struct region fsp_car_region = region_create(
+			temp_memory_start, temp_memory_end - temp_memory_start);
+	const struct region coreboot_car_region = region_create(
+			(uintptr_t)_car_region_start, (uintptr_t)_car_region_size);
+
 	printk(BIOS_DEBUG, "FSP: reported temp_mem region: [0x%08lx,0x%08lx)\n",
 	       temp_memory_start, temp_memory_end);
 	if (!region_is_subregion(&fsp_car_region, &coreboot_car_region)) {
diff --git a/src/drivers/intel/fsp2_0/fspt_report.c b/src/drivers/intel/fsp2_0/fspt_report.c
index 7fa3205e3d..87c08637c1 100644
--- a/src/drivers/intel/fsp2_0/fspt_report.c
+++ b/src/drivers/intel/fsp2_0/fspt_report.c
@@ -10,14 +10,11 @@ uintptr_t temp_memory_end;
 
 void report_fspt_output(void)
 {
-	const struct region fsp_car_region = {
-		.offset = temp_memory_start,
-		.size = temp_memory_end - temp_memory_start,
-	};
-	const struct region coreboot_car_region = {
-		.offset = (uintptr_t)_car_region_start,
-		.size = (uintptr_t)_car_region_size,
-	};
+	const struct region fsp_car_region = region_create(
+			temp_memory_start, temp_memory_end - temp_memory_start);
+	const struct region coreboot_car_region = region_create(
+			(uintptr_t)_car_region_start, (uintptr_t)_car_region_size);
+
 	printk(BIOS_DEBUG, "FSP-T: reported temp_mem region: [0x%08lx,0x%08lx)\n",
 	       temp_memory_start, temp_memory_end);
 	if (!region_is_subregion(&fsp_car_region, &coreboot_car_region)) {
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index 42952df531..11597c6c1d 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -610,12 +610,12 @@ int spi_flash_status(const struct spi_flash *flash, u8 *reg)
 int spi_flash_is_write_protected(const struct spi_flash *flash,
 				 const struct region *region)
 {
-	struct region flash_region = { 0 };
+	struct region flash_region;
 
 	if (!flash || !region)
 		return -1;
 
-	flash_region.size = flash->size;
+	flash_region = region_create(0, flash->size);
 
 	if (!region_is_subregion(&flash_region, region))
 		return -1;
@@ -633,13 +633,13 @@ int spi_flash_set_write_protected(const struct spi_flash *flash,
 				  const struct region *region,
 				  const enum spi_flash_status_reg_lockdown mode)
 {
-	struct region flash_region = { 0 };
+	struct region flash_region;
 	int ret;
 
 	if (!flash)
 		return -1;
 
-	flash_region.size = flash->size;
+	flash_region = region_create(0, flash->size);
 
 	if (!region_is_subregion(&flash_region, region))
 		return -1;
@@ -755,12 +755,12 @@ int spi_flash_ctrlr_protect_region(const struct spi_flash *flash,
 				   const enum ctrlr_prot_type type)
 {
 	const struct spi_ctrlr *ctrlr;
-	struct region flash_region = { 0 };
+	struct region flash_region;
 
 	if (!flash)
 		return -1;
 
-	flash_region.size = flash->size;
+	flash_region = region_create(0, flash->size);
 
 	if (!region_is_subregion(&flash_region, region))
 		return -1;
diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c
index 3137bfe8a7..3426d08e70 100644
--- a/src/drivers/spi/winbond.c
+++ b/src/drivers/spi/winbond.c
@@ -267,8 +267,7 @@ static void winbond_bpbits_to_region(const size_t granularity,
 		tb = !tb;
 	}
 
-	out->offset = tb ? 0 : flash_size - protected_size;
-	out->size = protected_size;
+	*out = region_create(tb ? 0 : flash_size - protected_size, protected_size);
 }
 
 /*
@@ -525,8 +524,8 @@ winbond_set_write_protection(const struct spi_flash *flash,
 
 	if (region_sz(&wp_region) > flash->size / 2) {
 		cmp = 1;
-		wp_region.offset = tb ? 0 : region_sz(&wp_region);
-		wp_region.size = flash->size - region_sz(&wp_region);
+		wp_region = region_create(tb ? 0 : region_sz(&wp_region),
+					  flash->size - region_sz(&wp_region));
 		tb = !tb;
 	} else {
 		cmp = 0;