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>
This commit is contained in:
@@ -324,19 +324,25 @@ struct mmap_window {
|
||||
static int mmap_window_table_size;
|
||||
static struct mmap_window mmap_window_table[MMAP_MAX_WINDOWS];
|
||||
|
||||
static void add_mmap_window(size_t flash_offset, size_t host_offset,
|
||||
size_t window_size)
|
||||
static int add_mmap_window(unsigned long flash_offset, unsigned long host_offset, unsigned long window_size)
|
||||
{
|
||||
if (mmap_window_table_size >= MMAP_MAX_WINDOWS) {
|
||||
ERROR("Too many memory map windows\n");
|
||||
return;
|
||||
return 1;
|
||||
}
|
||||
|
||||
if (region_create_untrusted(
|
||||
&mmap_window_table[mmap_window_table_size].flash_space,
|
||||
flash_offset, window_size) != CB_SUCCESS ||
|
||||
region_create_untrusted(
|
||||
&mmap_window_table[mmap_window_table_size].host_space,
|
||||
host_offset, window_size) != CB_SUCCESS) {
|
||||
ERROR("Invalid mmap window size %lu.\n", window_size);
|
||||
return 1;
|
||||
}
|
||||
|
||||
mmap_window_table[mmap_window_table_size].flash_space.offset = flash_offset;
|
||||
mmap_window_table[mmap_window_table_size].host_space.offset = host_offset;
|
||||
mmap_window_table[mmap_window_table_size].flash_space.size = window_size;
|
||||
mmap_window_table[mmap_window_table_size].host_space.size = window_size;
|
||||
mmap_window_table_size++;
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
@@ -377,7 +383,9 @@ static int decode_mmap_arg(char *arg)
|
||||
return 1;
|
||||
}
|
||||
|
||||
add_mmap_window(mmap_args.flash_base, mmap_args.mmap_base, mmap_args.mmap_size);
|
||||
if (add_mmap_window(mmap_args.flash_base, mmap_args.mmap_base, mmap_args.mmap_size))
|
||||
return 1;
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
@@ -403,7 +411,8 @@ static bool create_mmap_windows(void)
|
||||
* maximum of 16MiB. If the window is smaller than 16MiB, the SPI flash window is mapped
|
||||
* at the top of the host window just below 4G.
|
||||
*/
|
||||
add_mmap_window(std_window_flash_offset, DEFAULT_DECODE_WINDOW_TOP - std_window_size, std_window_size);
|
||||
if (add_mmap_window(std_window_flash_offset, DEFAULT_DECODE_WINDOW_TOP - std_window_size, std_window_size))
|
||||
return false;
|
||||
} else {
|
||||
/*
|
||||
* Check provided memory map
|
||||
|
@@ -820,15 +820,22 @@ static int cmd_add(void)
|
||||
return 0;
|
||||
}
|
||||
|
||||
static void parse_region(struct region *r, char *arg)
|
||||
static int parse_region(struct region *r, char *arg)
|
||||
{
|
||||
char *tok;
|
||||
|
||||
tok = strtok(arg, ":");
|
||||
r->offset = strtol(tok, NULL, 0);
|
||||
unsigned long offset = strtoul(tok, NULL, 0);
|
||||
|
||||
tok = strtok(NULL, ":");
|
||||
r->size = strtol(tok, NULL, 0);
|
||||
unsigned long size = strtoul(tok, NULL, 0);
|
||||
|
||||
if (region_create_untrusted(r, offset, size) != CB_SUCCESS) {
|
||||
ERROR("Invalid region: %lx:%lx\n", offset, size);
|
||||
return -1;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
static struct command {
|
||||
@@ -991,19 +998,24 @@ int main(int argc, char **argv)
|
||||
params.partition_type = atoi(optarg);
|
||||
break;
|
||||
case LONGOPT_BP1:
|
||||
parse_region(¶ms.layout_regions[BP1], optarg);
|
||||
if (parse_region(¶ms.layout_regions[BP1], optarg))
|
||||
return 1;
|
||||
break;
|
||||
case LONGOPT_BP2:
|
||||
parse_region(¶ms.layout_regions[BP2], optarg);
|
||||
if (parse_region(¶ms.layout_regions[BP2], optarg))
|
||||
return 1;
|
||||
break;
|
||||
case LONGOPT_BP3:
|
||||
parse_region(¶ms.layout_regions[BP3], optarg);
|
||||
if (parse_region(¶ms.layout_regions[BP3], optarg))
|
||||
return 1;
|
||||
break;
|
||||
case LONGOPT_BP4:
|
||||
parse_region(¶ms.layout_regions[BP4], optarg);
|
||||
if (parse_region(¶ms.layout_regions[BP4], optarg))
|
||||
return 1;
|
||||
break;
|
||||
case LONGOPT_DATA:
|
||||
parse_region(¶ms.layout_regions[DP], optarg);
|
||||
if (parse_region(¶ms.layout_regions[DP], optarg))
|
||||
return 1;
|
||||
break;
|
||||
case LONGOPT_BP1_FILE:
|
||||
params.layout_files[BP1] = optarg;
|
||||
|
Reference in New Issue
Block a user