hw/cxl: Check mailbox input length before copying payload

[PATCH] hw/cxl: Check mailbox input length before copying payload

Jia Jia posted 1 patch 3 weeks, 2 days ago

Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260507113011.3674785-1-physicalmtea@gmail.com

Maintainers: Jonathan Cameron <jonathan.cameron@huawei.com>, Fan Ni <fan.ni@samsung.com>

hw/cxl/cxl-device-utils.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)

Expand all Fold all

[PATCH] hw/cxl: Check mailbox input length before copying payload

Posted by Jia Jia 3 weeks, 2 days ago

mailbox_reg_write() copies the guest mailbox payload with g_memdup2(pl,
len_in) before checking whether len_in exceeds cci->payload_max. len_in
comes straight from CXL_DEV_MAILBOX_CMD.LENGTH, so a guest can make QEMU
read past the mailbox payload buffer before the command is dispatched.

A cxl-switch-mailbox-cci reproducer trips ASan with:

  ERROR: AddressSanitizer: heap-buffer-overflow
  READ of size 1048575
      #0 __interceptor_memcpy
      #1 g_memdup2
      #2 g_memdup2_qemu ../include/glib-compat.h:99
      #3 mailbox_reg_write ../hw/cxl/cxl-device-utils.c:205

Reject requests larger than cci->payload_max before copying the payload
and return CXL_MBOX_INVALID_PAYLOAD_LENGTH instead.

Fixes: c9460561ed ("hw/cxl/mbox: Generalize the CCI command processing")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3478
Signed-off-by: Jia Jia <physicalmtea@gmail.com>

---
 hw/cxl/cxl-device-utils.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
index e150d74457..6560fb146d 100644
--- a/hw/cxl/cxl-device-utils.c
+++ b/hw/cxl/cxl-device-utils.c
@@ -202,14 +202,19 @@ static void mailbox_reg_write(void *opaque, hwaddr offset, uint64_t value,
         bool bg_started = false;
         int rc;
 
-        pl_in_copy = g_memdup2(pl, len_in);
-        if (len_in == 0 || pl_in_copy) {
-            /* Avoid stale data  - including from earlier cmds */
-            memset(pl, 0, CXL_MAILBOX_MAX_PAYLOAD_SIZE);
-            rc = cxl_process_cci_message(cci, cmd_set, cmd, len_in, pl_in_copy,
-                                         &len_out, pl, &bg_started);
+        if (len_in > cci->payload_max) {
+            rc = CXL_MBOX_INVALID_PAYLOAD_LENGTH;
         } else {
-            rc = CXL_MBOX_INTERNAL_ERROR;
+            pl_in_copy = g_memdup2(pl, len_in);
+            if (len_in == 0 || pl_in_copy) {
+                /* Avoid stale data  - including from earlier cmds */
+                memset(pl, 0, CXL_MAILBOX_MAX_PAYLOAD_SIZE);
+                rc = cxl_process_cci_message(cci, cmd_set, cmd, len_in,
+                                             pl_in_copy, &len_out, pl,
+                                             &bg_started);
+            } else {
+                rc = CXL_MBOX_INTERNAL_ERROR;
+            }
         }
 
         /* Set bg and the return code */
-- 
2.39.5