From: "Cédric Le Goater" <clg@redhat.com>
To: qemu-arm@nongnu.org, qemu-devel@nongnu.org
Cc: "Kane Chen" <kane_chen@aspeedtech.com>,
qemu-stable@nongnu.org,
"Peter Maydell" <peter.maydell@linaro.org>,
"Cédric Le Goater" <clg@redhat.com>
Subject: [PULL 1/9] hw/misc/aspeed_sbc: Add bounds checking for OTP write operations
Date: Tue, 12 May 2026 19:13:46 +0200 [thread overview]
Message-ID: <20260512171354.4183887-2-clg@redhat.com> (raw)
In-Reply-To: <20260512171354.4183887-1-clg@redhat.com>
From: Kane Chen <kane_chen@aspeedtech.com>
There is a mismatch between the Aspeed OTP model and the Aspeed SBC
model in how the guest-provided address is handled.
aspeed_sbc_otp_prog() passes a word-indexed address directly
to address_space_write() without converting it to a byte offset,
whereas aspeed_otp_write() expects a byte offset and applies an
additional shift (otp_addr << 2). This double-shift confusion means
that an out-of-range word address can lead to a write beyond the
allocated storage.
Fix this by adding bounds checking on the word offset before
converting to byte offset and passing to address_space_write().
This matches the existing bounds check in aspeed_sbc_otp_read().
Cc: Kane-Chen-AS <kane_chen@aspeedtech.com>
Cc: qemu-stable@nongnu.org
Fixes: 1a00754ccf15 ("hw/misc: Add Aspeed Secure Boot Controller model")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3436
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Kane-Chen-AS <kane_chen@aspeedtech.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Link: https://lore.kernel.org/qemu-devel/20260428055254.76581-2-kane_chen@aspeedtech.com
[ clg: Kept otp_addr in event logged in aspeed_sbc_otp_prog() ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/misc/aspeed_sbc.c | 12 ++++++++++--
hw/nvram/aspeed_otp.c | 13 ++++++-------
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/hw/misc/aspeed_sbc.c b/hw/misc/aspeed_sbc.c
index 065e822e70d9..e5dab1c7bb7c 100644
--- a/hw/misc/aspeed_sbc.c
+++ b/hw/misc/aspeed_sbc.c
@@ -159,9 +159,17 @@ static bool aspeed_sbc_otp_prog(AspeedSBCState *s,
MemTxResult ret;
AspeedOTPState *otp = &s->otp;
uint32_t value = s->regs[R_CAMP1];
+ uint32_t otp_offset = otp_addr << 2;
- ret = address_space_write(&otp->as, otp_addr, MEMTXATTRS_UNSPECIFIED,
- &value, sizeof(value));
+ if (otp_addr >= OTP_TOTAL_DWORD_COUNT) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Invalid OTP addr 0x%x\n",
+ otp_addr);
+ return false;
+ }
+
+ ret = address_space_write(&otp->as, otp_offset, MEMTXATTRS_UNSPECIFIED,
+ &value, sizeof(value));
if (ret != MEMTX_OK) {
qemu_log_mask(LOG_GUEST_ERROR,
"Failed to write OTP memory, addr = %x\n",
diff --git a/hw/nvram/aspeed_otp.c b/hw/nvram/aspeed_otp.c
index a60289000c37..1a9d3841b8d6 100644
--- a/hw/nvram/aspeed_otp.c
+++ b/hw/nvram/aspeed_otp.c
@@ -57,12 +57,12 @@ static bool valid_program_data(uint32_t otp_addr,
return has_programmable_bits != 0;
}
-static bool program_otpmem_data(void *opaque, uint32_t otp_addr,
+static bool program_otpmem_data(void *opaque, hwaddr otp_offset,
uint32_t prog_bit, uint32_t *value)
{
AspeedOTPState *s = opaque;
+ uint32_t otp_addr = otp_offset >> 2;
bool is_odd = otp_addr & 1;
- uint32_t otp_offset = otp_addr << 2;
memcpy(value, s->storage + otp_offset, sizeof(uint32_t));
@@ -79,26 +79,25 @@ static bool program_otpmem_data(void *opaque, uint32_t otp_addr,
return true;
}
-static void aspeed_otp_write(void *opaque, hwaddr otp_addr,
+static void aspeed_otp_write(void *opaque, hwaddr otp_offset,
uint64_t val, unsigned size)
{
AspeedOTPState *s = opaque;
- uint32_t otp_offset, value;
+ uint32_t value;
- if (!program_otpmem_data(s, otp_addr, val, &value)) {
+ if (!program_otpmem_data(s, otp_offset, val, &value)) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: Failed to program data, value = %x, bit = %"PRIx64"\n",
__func__, value, val);
return;
}
- otp_offset = otp_addr << 2;
memcpy(s->storage + otp_offset, &value, size);
if (s->blk) {
if (blk_pwrite(s->blk, otp_offset, size, &value, 0) < 0) {
qemu_log_mask(LOG_GUEST_ERROR,
- "%s: Failed to write %x to %x\n",
+ "%s: Failed to write %x to %"HWADDR_PRIx"\n",
__func__, value, otp_offset);
return;
--
2.54.0
next prev parent reply other threads:[~2026-05-12 17:14 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-12 17:13 [PULL 0/9] aspeed queue Cédric Le Goater
2026-05-12 17:13 ` Cédric Le Goater [this message]
2026-05-12 17:13 ` [PULL 2/9] aspeed/hace: Fix out-of-bounds read in has_padding() Cédric Le Goater
2026-05-12 17:13 ` [PULL 3/9] aspeed/hace: Prevent total_req_len overflow Cédric Le Goater
2026-05-12 17:13 ` [PULL 4/9] aspeed/hace: Fix mapped address may not be unmapped issue Cédric Le Goater
2026-05-12 17:13 ` [PULL 5/9] hw/arm: Remove sonorapass-bmc machine Cédric Le Goater
2026-05-12 17:13 ` [PULL 6/9] hw/arm: Remove qcom-dc-scm-v1-bmc and qcom-firework-bmc machines Cédric Le Goater
2026-05-12 17:13 ` [PULL 7/9] hw/arm: Remove fp5280g2-bmc machine Cédric Le Goater
2026-05-12 17:13 ` [PULL 8/9] hw/arm: Remove fby35 machine Cédric Le Goater
2026-05-12 17:13 ` [PULL 9/9] hw/i3c/dw-i3c: Fix BCR/DCR extraction and PID assembly during ENTDAA Cédric Le Goater
2026-05-14 16:26 ` [PULL 0/9] aspeed queue Stefan Hajnoczi
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=20260512171354.4183887-2-clg@redhat.com \
--to=clg@redhat.com \
--cc=kane_chen@aspeedtech.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-stable@nongnu.org \
/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.