@samitouri / QOSamiQemu / commits / ff36712da5

hw/misc/aspeed_sbc: Add bounds checking for OTP write operations

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>

Kane Chen committed Apr 28, 2026 at 05:52 UTC ff36712da5ae73aca5a044fe5e61c585d427013a
2 files changed +16 -9
hw/misc/aspeed_sbc.c
+10 -2
@@ -159,9 +159,17 @@ static bool aspeed_sbc_otp_prog(AspeedSBCState *s,
159 MemTxResult ret;
160 AspeedOTPState *otp = &s->otp;
161 uint32_t value = s->regs[R_CAMP1];
162 + uint32_t otp_offset = otp_addr << 2;
163
163 - ret = address_space_write(&otp->as, otp_addr, MEMTXATTRS_UNSPECIFIED,
164 - &value, sizeof(value));
164 + if (otp_addr >= OTP_TOTAL_DWORD_COUNT) {
165 + qemu_log_mask(LOG_GUEST_ERROR,
166 + "Invalid OTP addr 0x%x\n",
167 + otp_addr);
168 + return false;
169 + }
170 +
171 + ret = address_space_write(&otp->as, otp_offset, MEMTXATTRS_UNSPECIFIED,
172 + &value, sizeof(value));
173 if (ret != MEMTX_OK) {
174 qemu_log_mask(LOG_GUEST_ERROR,
175 "Failed to write OTP memory, addr = %x\n",
hw/nvram/aspeed_otp.c
+6 -7
@@ -57,12 +57,12 @@ static bool valid_program_data(uint32_t otp_addr,
57 return has_programmable_bits != 0;
58 }
59
60 -static bool program_otpmem_data(void *opaque, uint32_t otp_addr,
60 +static bool program_otpmem_data(void *opaque, hwaddr otp_offset,
61 uint32_t prog_bit, uint32_t *value)
62 {
63 AspeedOTPState *s = opaque;
64 + uint32_t otp_addr = otp_offset >> 2;
65 bool is_odd = otp_addr & 1;
65 - uint32_t otp_offset = otp_addr << 2;
66
67 memcpy(value, s->storage + otp_offset, sizeof(uint32_t));
68
@@ -79,26 +79,25 @@ static bool program_otpmem_data(void *opaque, uint32_t otp_addr,
79 return true;
80 }
81
82 -static void aspeed_otp_write(void *opaque, hwaddr otp_addr,
82 +static void aspeed_otp_write(void *opaque, hwaddr otp_offset,
83 uint64_t val, unsigned size)
84 {
85 AspeedOTPState *s = opaque;
86 - uint32_t otp_offset, value;
86 + uint32_t value;
87
88 - if (!program_otpmem_data(s, otp_addr, val, &value)) {
88 + if (!program_otpmem_data(s, otp_offset, val, &value)) {
89 qemu_log_mask(LOG_GUEST_ERROR,
90 "%s: Failed to program data, value = %x, bit = %"PRIx64"\n",
91 __func__, value, val);
92 return;
93 }
94
95 - otp_offset = otp_addr << 2;
95 memcpy(s->storage + otp_offset, &value, size);
96
97 if (s->blk) {
98 if (blk_pwrite(s->blk, otp_offset, size, &value, 0) < 0) {
99 qemu_log_mask(LOG_GUEST_ERROR,
101 - "%s: Failed to write %x to %x\n",
100 + "%s: Failed to write %x to %"HWADDR_PRIx"\n",
101 __func__, value, otp_offset);
102
103 return;