From af0d4bce65df277b56e495892dff1c712ed76ddd Mon Sep 17 00:00:00 2001 From: Nico Huber Date: Thu, 11 Jan 2024 18:59:24 +0100 Subject: 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 Signed-off-by: Nico Huber Reviewed-on: https://review.coreboot.org/c/coreboot/+/79905 Tested-by: build bot (Jenkins) Reviewed-by: Felix Held --- util/cbfstool/cse_serger.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) (limited to 'util/cbfstool/cse_serger.c') diff --git a/util/cbfstool/cse_serger.c b/util/cbfstool/cse_serger.c index f53b3904af..006a01e9be 100644 --- a/util/cbfstool/cse_serger.c +++ b/util/cbfstool/cse_serger.c @@ -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; -- cgit v1.2.3