File xsa460.patch of Package xen.36362

Overview Repositories Revisions Requests Users Attributes Meta

File xsa460.patch of Package xen.36362

From: Teddy Astie <teddy.astie@vates.tech>
Subject: x86/IOMMU: move tracking in iommu_identity_mapping()

If for some reason xmalloc() fails after having mapped the reserved
regions, an error is reported, but the regions are actually mapped in
p2m. Similarly if out of perhaps multiple set_identity_p2m_entry() some
(but not the first one) fail, the partial mappings of region would be
retained without being tracked anywhere, and hence without there being a
way to remove them again from the domain's P2M.

Move the setting up of the list entry ahead of trying to map the region.
In case other than the first mapping fails, keep record of the full
region, such that a subsequent unmapping request can be properly matched
(as that'll pass in the same original range, the caller being unaware of
how much of the range actually having been mapped).

To compensate for the potentially excess unmapping requests, don't log a
warning from p2m_remove_identity_entry() when there really was nothing
mapped at a given GFN.

This is XSA-460 / CVE-2024-31145.

Fixes: 2201b67b9128 ("VT-d: improve RMRR region handling")
Fixes: c0e19d7c6c42 ("IOMMU: generalize VT-d's tracking of mapped RMRR regions")
Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
Arguably iommu_identity_mapping() could also unmap when other than the
first page's mapping fails. However, clear_identity_p2m_entry() can in
principle fail, too, and dealing with such failure would end up ugly.
Callers, upon observing failure for a DomU, will do a full unmap anyway.
---
v3: Add comment. Re-add 2nd Fixes: tag.
v2: Move more than just the allocation.

--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1588,9 +1588,11 @@ int clear_identity_p2m_entry(struct doma
     else
     {
         gfn_unlock(p2m, gfn, 0);
-        printk(XENLOG_G_WARNING
-               "non-identity map d%d:%lx not cleared (mapped to %lx)\n",
-               d->domain_id, gfn_l, mfn_x(mfn));
+        if ( (p2mt != p2m_invalid && p2mt != p2m_mmio_dm) ||
+             a != p2m_access_n || !mfn_eq(mfn, INVALID_MFN) )
+           printk(XENLOG_G_WARNING
+                  "non-identity map %pd:%lx not cleared (mapped to %lx)\n",
+                  d, gfn_l, mfn_x(mfn));
         ret = 0;
     }
 
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -226,24 +226,36 @@ int iommu_identity_mapping(struct domain
     if ( p2ma == p2m_access_x )
         return -ENOENT;
 
-    while ( base_pfn < end_pfn )
-    {
-        int err = set_identity_p2m_entry(d, base_pfn, p2ma, flag);
-
-        if ( err )
-            return err;
-        base_pfn++;
-    }
-
     map = xmalloc(struct identity_map);
     if ( !map )
         return -ENOMEM;
+
     map->base = base;
     map->end = end;
     map->access = p2ma;
     map->count = 1;
+
+    /*
+     * Insert into list ahead of mapping, so the range can be found when
+     * trying to clean up.
+     */
     list_add_tail(&map->list, &hd->arch.identity_maps);
 
+    for ( ; base_pfn < end_pfn; ++base_pfn )
+    {
+        int err = set_identity_p2m_entry(d, base_pfn, p2ma, flag);
+
+        if ( !err )
+            continue;
+
+        if ( (map->base >> PAGE_SHIFT_4K) == base_pfn )
+        {
+            list_del(&map->list);
+            xfree(map);
+        }
+        return err;
+    }
+
     return 0;
 }

Places

File xsa460.patch of Package xen.36362

Places