The ASPEED I2C controller emulation used a fixed-size register array (28 dwords) for all SoC variants, while multiple ASPEED SoCs (AST2600, AST1030, AST2700) expose a larger MMIO register window (e.g. reg_size = 0x80).
This mismatch allows MMIO accesses beyond the allocated register array, leading to out-of-bounds reads in the I2C controller model. Fix this by converting the register storage to a dynamically allocated array sized according to the controller class reg_size. The register array is now allocated during bus realize and free on unrealize, ensuring safe access across different ASPEED SoC implementations. This change eliminates I2C register out-of-bounds access caused by SoC-specific register size differences. Signed-off-by: Jamin Lin <[email protected]> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3290 --- include/hw/i2c/aspeed_i2c.h | 4 +--- hw/i2c/aspeed_i2c.c | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index 68bd138026..205f0a58d2 100644 --- a/include/hw/i2c/aspeed_i2c.h +++ b/include/hw/i2c/aspeed_i2c.h @@ -36,8 +36,6 @@ OBJECT_DECLARE_TYPE(AspeedI2CState, AspeedI2CClass, ASPEED_I2C) #define ASPEED_I2C_NR_BUSSES 16 #define ASPEED_I2C_SHARE_POOL_SIZE 0x800 #define ASPEED_I2C_BUS_POOL_SIZE 0x20 -#define ASPEED_I2C_OLD_NUM_REG 11 -#define ASPEED_I2C_NEW_NUM_REG 28 #define A_I2CD_M_STOP_CMD BIT(5) #define A_I2CD_M_RX_CMD BIT(3) @@ -256,7 +254,7 @@ struct AspeedI2CBus { uint8_t id; qemu_irq irq; - uint32_t regs[ASPEED_I2C_NEW_NUM_REG]; + uint32_t *regs; uint8_t pool[ASPEED_I2C_BUS_POOL_SIZE]; uint64_t dma_dram_offset; }; diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index fb3d6a5600..cf3a003978 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -1091,10 +1091,9 @@ static const MemoryRegionOps aspeed_i2c_bus_pool_ops = { static const VMStateDescription aspeed_i2c_bus_vmstate = { .name = TYPE_ASPEED_I2C, - .version_id = 6, - .minimum_version_id = 6, + .version_id = 7, + .minimum_version_id = 7, .fields = (const VMStateField[]) { - VMSTATE_UINT32_ARRAY(regs, AspeedI2CBus, ASPEED_I2C_NEW_NUM_REG), VMSTATE_UINT8_ARRAY(pool, AspeedI2CBus, ASPEED_I2C_BUS_POOL_SIZE), VMSTATE_UINT64(dma_dram_offset, AspeedI2CBus), VMSTATE_END_OF_LIST() @@ -1465,8 +1464,9 @@ static const TypeInfo aspeed_i2c_bus_slave_info = { static void aspeed_i2c_bus_reset(DeviceState *dev) { AspeedI2CBus *s = ASPEED_I2C_BUS(dev); + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(s->controller); - memset(s->regs, 0, sizeof(s->regs)); + memset(s->regs, 0, aic->reg_size); i2c_end_transfer(s->bus); } @@ -1492,6 +1492,7 @@ static void aspeed_i2c_bus_realize(DeviceState *dev, Error **errp) s->slave = i2c_slave_create_simple(s->bus, TYPE_ASPEED_I2C_BUS_SLAVE, 0xff); + s->regs = g_new(uint32_t, aic->reg_size >> 2); memory_region_init_io(&s->mr, OBJECT(s), &aspeed_i2c_bus_ops, s, s->name, aic->reg_size); sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mr); @@ -1501,6 +1502,14 @@ static void aspeed_i2c_bus_realize(DeviceState *dev, Error **errp) sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mr_pool); } +static void aspeed_i2c_bus_unrealize(DeviceState *dev) +{ + AspeedI2CBus *s = ASPEED_I2C_BUS(dev); + + g_free(s->regs); + s->regs = NULL; +} + static const Property aspeed_i2c_bus_properties[] = { DEFINE_PROP_UINT8("bus-id", AspeedI2CBus, id, 0), DEFINE_PROP_LINK("controller", AspeedI2CBus, controller, TYPE_ASPEED_I2C, @@ -1514,6 +1523,7 @@ static void aspeed_i2c_bus_class_init(ObjectClass *klass, const void *data) dc->desc = "Aspeed I2C Bus"; dc->realize = aspeed_i2c_bus_realize; + dc->unrealize = aspeed_i2c_bus_unrealize; device_class_set_legacy_reset(dc, aspeed_i2c_bus_reset); device_class_set_props(dc, aspeed_i2c_bus_properties); } -- 2.43.0
