All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jamin Lin <jamin_lin@aspeedtech.com>
To: "Cédric Le Goater" <clg@kaod.org>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Steven Lee" <steven_lee@aspeedtech.com>,
	"Troy Lee" <leetroy@gmail.com>,
	"Andrew Jeffery" <andrew@codeconstruct.com.au>,
	"Joel Stanley" <joel@jms.id.au>,
	"Fabiano Rosas" <farosas@suse.de>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"open list:ASPEED BMCs" <qemu-arm@nongnu.org>,
	"open list:All patches CC here" <qemu-devel@nongnu.org>
Cc: Jamin Lin <jamin_lin@aspeedtech.com>,
	Troy Lee <troy_lee@aspeedtech.com>,
	 Kane Chen <kane_chen@aspeedtech.com>,
	"nabihestefan@google.com" <nabihestefan@google.com>
Subject: [PATCH v1 01/10] hw/i2c/aspeed_i2c: Fix Out-of-Bounds access by using dynamic register array
Date: Fri, 6 Feb 2026 05:33:42 +0000	[thread overview]
Message-ID: <20260206053340.3716041-2-jamin_lin@aspeedtech.com> (raw)
In-Reply-To: <20260206053340.3716041-1-jamin_lin@aspeedtech.com>

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


  reply	other threads:[~2026-02-06  5:34 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-06  5:33 [PATCH v1 00/10] Support ASPEED AST2700 A2 Jamin Lin
2026-02-06  5:33 ` Jamin Lin [this message]
2026-02-06 10:29   ` [PATCH v1 01/10] hw/i2c/aspeed_i2c: Fix Out-of-Bounds access by using dynamic register array Cédric Le Goater
2026-02-09  1:31     ` Jamin Lin
2026-02-06  5:33 ` [PATCH v1 1/1] tests/functional/aarch64/test_aspeed_ast2700fc: Use AST2700 A2 SDK image for FC tests Jamin Lin
2026-02-06  9:31   ` Cédric Le Goater
2026-02-06  9:37     ` Jamin Lin
2026-02-06  5:33 ` [PATCH v1 02/10] hw/i2c/aspeed_i2c: Increase I2C device register size to 0xA0 Jamin Lin
2026-02-06  9:36   ` Cédric Le Goater
2026-02-09  2:02     ` Jamin Lin
2026-02-09  6:51       ` Cédric Le Goater
2026-02-09  6:53         ` Jamin Lin
2026-02-06  5:33 ` [PATCH v1 03/10] hw/misc/aspeed_scu: Remove unused SoC silicon revision definitions Jamin Lin
2026-02-06  9:36   ` Cédric Le Goater
2026-02-06  5:33 ` [PATCH v1 04/10] hw/misc/aspeed_scu: Add AST2700 A2 silicon revisions Jamin Lin
2026-02-06  9:36   ` Cédric Le Goater
2026-02-06  5:33 ` [PATCH v1 05/10] hw/arm/aspeed_ast27x0: Add AST2700 A2 SoC support Jamin Lin
2026-02-06  9:37   ` Cédric Le Goater
2026-02-09  2:12     ` Jamin Lin
2026-02-06  5:33 ` [PATCH v1 06/10] hw/arm/aspeed_ast27x0_evb: Add AST2700 A2 EVB machine Jamin Lin
2026-02-06  9:38   ` Cédric Le Goater
2026-02-09  2:16     ` Jamin Lin
2026-02-06  5:33 ` [PATCH v1 07/10] tests/qtest/ast2700-hace-test: Use ast2700-evb alias for AST2700 HACE tests Jamin Lin
2026-02-06  9:41   ` Cédric Le Goater
2026-02-06  5:33 ` [PATCH v1 08/10] tests/functional/aarch64/test_aspeed_ast2700: Add AST2700 A2 EVB functional tests Jamin Lin
2026-02-06 10:11   ` Cédric Le Goater
2026-02-06 10:40     ` Thomas Huth
2026-02-06 11:33       ` Peter Maydell
2026-02-06 11:35         ` Thomas Huth
2026-02-09  2:40           ` Jamin Lin
2026-02-06  5:33 ` [PATCH v1 09/10] hw/arm/aspeed_ast27x0-fc: Switch AST2700 FC machine to A2 SoC Jamin Lin
2026-02-06 10:00   ` Cédric Le Goater
2026-02-09  2:31     ` Jamin Lin
2026-02-09  6:55       ` Cédric Le Goater
2026-02-06  5:33 ` [PATCH v1 10/10] tests/functional/aarch64/test_aspeed_ast2700fc: Use AST2700 A2 SDK image for FC tests Jamin Lin
2026-02-06 10:00   ` Cédric Le Goater
2026-02-06  9:34 ` [PATCH v1 00/10] Support ASPEED AST2700 A2 Cédric Le Goater
2026-02-09  3:26   ` Jamin Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260206053340.3716041-2-jamin_lin@aspeedtech.com \
    --to=jamin_lin@aspeedtech.com \
    --cc=andrew@codeconstruct.com.au \
    --cc=clg@kaod.org \
    --cc=farosas@suse.de \
    --cc=joel@jms.id.au \
    --cc=kane_chen@aspeedtech.com \
    --cc=leetroy@gmail.com \
    --cc=lvivier@redhat.com \
    --cc=nabihestefan@google.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=steven_lee@aspeedtech.com \
    --cc=troy_lee@aspeedtech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.