All of lore.kernel.org
 help / color / mirror / Atom feed
* Xen Security Advisory 384 v3 (CVE-2021-28701) - Another race in XENMAPSPACE_grant_table handling
@ 2021-09-08 12:28 Xen.org security team
  0 siblings, 0 replies; only message in thread
From: Xen.org security team @ 2021-09-08 12:28 UTC (permalink / raw)
  To: xen-announce, xen-devel, xen-users, oss-security; +Cc: Xen.org security team

[-- Attachment #1: Type: text/plain, Size: 4546 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

            Xen Security Advisory CVE-2021-28701 / XSA-384
                               version 3

            Another race in XENMAPSPACE_grant_table handling

UPDATES IN VERSION 3
====================

Public release.

ISSUE DESCRIPTION
=================

Guests are permitted access to certain Xen-owned pages of memory.  The
majority of such pages remain allocated / associated with a guest for
its entire lifetime.  Grant table v2 status pages, however, are
de-allocated when a guest switches (back) from v2 to v1.  Freeing
such pages requires that the hypervisor enforce that no parallel request
can result in the addition of a mapping of such a page to a guest.  That
enforcement was missing, allowing guests to retain access to pages
that were freed and perhaps re-used for other purposes.

Unfortunately, when XSA-379 was being prepared, this similar issue was
not noticed.

IMPACT
======

A malicious guest may be able to elevate its privileges to that of the
host, cause host or guest Denial of Service (DoS), or cause information
leaks.

VULNERABLE SYSTEMS
==================

All Xen versions from 4.0 onwards are affected.  Xen versions 3.4 and
older are not affected.

Only x86 HVM and PVH guests permitted to use grant table version 2
interfaces can leverage this vulnerability.  x86 PV guests cannot
leverage this vulnerability.  On Arm, grant table v2 use is explicitly
unsupported.

MITIGATION
==========

Running only PV guests will avoid this vulnerability.

Suppressing use of grant table v2 interfaces for HVM or PVH guests will
also avoid this vulnerability.

NOTE REGARDING EMBARGO
======================

Please note that the public embargo time for this advisory is
2021-09-08, two weeks later than XSA-378,379,380,382,383.

CREDITS
=======

This issue was discovered by Julien Grall of Amazon.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

Note that patches for released versions are generally prepared to
apply to the stable branches, and may not apply cleanly to the most
recent release tarball.  Downstreams are encouraged to update to the
tip of the stable branch before applying these patches.

xsa384.patch           xen-unstable - Xen 4.15.x
xsa384-4.14.patch      Xen 4.14.x - 4.12.x
xsa384-4.11.patch      Xen 4.11.x

$ sha256sum xsa384*
3bf555e8b2a37dec361b86c0b6a3f59af2e1a24e3457ed92e0cfeaa662f1663a  xsa384.patch
435b431dc77d255031832dc265a8d5aa2f13f3b1a7de497b62ac2df5ad61da90  xsa384-4.11.patch
f98ec4c25fb122de6353eb7d9f5678dd09982f887bf201d6f178e9b647618c9a  xsa384-4.14.patch
$

DEPLOYMENT DURING EMBARGO
=========================

Deployment of the patches and/or PV-guest-only mitigations described
above (or others which are substantially similar) is permitted during
the embargo, even on public-facing systems with untrusted guest users
and administrators.

HOWEVER, distribution of updated software is prohibited (except to other
members of the predisclosure list).

ADDITIONALLY, deployment of the grant table v2 disabling mitigation
described above *is* now (following the public release of XSA-379)
permitted during the embargo on public-facing systems with untrusted
guest users and administrators.  This is because (although such a
configuration change is recognizable by the affected guests) it is a
mitigation recommended in XSA-379, so such a change would not reveal
the existence of a further problem.

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.

(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----

iQE/BAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmE4rD4MHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZ8S8H+ITq1u5AaK+N1wgsgztLJRh1cjFTDaLZdI0mypwV
+wPhNdw1FFH19wYDZhOJIzv3h5352YVUYs8P6HvhFWrsUaq5n4cz17wxUHch74r3
MUUU9WUdTffQrAAOSSK65yWTUewv/FglEWP55YFBDPqBJHpVWt2MMidmP6My6azK
GZAHSWU7+qNXbs4/OM+dQJyUJm6yZtAltVtVmiJdJ5bSZyqe+82zRMnS39jlkZEh
VwFnw6rdlPcYO/fNYi25bQSlXbFeruSJYK+omrrFsyd65Z4D5LyZc5CQkfRJgEt6
vMDsQR+5hrE/KXKr2mfyTx6nh0RdR8kcI2Wh017BYuLqhA==
=AEo5
-----END PGP SIGNATURE-----

[-- Attachment #2: xsa384.patch --]
[-- Type: application/octet-stream, Size: 2852 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: 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@amazon.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
---
v2: Pull get_page() earlier and fold if()s.

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1420,6 +1420,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;
@@ -1487,9 +1489,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);
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2726,6 +2726,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 reference obtained in gnttab_map_frame(). */
+        page = mfn_to_page(mfn);
         break;
 
     case XENMAPSPACE_gmfn:
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -4097,7 +4097,16 @@ int gnttab_map_frame(struct domain *d, u
     }
 
     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);
 

[-- Attachment #3: xsa384-4.11.patch --]
[-- Type: application/octet-stream, Size: 2870 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: 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@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
@@ -1238,6 +1238,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;
@@ -1304,9 +1306,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);
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4751,6 +4751,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:
         {
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3964,7 +3964,16 @@ int gnttab_map_frame(struct domain *d, u
                                        *mfn, 0);
 
     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);
 

[-- Attachment #4: xsa384-4.14.patch --]
[-- Type: application/octet-stream, Size: 2827 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: 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@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
@@ -1423,6 +1423,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;
@@ -1490,9 +1492,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);
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4620,6 +4620,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:
         {
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -4102,7 +4102,16 @@ int gnttab_map_frame(struct domain *d, u
     }
 
     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);
 

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-09-08 12:28 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-08 12:28 Xen Security Advisory 384 v3 (CVE-2021-28701) - Another race in XENMAPSPACE_grant_table handling Xen.org security team

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.