AMD/IOMMU: correct global exclusion range extending

Besides unity mapping regions, the AMD IOMMU spec also provides for
exclusion ranges (areas of memory not to be subject to DMA translation)
to be specified by firmware in the ACPI tables. The spec does not put
any constraints on the number of such regions.

Blindly assuming all addresses between any two such ranges should also
be excluded can't be right. Since hardware has room for just a single
such range (comprised of the Exclusion Base Register and the Exclusion
Range Limit Register), combine only adjacent or overlapping regions (for
now; this may require further adjustment in case table entries aren't
sorted by address) with matching exclusion_allow_all settings. This
requires bubbling up error indicators, such that IOMMU init can be
failed when concatenation wasn't possible.

Furthermore, since the exclusion range specified in IOMMU registers
implies R/W access, reject requests asking for less permissions (this
will be brought closer to the spec by a subsequent change).

This is part of XSA-378 / CVE-2021-28695.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
master commit: b02c5c88982411be11e3413159862f255f1f39dc
master date: 2021-08-25 14:12:13 +0200

diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
index 64d10481d738fe624fd1e8a4d941a2a835968ece..9f7659340a3578421f964d54f84e44621baec3b5 100644 (file)
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -98,12 +98,21 @@ static struct amd_iommu * __init find_iommu_from_bdf_cap(
     return NULL;
 }
 
-static void __init reserve_iommu_exclusion_range(
-    struct amd_iommu *iommu, uint64_t base, uint64_t limit)
+static int __init reserve_iommu_exclusion_range(
+    struct amd_iommu *iommu, uint64_t base, uint64_t limit,
+    bool all, bool iw, bool ir)
 {
+    if ( !ir || !iw )
+        return -EPERM;
+
     /* need to extend exclusion range? */
     if ( iommu->exclusion_enable )
     {
+        if ( iommu->exclusion_limit + PAGE_SIZE < base ||
+             limit + PAGE_SIZE < iommu->exclusion_base ||
+             iommu->exclusion_allow_all != all )
+            return -EBUSY;
+
         if ( iommu->exclusion_base < base )
             base = iommu->exclusion_base;
         if ( iommu->exclusion_limit > limit )
@@ -111,16 +120,11 @@ static void __init reserve_iommu_exclusion_range(
     }
 
     iommu->exclusion_enable = IOMMU_CONTROL_ENABLED;
+    iommu->exclusion_allow_all = all;
     iommu->exclusion_base = base;
     iommu->exclusion_limit = limit;
-}
 
-static void __init reserve_iommu_exclusion_range_all(
-    struct amd_iommu *iommu,
-    unsigned long base, unsigned long limit)
-{
-    reserve_iommu_exclusion_range(iommu, base, limit);
-    iommu->exclusion_allow_all = IOMMU_CONTROL_ENABLED;
+    return 0;
 }
 
 static void __init reserve_unity_map_for_device(
@@ -158,6 +162,7 @@ static int __init register_exclusion_range_for_all_devices(
     unsigned long range_top, iommu_top, length;
     struct amd_iommu *iommu;
     unsigned int bdf;
+    int rc = 0;
 
     /* is part of exclusion range inside of IOMMU virtual address space? */
     /* note: 'limit' parameter is assumed to be page-aligned */
@@ -179,10 +184,15 @@ static int __init register_exclusion_range_for_all_devices(
     if ( limit >= iommu_top )
     {
         for_each_amd_iommu( iommu )
-            reserve_iommu_exclusion_range_all(iommu, base, limit);
+        {
+            rc = reserve_iommu_exclusion_range(iommu, base, limit,
+                                               true /* all */, iw, ir);
+            if ( rc )
+                break;
+        }
     }
 
-    return 0;
+    return rc;
 }
 
 static int __init register_exclusion_range_for_device(
@@ -193,6 +203,7 @@ static int __init register_exclusion_range_for_device(
     unsigned long range_top, iommu_top, length;
     struct amd_iommu *iommu;
     u16 req;
+    int rc = 0;
 
     iommu = find_iommu_for_device(seg, bdf);
     if ( !iommu )
@@ -222,12 +233,13 @@ static int __init register_exclusion_range_for_device(
     /* register IOMMU exclusion range settings for device */
     if ( limit >= iommu_top  )
     {
-        reserve_iommu_exclusion_range(iommu, base, limit);
+        rc = reserve_iommu_exclusion_range(iommu, base, limit,
+                                           false /* all */, iw, ir);
         ivrs_mappings[bdf].dte_allow_exclusion = IOMMU_CONTROL_ENABLED;
         ivrs_mappings[req].dte_allow_exclusion = IOMMU_CONTROL_ENABLED;
     }
 
-    return 0;
+    return rc;
 }
 
 static int __init register_exclusion_range_for_iommu_devices(
@@ -237,6 +249,7 @@ static int __init register_exclusion_range_for_iommu_devices(
     unsigned long range_top, iommu_top, length;
     unsigned int bdf;
     u16 req;
+    int rc = 0;
 
     /* is part of exclusion range inside of IOMMU virtual address space? */
     /* note: 'limit' parameter is assumed to be page-aligned */
@@ -267,8 +280,10 @@ static int __init register_exclusion_range_for_iommu_devices(
 
     /* register IOMMU exclusion range settings */
     if ( limit >= iommu_top )
-        reserve_iommu_exclusion_range_all(iommu, base, limit);
-    return 0;
+        rc = reserve_iommu_exclusion_range(iommu, base, limit,
+                                           true /* all */, iw, ir);
+
+    return rc;
 }
 
 static int __init parse_ivmd_device_select(