From 6f87a5d393e15b9a47c9c4bb1e8e153878882c0c Mon Sep 17 00:00:00 2001 From: Zheyu Ma Date: Tue, 18 Jun 2024 15:09:28 +0200 Subject: [PATCH] hw/gpio/aspeed: Add bounds checking for register table access Added bounds checking in the aspeed_gpio_read() and aspeed_gpio_write() functions to ensure the index idx is within the valid range of the reg_table array. The correct size of reg_table is determined dynamically based on whether it is aspeed_3_3v_gpios or aspeed_1_8v_gpios. If idx exceeds the size of reg_table, an error is logged, and the function returns. AddressSanitizer log indicating the issue: ==2602930==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55a5da29e128 at pc 0x55a5d700dc62 bp 0x7fff096c4e90 sp 0x7fff096c4e88 READ of size 2 at 0x55a5da29e128 thread T0 #0 0x55a5d700dc61 in aspeed_gpio_read hw/gpio/aspeed_gpio.c:564:14 #1 0x55a5d933f3ab in memory_region_read_accessor system/memory.c:445:11 #2 0x55a5d92fba40 in access_with_adjusted_size system/memory.c:573:18 #3 0x55a5d92f842c in memory_region_dispatch_read1 system/memory.c:1426:16 #4 0x55a5d92f7b68 in memory_region_dispatch_read system/memory.c:1459:9 #5 0x55a5d9376ad1 in flatview_read_continue_step system/physmem.c:2836:18 #6 0x55a5d9376399 in flatview_read_continue system/physmem.c:2877:19 #7 0x55a5d93775b8 in flatview_read system/physmem.c:2907:12 Signed-off-by: Zheyu Ma Message-Id: <20240618130928.3075494-1-zheyuma97@gmail.com> --- hw/gpio/aspeed_gpio.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c index c1781e2ba36f..1441046f6c43 100644 --- a/hw/gpio/aspeed_gpio.c +++ b/hw/gpio/aspeed_gpio.c @@ -550,6 +550,7 @@ static uint64_t aspeed_gpio_read(void *opaque, hwaddr offset, uint32_t size) GPIOSets *set; uint32_t value = 0; uint64_t debounce_value; + uint32_t reg_table_size; idx = offset >> 2; if (idx >= GPIO_DEBOUNCE_TIME_1 && idx <= GPIO_DEBOUNCE_TIME_3) { @@ -559,6 +560,18 @@ static uint64_t aspeed_gpio_read(void *opaque, hwaddr offset, uint32_t size) return debounce_value; } + if (agc->reg_table == aspeed_3_3v_gpios) { + reg_table_size = GPIO_3_3V_REG_ARRAY_SIZE; + } else { + reg_table_size = GPIO_1_8V_REG_ARRAY_SIZE; + } + + if (idx >= reg_table_size) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: idx 0x%" PRIx64 " out of bounds\n", + __func__, idx); + return 0; + } + reg = &agc->reg_table[idx]; if (reg->set_idx >= agc->nr_gpio_sets) { qemu_log_mask(LOG_GUEST_ERROR, "%s: no getter for offset 0x%" @@ -768,6 +781,7 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data, const AspeedGPIOReg *reg; GPIOSets *set; uint32_t cleared; + uint32_t reg_table_size; trace_aspeed_gpio_write(offset, data); @@ -785,6 +799,18 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data, return; } + if (agc->reg_table == aspeed_3_3v_gpios) { + reg_table_size = GPIO_3_3V_REG_ARRAY_SIZE; + } else { + reg_table_size = GPIO_1_8V_REG_ARRAY_SIZE; + } + + if (idx >= reg_table_size) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: idx 0x%" PRIx64 " out of bounds\n", + __func__, idx); + return; + } + reg = &agc->reg_table[idx]; if (reg->set_idx >= agc->nr_gpio_sets) { qemu_log_mask(LOG_GUEST_ERROR, "%s: no setter for offset 0x%"