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>
(cherry picked from commit ff36712da5ae73aca5a044fe5e61c585d427013a)
(Mjt: actual Fixes: tag is this one)
Fixes: 9f58dd0a8c30 ("hw/misc/aspeed_sbc: Connect ASPEED OTP memory device to SBC")
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
diff --git a/hw/misc/aspeed_sbc.c b/hw/misc/aspeed_sbc.c
index 2fc5db749d..db92364fb8 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 dcf8ed3917..605e96139a 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.47.3