The PMBus string registers (e.g. MFR_MODEL on the ADM1266) are writable
by the guest. pmbus_receive_block() can fill the destination field
completely, leaving no NUL terminator. When the value is later read back,
pmbus_send_string() calls strlen() on it, which reads past the end of the
array and returns a length that trips
g_assert(len + pmdev->out_buf_len < SMBUS_DATA_MAX_LEN);
aborting QEMU. This is guest-triggerable.
Add pmbus_receive_string(), the write-side mirror of pmbus_send_string(),
which reserves the last byte of the destination so the stored value is
always NUL-terminated, and use it for the ADM1266 MFR string registers.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3388
Signed-off-by: imaginos <[email protected]>
---
hw/i2c/pmbus_device.c | 13 +++++++++++++
hw/sensor/adm1266.c | 7 +++----
include/hw/i2c/pmbus_device.h | 10 ++++++++++
3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index b1f9843f52..7c65d8c8b5 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -142,6 +142,19 @@ uint8_t pmbus_receive_block(PMBusDevice *pmdev, uint8_t
*dest, size_t len)
return len;
}
+uint8_t pmbus_receive_string(PMBusDevice *pmdev, char *dest, size_t len)
+{
+ uint8_t rv;
+
+ if (len == 0) {
+ return 0;
+ }
+
+ rv = pmbus_receive_block(pmdev, (uint8_t *)dest, len - 1);
+ dest[len - 1] = '\0';
+ return rv;
+}
+
static uint64_t pmbus_receive_uint(PMBusDevice *pmdev)
{
diff --git a/hw/sensor/adm1266.c b/hw/sensor/adm1266.c
index 37d1cffd57..81545385a5 100644
--- a/hw/sensor/adm1266.c
+++ b/hw/sensor/adm1266.c
@@ -142,16 +142,15 @@ static int adm1266_write_data(PMBusDevice *pmdev, const
uint8_t *buf,
switch (pmdev->code) {
case PMBUS_MFR_ID: /* R/W block */
- pmbus_receive_block(pmdev, (uint8_t *)s->mfr_id, sizeof(s->mfr_id));
+ pmbus_receive_string(pmdev, s->mfr_id, sizeof(s->mfr_id));
break;
case PMBUS_MFR_MODEL: /* R/W block */
- pmbus_receive_block(pmdev, (uint8_t *)s->mfr_model,
- sizeof(s->mfr_model));
+ pmbus_receive_string(pmdev, s->mfr_model, sizeof(s->mfr_model));
break;
case PMBUS_MFR_REVISION: /* R/W block*/
- pmbus_receive_block(pmdev, (uint8_t *)s->mfr_rev, sizeof(s->mfr_rev));
+ pmbus_receive_string(pmdev, s->mfr_rev, sizeof(s->mfr_rev));
break;
case ADM1266_SET_RTC: /* do nothing */
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index f195c11384..4c4d3e5ae2 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -518,6 +518,16 @@ void pmbus_send_string(PMBusDevice *state, const char
*data);
*/
uint8_t pmbus_receive_block(PMBusDevice *pmdev, uint8_t *dest, size_t len);
+/**
+ * @brief Receive a Block Write and store it as a NUL-terminated string.
+ * Write-side mirror of pmbus_send_string(): guarantees dest is always a valid
+ * C string so later reads cannot run past the field. Use for string registers
+ * (e.g. MFR_ID, MFR_MODEL); use pmbus_receive_block() for binary blocks.
+ * @param dest - string buffer with enough capacity to receive the write
+ * @param len - the capacity of dest (the last byte is reserved for the NUL)
+ */
+uint8_t pmbus_receive_string(PMBusDevice *pmdev, char *dest, size_t len);
+
/**
* @brief Receive data over PMBus
* These methods help track how much data is being received over PMBus
--
2.43.0
