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 <[email protected]> Signed-off-by: Junjie Cao <[email protected]> --- 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
