File xsa384.patch of Package xen.22546

Overview Repositories Revisions Requests Users Attributes Meta

File xsa384.patch of Package xen.22546

gnttab: deal with status frame mapping race

Once gnttab_map_frame() drops the grant table lock, the MFN it reports
back to its caller is free to other manipulation. In particular
gnttab_unpopulate_status_frames() might free it, by a racing request on
another CPU, thus resulting in a reference to a deallocated page getting
added to a domain's P2M.

Obtain a page reference in gnttab_map_frame() to prevent freeing of the
page until xenmem_add_to_physmap_one() has actually completed its acting
on the page. Do so uniformly, even if only strictly required for v2
status pages, to avoid extra conditionals (which then would all need to
be kept in sync going forward).

This is XSA-384.

Reported-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1080,7 +1080,15 @@ int xenmem_add_to_physmap_one(
         {
             idx &= ~XENMAPIDX_grant_table_status;
             if ( idx < nr_status_frames(d->grant_table) )
+            {
                 mfn = virt_to_mfn(d->grant_table->status[idx]);
+                page = mfn_to_page(mfn);
+                if ( !get_page(page, d) )
+                {
+                    grant_write_unlock(d->grant_table);
+                    return -EBUSY;
+                }
+            }
             else
                 mfn = INVALID_MFN;
             status = 1;
@@ -1107,6 +1115,8 @@ int xenmem_add_to_physmap_one(
             if ( rc )
             {
                 grant_write_unlock(d->grant_table);
+                if ( page )
+                    put_page(page);
                 return rc;
             }
         }
@@ -1121,7 +1131,10 @@ int xenmem_add_to_physmap_one(
         grant_write_unlock(d->grant_table);
 
         if ( mfn == INVALID_MFN )
+        {
+            ASSERT(!page);
             return -EINVAL;
+        }
 
         break;
     case XENMAPSPACE_shared_info:
@@ -1187,9 +1200,12 @@ int xenmem_add_to_physmap_one(
     /* Map at new location. */
     rc = guest_physmap_add_entry(d, gpfn, mfn, 0, t);
 
-    /* If we fail to add the mapping, we need to drop the reference we
-     * took earlier on foreign pages */
-    if ( rc && space == XENMAPSPACE_gmfn_foreign )
+    /*
+     * For XENMAPSPACE_gmfn_foreign if we failed to add the mapping, we need
+     * to drop the reference we took earlier. In all other cases we need to
+     * drop any reference we took earlier (perhaps indirectly).
+     */
+    if ( space == XENMAPSPACE_gmfn_foreign ? rc : page != NULL )
     {
         ASSERT(page != NULL);
         put_page(page);
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5667,7 +5667,15 @@ int xenmem_add_to_physmap_one(
             {
                 idx &= ~XENMAPIDX_grant_table_status;
                 if ( idx < nr_status_frames(d->grant_table) )
+                {
                     mfn = virt_to_mfn(d->grant_table->status[idx]);
+                    page = mfn_to_page(mfn);
+                    if ( !get_page(page, d) )
+                    {
+                        grant_write_unlock(d->grant_table);
+                        return -EBUSY;
+                    }
+                }
             }
             else
             {

Places

File xsa384.patch of Package xen.22546

Places