Changeset 86763 in vbox for trunk/src/VBox/Devices

Timestamp:

Oct 30, 2020 5:29:14 AM (5 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

141157

Message:

AMD IOMMU: bugref:9654 Ensure the DTE falls within the device table segment limit and use uint32_t for offDte, uint16_t isn't enough.

File:

• trunk/src/VBox/Devices/Bus/DevIommuAmd.cpp (modified) (9 diffs)

trunk/src/VBox/Devices/Bus/DevIommuAmd.cpp

@@ -891 +891 @@
     /* Update the register. */
     pThis->aDevTabBaseAddrs[0].u64 = u64Value;
+
+    /* Paranoia. */
+    Assert(pThis->aDevTabBaseAddrs[0].n.u9Size <= g_auDevTabSegMaxSizes[0]);
     return VINF_SUCCESS;
 }
@@ -2667 +2670 @@
     IOMMU_CTRL_T const Ctrl = iommuAmdGetCtrl(pThis);
 
+    /* Figure out which device table segment is being accessed. */
     uint8_t const idxSegsEn = Ctrl.n.u3DevTabSegEn;
     Assert(idxSegsEn < RT_ELEMENTS(g_auDevTabSegShifts));
@@ -2675 +2679 @@
 
     RTGCPHYS const GCPhysDevTab = pThis->aDevTabBaseAddrs[idxSeg].n.u40Base << X86_PAGE_4K_SHIFT;
-    uint16_t const offDte       = (uDevId & ~g_auDevTabSegMasks[idxSegsEn]) * sizeof(DTE_T);
+    uint32_t const offDte       = (uDevId & ~g_auDevTabSegMasks[idxSegsEn]) * sizeof(DTE_T);
     RTGCPHYS const GCPhysDte    = GCPhysDevTab + offDte;
 
-    Assert(!(GCPhysDevTab & X86_PAGE_4K_OFFSET_MASK));
-    int rc = PDMDevHlpPCIPhysRead(pDevIns, GCPhysDte, pDte, sizeof(*pDte));
-    if (RT_FAILURE(rc))
-    {
+    /* Ensure the DTE falls completely within the device table segment. */
+    uint32_t const cbDevTabSeg  = (pThis->aDevTabBaseAddrs[idxSeg].n.u9Size + 1) << X86_PAGE_4K_SHIFT;
+    if (offDte + sizeof(DTE_T) <= cbDevTabSeg)
+    {
+        /* Read the device table entry from guest memory. */
+        Assert(!(GCPhysDevTab & X86_PAGE_4K_OFFSET_MASK));
+        int rc = PDMDevHlpPCIPhysRead(pDevIns, GCPhysDte, pDte, sizeof(*pDte));
+        if (RT_SUCCESS(rc))
+            return rc;
+
+        /* Raise a device table hardware error. */
         LogFunc(("Failed to read device table entry at %#RGp. rc=%Rrc -> DevTabHwError\n", GCPhysDte, rc));
 
@@ -2687 +2698 @@
         iommuAmdInitDevTabHwErrorEvent(uDevId, GCPhysDte, enmOp, &EvtDevTabHwErr);
         iommuAmdRaiseDevTabHwErrorEvent(pDevIns, enmOp, &EvtDevTabHwErr);
-        return VERR_IOMMU_IPE_1;
-    }
-
-    return rc;
+        return VERR_IOMMU_DTE_READ_FAILED;
+    }
+
+    /* Raise an I/O page fault for out-of-bounds acccess. */
+    EVT_IO_PAGE_FAULT_T EvtIoPageFault;
+    iommuAmdInitIoPageFaultEvent(uDevId, 0 /* uDomainId */, 0 /* uIova */, false /* fPresent */, false /* fRsvdNotZero */,
+                                 false /* fPermDenied */, enmOp, &EvtIoPageFault);
+    iommuAmdRaiseIoPageFaultEvent(pDevIns, pDte, NULL /* pIrte */, enmOp, &EvtIoPageFault, kIoPageFaultType_DevId_Invalid);
+    return VERR_IOMMU_DTE_BAD_OFFSET;
 }
 
@@ -3244 +3260 @@
         iommuAmdInitIoPageFaultEvent(uDevId, pDte->n.u16DomainId, GCPhysIn, false /* fPresent */, false /* fRsvdNotZero */,
                                      false /* fPermDenied */, enmOp, &EvtIoPageFault);
-        iommuAmdRaiseIoPageFaultEvent(pDevIns, pDte, NULL /* pIrte */, enmOp, &EvtIoPageFault,
-                                      kIoPageFaultType_IrteAddrInvalid);
+        iommuAmdRaiseIoPageFaultEvent(pDevIns, pDte, NULL /* pIrte */, enmOp, &EvtIoPageFault, kIoPageFaultType_IrteAddrInvalid);
         return VERR_IOMMU_ADDR_TRANSLATION_FAILED;
     }
@@ -3348 +3363 @@
 {
     /* Read the device table entry from memory. */
-    LogFlowFunc(("uDevId=%#x enmOp=%u\n", uDevId, enmOp));
+    LogFlowFunc(("uDevId=%#x (%#x:%#x:%#x) enmOp=%u\n", uDevId,
+                 ((uDevId >> VBOX_PCI_BUS_SHIFT) & VBOX_PCI_BUS_MASK),
+                 ((uDevId >> VBOX_PCI_DEVFN_DEV_SHIFT) & VBOX_PCI_DEVFN_DEV_MASK), (uDevId & VBOX_PCI_DEVFN_FUN_MASK), enmOp));
 
     DTE_T Dte;
@@ -3404 +3421 @@
                     {
                         uint8_t const uIntrCtrl = Dte.n.u2IntrCtrl;
-                        if (uIntrCtrl == IOMMU_INTR_CTRL_TARGET_ABORT)
-                        {
-                            LogFunc(("IntCtl=0: Target aborting fixed/arbitrated interrupt -> Target abort\n"));
-                            iommuAmdSetPciTargetAbort(pDevIns);
-                            return VERR_IOMMU_INTR_REMAP_DENIED;
-                        }
-
-                        if (uIntrCtrl == IOMMU_INTR_CTRL_FWD_UNMAPPED)
-                        {
-                            fPassThru = true;
-                            break;
-                        }
-
                         if (uIntrCtrl == IOMMU_INTR_CTRL_REMAP)
                         {
@@ -3444 +3448 @@
                         }
 
+                        if (uIntrCtrl == IOMMU_INTR_CTRL_FWD_UNMAPPED)
+                        {
+                            fPassThru = true;
+                            break;
+                        }
+
+                        if (uIntrCtrl == IOMMU_INTR_CTRL_TARGET_ABORT)
+                        {
+                            LogFunc(("IntCtl=0: Remapping disallowed for fixed/arbitrated interrupt (%#x) -> Target abort\n",
+                                     pMsiIn->Data.n.u8Vector));
+                            iommuAmdSetPciTargetAbort(pDevIns);
+                            return VERR_IOMMU_INTR_REMAP_DENIED;
+                        }
+
                         /* Paranoia. */
                         Assert(uIntrCtrl == IOMMU_INTR_CTRL_RSVD);
 
                         LogFunc(("IntCtl mode invalid %#x -> Illegal DTE\n", uIntrCtrl));
-
                         EVT_ILLEGAL_DTE_T Event;
                         iommuAmdInitIllegalDteEvent(uDevId, pMsiIn->Addr.u64, true /* fRsvdNotZero */, enmOp, &Event);
@@ -3474 +3491 @@
                 }
 
+                LogFunc(("Remapping/passthru disallowed for interrupt (%#x) -> Target abort\n", pMsiIn->Data.n.u8Vector));
                 iommuAmdSetPciTargetAbort(pDevIns);
                 return VERR_IOMMU_INTR_REMAP_DENIED;