From 41feb32559d574e7e08f42742dc4ef8abc3ddd46 Mon Sep 17 00:00:00 2001 From: Nico Huber Date: Sun, 14 Jan 2024 14:26:37 +0100 Subject: region: Turn region_end() into an inclusive region_last() The current region_end() implementation is susceptible to overflow if the region is at the end of the addressable space. A common case with the memory-mapped flash of x86 directly below the 32-bit limit. Note: This patch also changes console output to inclusive limits. IMO, to the better. Change-Id: Ic4bd6eced638745b7e845504da74542e4220554a Signed-off-by: Nico Huber Reviewed-on: https://review.coreboot.org/c/coreboot/+/79946 Tested-by: build bot (Jenkins) Reviewed-by: Julius Werner --- src/commonlib/include/commonlib/region.h | 12 ++++++------ src/commonlib/region.c | 4 ++-- src/cpu/x86/smm/smm_module_loader.c | 6 +++--- src/drivers/spi/winbond.c | 6 +++--- 4 files changed, 14 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/commonlib/include/commonlib/region.h b/src/commonlib/include/commonlib/region.h index 84717979e7..e066ea0eb6 100644 --- a/src/commonlib/include/commonlib/region.h +++ b/src/commonlib/include/commonlib/region.h @@ -124,15 +124,15 @@ static inline size_t region_sz(const struct region *r) return r->size; } -static inline size_t region_end(const struct region *r) +static inline size_t region_last(const struct region *r) { - return region_offset(r) + region_sz(r); + return region_offset(r) + (region_sz(r) - 1); } static inline bool region_overlap(const struct region *r1, const struct region *r2) { - return (region_end(r1) > region_offset(r2)) && - (region_offset(r1) < region_end(r2)); + return (region_last(r1) >= region_offset(r2)) && + (region_offset(r1) <= region_last(r2)); } /* Helper to dynamically initialize region device. */ @@ -156,9 +156,9 @@ static inline size_t region_device_offset(const struct region_device *rdev) return region_offset(region_device_region(rdev)); } -static inline size_t region_device_end(const struct region_device *rdev) +static inline size_t region_device_last(const struct region_device *rdev) { - return region_end(region_device_region(rdev)); + return region_last(region_device_region(rdev)); } /* Memory map entire region device. Same semantics as rdev_mmap() above. */ diff --git a/src/commonlib/region.c b/src/commonlib/region.c index 4153f0a47d..fbc6276e96 100644 --- a/src/commonlib/region.c +++ b/src/commonlib/region.c @@ -10,10 +10,10 @@ int region_is_subregion(const struct region *p, const struct region *c) if (region_offset(c) < region_offset(p)) return 0; - if (region_end(c) > region_end(p)) + if (region_last(c) > region_last(p)) return 0; - if (region_end(c) < region_offset(c)) + if (region_last(c) < region_offset(c)) return 0; return 1; diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 979fa485ab..c6d0753c6b 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -341,7 +341,7 @@ static void setup_smihandler_params(struct smm_runtime *mod_params, } for (int i = 0; i < loader_params->num_cpus; i++) - mod_params->save_state_top[i] = region_end(&cpus[i].ss); + mod_params->save_state_top[i] = region_last(&cpus[i].ss) + 1; if (CONFIG(RUNTIME_CONFIGURABLE_SMM_LOGLEVEL)) mod_params->smm_log_level = mainboard_set_smm_log_level(); @@ -371,7 +371,7 @@ static void setup_smihandler_params(struct smm_runtime *mod_params, static void print_region(const char *name, const struct region region) { printk(BIOS_DEBUG, "%-12s [0x%zx-0x%zx]\n", name, region_offset(®ion), - region_end(®ion)); + region_last(®ion)); } /* STM + Handler + (Stub + Save state) * CONFIG_MAX_CPUS + stacks + page tables*/ @@ -482,7 +482,7 @@ int smm_load_module(const uintptr_t smram_base, const size_t smram_size, return -1; const struct region smram = region_create(smram_base, smram_size); - const uintptr_t smram_top = region_end(&smram); + const uintptr_t smram_top = region_last(&smram) + 1; const size_t stm_size = CONFIG(STM) ? CONFIG_MSEG_SIZE + CONFIG_BIOS_RESOURCE_LIST_SIZE : 0; diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index 3426d08e70..db93ff616c 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -374,7 +374,7 @@ static int winbond_get_write_protection(const struct spi_flash *flash, } printk(BIOS_DEBUG, "WINBOND: flash protected range 0x%08zx-0x%08zx\n", - region_offset(&wp_region), region_end(&wp_region)); + region_offset(&wp_region), region_last(&wp_region)); return region_is_subregion(&wp_region, region); } @@ -502,7 +502,7 @@ winbond_set_write_protection(const struct spi_flash *flash, int ret; /* Need to touch TOP or BOTTOM */ - if (region_offset(region) != 0 && region_end(region) != flash->size) + if (region_offset(region) != 0 && region_last(region) != flash->size - 1) return -1; params = flash->part; @@ -600,7 +600,7 @@ winbond_set_write_protection(const struct spi_flash *flash, return ret; printk(BIOS_DEBUG, "WINBOND: write-protection set to range " - "0x%08zx-0x%08zx\n", region_offset(region), region_end(region)); + "0x%08zx-0x%08zx\n", region_offset(region), region_last(region)); return ret; } -- cgit v1.2.3