[PATCH v4 1/2] intel_iommu: fix guest-triggerable abort on oversized MMIO access

[Junjie Cao <junjie.cao@intel.com>]

An 8-byte guest access to a 32-bit-only VT-d register hit
assert(size == 4) and aborted QEMU.  Remove all 25 asserts.

All 3 read-side and 18 of 22 write-side asserts are at
non-8-aligned offsets (unreachable, rejected by
memory_region_access_valid()) -- simply deleted.

The remaining 4, all writes at 8-aligned offsets, are
reachable: FECTL 0x38, IECTL 0xa0, IEADDR 0xa8, PECTL 0xe0.
Truncating the high half via vtd_set_long() matches prior
behavior; log under -d guest_errors since the VT-d spec is
silent on oversized accesses to 32-bit registers, and add a
comment so future maintainers don't delete the check as
"harmless".  min_access_size stays 4, so all size-based
branches on 64-bit register pairs are preserved.

Found by generic-fuzz (24 distinct crash seeds, all fixed).

Suggested-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
---
 hw/i386/intel_iommu.c | 74 ++++++++++++++++++++++++++++---------------
 1 file changed, 49 insertions(+), 25 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index b784c5f10a..17f897177d 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -21,6 +21,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
+#include "qemu/log.h"
 #include "qemu/main-loop.h"
 #include "qapi/error.h"
 #include "hw/core/sysbus.h"
@@ -3713,7 +3714,6 @@ static uint64_t vtd_mem_read(void *opaque, hwaddr addr, unsigned size)
         break;
 
     case DMAR_RTADDR_REG_HI:
-        assert(size == 4);
         val = vtd_get_quad_raw(s, DMAR_RTADDR_REG) >> 32;
         break;
 
@@ -3728,12 +3728,10 @@ static uint64_t vtd_mem_read(void *opaque, hwaddr addr, unsigned size)
         break;
 
     case DMAR_IQA_REG_HI:
-        assert(size == 4);
         val = s->iq >> 32;
         break;
 
     case DMAR_PEUADDR_REG:
-        assert(size == 4);
         val = vtd_get_long_raw(s, DMAR_PEUADDR_REG);
         break;
 
@@ -3779,7 +3777,6 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
         break;
 
     case DMAR_CCMD_REG_HI:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         vtd_handle_ccmd_write(s);
         break;
@@ -3795,13 +3792,11 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
         break;
 
     case DMAR_IOTLB_REG_HI:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         vtd_handle_iotlb_write(s);
         break;
 
     case DMAR_PEUADDR_REG:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         break;
 
@@ -3815,27 +3810,35 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
         break;
 
     case DMAR_IVA_REG_HI:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         break;
 
     /* Fault Status Register, 32-bit */
     case DMAR_FSTS_REG:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         vtd_handle_fsts_write(s);
         break;
 
     /* Fault Event Control Register, 32-bit */
     case DMAR_FECTL_REG:
-        assert(size == 4);
+        /*
+         * 32-bit register at an 8-byte-aligned offset: a well-formed
+         * 8-byte guest access reaches this handler.  vtd_set_long()
+         * takes uint32_t and truncates the high half -- undefined per
+         * the VT-d spec but harmless here.  Flag it under
+         * -d guest_errors so the guest-side bug surfaces.
+         */
+        if (size != 4) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: invalid %u-byte access to 32-bit reg "
+                          "addr=0x%" PRIx64 "\n", __func__, size, addr);
+        }
         vtd_set_long(s, addr, val);
         vtd_handle_fectl_write(s);
         break;
 
     /* Fault Event Data Register, 32-bit */
     case DMAR_FEDATA_REG:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         break;
 
@@ -3854,13 +3857,11 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
 
     /* Fault Event Upper Address Register, 32-bit */
     case DMAR_FEUADDR_REG:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         break;
 
     /* Protected Memory Enable Register, 32-bit */
     case DMAR_PMEN_REG:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         break;
 
@@ -3874,7 +3875,6 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
         break;
 
     case DMAR_RTADDR_REG_HI:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         break;
 
@@ -3889,7 +3889,6 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
         break;
 
     case DMAR_IQT_REG_HI:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         /* 19:63 of IQT_REG is RsvdZ, do nothing here */
         break;
@@ -3905,39 +3904,57 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
         break;
 
     case DMAR_IQA_REG_HI:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         break;
 
     /* Invalidation Completion Status Register, 32-bit */
     case DMAR_ICS_REG:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         vtd_handle_ics_write(s);
         break;
 
     /* Invalidation Event Control Register, 32-bit */
     case DMAR_IECTL_REG:
-        assert(size == 4);
+        /*
+         * 32-bit register at an 8-byte-aligned offset: a well-formed
+         * 8-byte guest access reaches this handler.  vtd_set_long()
+         * takes uint32_t and truncates the high half -- undefined per
+         * the VT-d spec but harmless here.  Flag it under
+         * -d guest_errors so the guest-side bug surfaces.
+         */
+        if (size != 4) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: invalid %u-byte access to 32-bit reg "
+                          "addr=0x%" PRIx64 "\n", __func__, size, addr);
+        }
         vtd_set_long(s, addr, val);
         vtd_handle_iectl_write(s);
         break;
 
     /* Invalidation Event Data Register, 32-bit */
     case DMAR_IEDATA_REG:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         break;
 
     /* Invalidation Event Address Register, 32-bit */
     case DMAR_IEADDR_REG:
-        assert(size == 4);
+        /*
+         * 32-bit register at an 8-byte-aligned offset: a well-formed
+         * 8-byte guest access reaches this handler.  vtd_set_long()
+         * takes uint32_t and truncates the high half -- undefined per
+         * the VT-d spec but harmless here.  Flag it under
+         * -d guest_errors so the guest-side bug surfaces.
+         */
+        if (size != 4) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: invalid %u-byte access to 32-bit reg "
+                          "addr=0x%" PRIx64 "\n", __func__, size, addr);
+        }
         vtd_set_long(s, addr, val);
         break;
 
     /* Invalidation Event Upper Address Register, 32-bit */
     case DMAR_IEUADDR_REG:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         break;
 
@@ -3951,7 +3968,6 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
         break;
 
     case DMAR_FRCD_REG_0_1:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         break;
 
@@ -3966,7 +3982,6 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
         break;
 
     case DMAR_FRCD_REG_0_3:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         /* May clear bit 127 (Fault), update PPF */
         vtd_update_fsts_ppf(s);
@@ -3981,18 +3996,27 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
         break;
 
     case DMAR_IRTA_REG_HI:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         break;
 
     case DMAR_PRS_REG:
-        assert(size == 4);
         vtd_set_long(s, addr, val);
         vtd_handle_prs_write(s);
         break;
 
     case DMAR_PECTL_REG:
-        assert(size == 4);
+        /*
+         * 32-bit register at an 8-byte-aligned offset: a well-formed
+         * 8-byte guest access reaches this handler.  vtd_set_long()
+         * takes uint32_t and truncates the high half -- undefined per
+         * the VT-d spec but harmless here.  Flag it under
+         * -d guest_errors so the guest-side bug surfaces.
+         */
+        if (size != 4) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: invalid %u-byte access to 32-bit reg "
+                          "addr=0x%" PRIx64 "\n", __func__, size, addr);
+        }
         vtd_set_long(s, addr, val);
         vtd_handle_pectl_write(s);
         break;
-- 
2.43.0