On 2/9/26 02:45, Jamin Lin wrote:
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
The ASPEED_I2C_NEW_NUM_REG value correctly covers the supported range
of registers of both 'old' and 'new' implementations.
Instead of dynamically allocating the regs array, which introduces
vmstate complexity, I would fix the way the array is accessed.
In aspeed_i2c_bus_old_read(), change :
uint64_t value = bus->regs[offset / sizeof(*bus->regs)];
to
uint64_t value = -1;
and adjust how 'value' is assigned else where.
Same for aspeed_i2c_bus_new_read().
If you want to keep the dynamic allocation, the vmstate needs a fix.
May be introduce a 'regs_nr' attribute for that and check how other
models save/load dynamic arrays.
Thanks,
C.
