[PULL,2/9] hw/i2c: pmbus: fix error returns and guard against out of range accesses

Commit Message

Philippe Mathieu-Daudé March 8, 2022, 6:13 p.m. UTC

From: Titus Rwantare <[e-mail]>

Signed-off-by: Titus Rwantare <[e-mail]>
Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>
Acked-by: Corey Minyard <[e-mail]>
Message-Id: <[e-mail]>
Signed-off-by: Philippe Mathieu-Daudé <[e-mail]>
---
 hw/i2c/pmbus_device.c         | 47 ++++++++++++++++++++++++++++++++---
 include/hw/i2c/pmbus_device.h |  2 ++
 2 files changed, 45 insertions(+), 4 deletions(-)

Patch

diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 07a45c99f9..c7ec8e5499 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -149,7 +149,7 @@  static uint8_t pmbus_out_buf_pop(PMBusDevice *pmdev)
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: tried to read from empty buffer",
                       __func__);
-        return 0xFF;
+        return PMBUS_ERR_BYTE;
     }
     uint8_t data = pmdev->out_buf[pmdev->out_buf_len - 1];
     pmdev->out_buf_len--;
@@ -243,18 +243,47 @@  void pmbus_check_limits(PMBusDevice *pmdev)
     }
 }
 
+/* assert the status_cml error upon receipt of malformed command */
+static void pmbus_cml_error(PMBusDevice *pmdev)
+{
+    for (int i = 0; i < pmdev->num_pages; i++) {
+        pmdev->pages[i].status_word |= PMBUS_STATUS_CML;
+        pmdev->pages[i].status_cml |= PB_CML_FAULT_INVALID_CMD;
+    }
+}
+
 static uint8_t pmbus_receive_byte(SMBusDevice *smd)
 {
     PMBusDevice *pmdev = PMBUS_DEVICE(smd);
     PMBusDeviceClass *pmdc = PMBUS_DEVICE_GET_CLASS(pmdev);
-    uint8_t ret = 0xFF;
-    uint8_t index = pmdev->page;
+    uint8_t ret = PMBUS_ERR_BYTE;
+    uint8_t index;
 
     if (pmdev->out_buf_len != 0) {
         ret = pmbus_out_buf_pop(pmdev);
         return ret;
     }
 
+    /*
+     * Reading from all pages will return the value from page 0,
+     * this is unspecified behaviour in general.
+     */
+    if (pmdev->page == PB_ALL_PAGES) {
+        index = 0;
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: tried to read from all pages\n",
+                      __func__);
+        pmbus_cml_error(pmdev);
+    } else if (pmdev->page > pmdev->num_pages - 1) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: page %d is out of range\n",
+                      __func__, pmdev->page);
+        pmbus_cml_error(pmdev);
+        return PMBUS_ERR_BYTE;
+    } else {
+        index = pmdev->page;
+    }
+
     switch (pmdev->code) {
     case PMBUS_PAGE:
         pmbus_send8(pmdev, pmdev->page);
@@ -1019,7 +1048,7 @@  static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len)
 
     if (len == 0) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: writing empty data\n", __func__);
-        return -1;
+        return PMBUS_ERR_BYTE;
     }
 
     if (!pmdev->pages) { /* allocate memory for pages on first use */
@@ -1038,6 +1067,7 @@  static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len)
         pmdev->page = pmbus_receive8(pmdev);
         return 0;
     }
+
     /* loop through all the pages when 0xFF is received */
     if (pmdev->page == PB_ALL_PAGES) {
         for (int i = 0; i < pmdev->num_pages; i++) {
@@ -1048,6 +1078,15 @@  static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len)
         return 0;
     }
 
+    if (pmdev->page > pmdev->num_pages - 1) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                        "%s: page %u is out of range\n",
+                        __func__, pmdev->page);
+        pmdev->page = 0; /* undefined behaviour - reset to page 0 */
+        pmbus_cml_error(pmdev);
+        return PMBUS_ERR_BYTE;
+    }
+
     index = pmdev->page;
 
     switch (pmdev->code) {
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index 72c0483149..bab4526734 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -228,6 +228,8 @@  enum pmbus_registers {
 #define PB_MAX_PAGES            0x1F
 #define PB_ALL_PAGES            0xFF
 
+#define PMBUS_ERR_BYTE          0xFF
+
 #define TYPE_PMBUS_DEVICE "pmbus-device"
 OBJECT_DECLARE_TYPE(PMBusDevice, PMBusDeviceClass,
                     PMBUS_DEVICE)