[xen stable-4.13] gnttab: deal with status frame mapping race

[patchbot@xxxxxxx]

commit 9c4b19c110e1410e50a9f1dbd15d337b05e9cc9d
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Wed Sep 8 14:57:31 2021 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Sep 8 14:57:31 2021 +0200

    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 CVE-2021-28701 / XSA-384.
    
    Reported-by: Julien Grall <jgrall@xxxxxxxxxx>
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
    master commit: eb6bbf7b30da5bae87932514d54d0e3c68b23757
    master date: 2021-09-08 14:37:45 +0200
---
 xen/arch/arm/mm.c        | 11 ++++++++---
 xen/arch/x86/mm.c        |  2 ++
 xen/common/grant_table.c | 11 ++++++++++-
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 67853d58d7..86fbdf8696 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1426,6 +1426,8 @@ int xenmem_add_to_physmap_one(
         if ( rc )
             return rc;
 
+        /* Need to take care of the reference obtained in gnttab_map_frame(). 
*/
+        page = mfn_to_page(mfn);
         t = p2m_ram_rw;
 
         break;
@@ -1493,9 +1495,12 @@ int xenmem_add_to_physmap_one(
     /* Map at new location. */
     rc = guest_physmap_add_entry(d, gfn, 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);
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 7cdd23cd3c..ad89bfb45f 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4738,6 +4738,8 @@ int xenmem_add_to_physmap_one(
             rc = gnttab_map_frame(d, idx, gpfn, &mfn);
             if ( rc )
                 return rc;
+            /* Need to take care of the ref obtained in gnttab_map_frame(). */
+            page = mfn_to_page(mfn);
             break;
         case XENMAPSPACE_gmfn:
         {
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index a02e906e94..d2853a664a 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -4072,7 +4072,16 @@ int gnttab_map_frame(struct domain *d, unsigned long 
idx, gfn_t gfn, mfn_t *mfn)
     }
 
     if ( !rc )
-        gnttab_set_frame_gfn(gt, status, idx, gfn);
+    {
+        /*
+         * Make sure gnttab_unpopulate_status_frames() won't (successfully)
+         * free the page until our caller has completed its operation.
+         */
+        if ( get_page(mfn_to_page(*mfn), d) )
+            gnttab_set_frame_gfn(gt, status, idx, gfn);
+        else
+            rc = -EBUSY;
+    }
 
     grant_write_unlock(gt);
 
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.13