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 <jamin_lin@aspeedtech.com>
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