Xen Security Advisory 397 v2 (CVE-2022-26356) - Racy interactions between dirty vram tracking and paging log dirty hypercalls

Xen.org security team posted 1 patch 2 years, 1 month ago
Failed in applying to current master (apply log)
xen/arch/x86/include/asm/paging.h |  3 ---
xen/arch/x86/mm/hap/hap.c         | 11 ++++-------
xen/arch/x86/mm/paging.c          |  2 +-
3 files changed, 5 insertions(+), 11 deletions(-)
Xen Security Advisory 397 v2 (CVE-2022-26356) - Racy interactions between dirty vram tracking and paging log dirty hypercalls
Posted by Xen.org security team 2 years, 1 month ago
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

            Xen Security Advisory CVE-2022-26356 / XSA-397
                               version 2

 Racy interactions between dirty vram tracking and paging log dirty hypercalls

UPDATES IN VERSION 2
====================

Public release.

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

Activation of log dirty mode done by XEN_DMOP_track_dirty_vram (was named
HVMOP_track_dirty_vram before Xen 4.9) is racy with ongoing log dirty
hypercalls.  A suitably timed call to XEN_DMOP_track_dirty_vram can enable
log dirty while another CPU is still in the process of tearing down the
structures related to a previously enabled log dirty mode
(XEN_DOMCTL_SHADOW_OP_OFF).  This is due to lack of mutually exclusive locking
between both operations and can lead to entries being added in already freed
slots, resulting in a memory leak.

IMPACT
======

An attacker can cause Xen to leak memory, eventually leading to a Denial of
Service (DoS) affecting the entire host.

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

All Xen versions from at least 4.0 onwards are vulnerable.

Only x86 systems are vulnerable.  Arm systems are not vulnerable.

Only domains controlling an x86 HVM guest using Hardware Assisted Paging (HAP)
can leverage the vulnerability.  On common deployments this is limited to
domains that run device models on behalf of guests.

MITIGATION
==========

Using only PV or PVH guests and/or running HVM guests in shadow mode will avoid
the vulnerability.

CREDITS
=======

This issue was discovered by Roger Pau Monné of Citrix.

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.

xsa397.patch           xen-unstable
xsa397-4.16.patch      Xen 4.16.x - Xen 4.15.x
xsa397-4.14.patch      Xen 4.14.x - Xen 4.13.x
xsa397-4.12.patch      Xen 4.12.x

$ sha256sum xsa397*
49c663e2bb9131dbc2488e12487f79bdf0dafd51a32413cbf3964e39d8779cae  xsa397.patch
24f95f47b79739c9cb5b9110137c802989356c82d0aa27963b5ac7e33f667285  xsa397-4.12.patch
9af14f90ba10d074425eb6072a6c648082c92c1cf8b6f881f57ed2fc13d6e49d  xsa397-4.14.patch
ff5dd3b7a8dbf349c3b832b7916322c0296fa59c7f9cd2ba30858989add5f65c  xsa397-4.16.patch
$

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

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

But: Distribution of updated software (except to other members of the
predisclosure list) or deployment of mitigations is prohibited.

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-----

iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmJMJDEMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZOUMH/RRZ8aMaoywqTV38SeTFne2tFT5jnWPPXR1ZGCvh
825hmSqzcYUaILbWFruUfT2PdpGoU9Eprz3xWXBDwgsUEGvKt7ZhGoWvxzXASlDh
cPRh/XwQVEEYsB1cRSk/GoLxLCQEV8oGNpmAcjEM4K1dG0VbVaRD0W2thNCmyPcv
d7aTkAdD2IE8NU4hX8YGN6v+UCkjrgzL0AF/hff9CMj7Sn/wBRrdStLT0LDZU20c
G/5+9nsOAVM7EwrzImI5Lx9KELyHwl37XUPffbftyTLUofdHJ5PK40J1tNIRS/RW
YYvs2alF7ng7LlwB/Go8gtn4XRx6xZidceYrUk22oB4JBqo=
=Fje3
-----END PGP SIGNATURE-----
From 55a9acef857cc0abba2996caa4fcb4c8d5dcf580 Mon Sep 17 00:00:00 2001
From: Roger Pau Monne <roger.pau@citrix.com>
Date: Wed, 23 Feb 2022 09:40:40 +0100
Subject: [PATCH] x86/hap: do not switch on log dirty for VRAM tracking

XEN_DMOP_track_dirty_vram possibly calls into paging_log_dirty_enable
when using HAP mode, and it can interact badly with other ongoing
paging domctls, as XEN_DMOP_track_dirty_vram is not holding the domctl
lock.

This was detected as a result of the following assert triggering when
doing repeated migrations of a HAP HVM domain with a stubdom:

Assertion 'd->arch.paging.log_dirty.allocs == 0' failed at paging.c:198
----[ Xen-4.17-unstable  x86_64  debug=y  Not tainted ]----
CPU:    34
RIP:    e008:[<ffff82d040314b3b>] arch/x86/mm/paging.c#paging_free_log_dirty_bitmap+0x606/0x6
RFLAGS: 0000000000010206   CONTEXT: hypervisor (d0v23)
[...]
Xen call trace:
   [<ffff82d040314b3b>] R arch/x86/mm/paging.c#paging_free_log_dirty_bitmap+0x606/0x63a
   [<ffff82d040279f96>] S xsm/flask/hooks.c#domain_has_perm+0x5a/0x67
   [<ffff82d04031577f>] F paging_domctl+0x251/0xd41
   [<ffff82d04031640c>] F paging_domctl_continuation+0x19d/0x202
   [<ffff82d0403202fa>] F pv_hypercall+0x150/0x2a7
   [<ffff82d0403a729d>] F lstar_enter+0x12d/0x140

Such assert triggered because the stubdom used
XEN_DMOP_track_dirty_vram while dom0 was in the middle of executing
XEN_DOMCTL_SHADOW_OP_OFF, and so log dirty become enabled while
retiring the old structures, thus leading to new entries being
populated in already clear slots.

Fix this by not enabling log dirty for VRAM tracking, similar to what
is done when using shadow instead of HAP. Call
p2m_enable_hardware_log_dirty when enabling VRAM tracking in order to
get some hardware assistance if available. As a side effect the memory
pressure on the p2m pool should go down if only VRAM tracking is
enabled, as the dirty bitmap is no longer allocated.

Note that paging_log_dirty_range (used to get the dirty bitmap for
VRAM tracking) doesn't use the log dirty bitmap, and instead relies on
checking whether each gfn on the range has been switched from
p2m_ram_logdirty to p2m_ram_rw in order to account for dirty pages.

This is CVE-2022-26356 / XSA-397.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/include/asm/paging.h |  3 ---
 xen/arch/x86/mm/hap/hap.c         | 11 ++++-------
 xen/arch/x86/mm/paging.c          |  2 +-
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/include/asm/paging.h b/xen/arch/x86/include/asm/paging.h
index 2ddcfb022c..e20a5f9166 100644
--- a/xen/arch/x86/include/asm/paging.h
+++ b/xen/arch/x86/include/asm/paging.h
@@ -160,9 +160,6 @@ void paging_log_dirty_range(struct domain *d,
                             unsigned long nr,
                             uint8_t *dirty_bitmap);
 
-/* enable log dirty */
-int paging_log_dirty_enable(struct domain *d, bool log_global);
-
 /* log dirty initialization */
 void paging_log_dirty_init(struct domain *d, const struct log_dirty_ops *ops);
 
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index de4b13565a..0434185a8a 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -69,13 +69,6 @@ int hap_track_dirty_vram(struct domain *d,
     {
         unsigned int size = DIV_ROUND_UP(nr_frames, BITS_PER_BYTE);
 
-        if ( !paging_mode_log_dirty(d) )
-        {
-            rc = paging_log_dirty_enable(d, false);
-            if ( rc )
-                goto out;
-        }
-
         rc = -ENOMEM;
         dirty_bitmap = vzalloc(size);
         if ( !dirty_bitmap )
@@ -107,6 +100,10 @@ int hap_track_dirty_vram(struct domain *d,
 
             paging_unlock(d);
 
+            domain_pause(d);
+            p2m_enable_hardware_log_dirty(d);
+            domain_unpause(d);
+
             if ( oend > ostart )
                 p2m_change_type_range(d, ostart, oend,
                                       p2m_ram_logdirty, p2m_ram_rw);
diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index dc3a4a0b4b..8764ea6750 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -212,7 +212,7 @@ static int paging_free_log_dirty_bitmap(struct domain *d, int rc)
     return rc;
 }
 
-int paging_log_dirty_enable(struct domain *d, bool log_global)
+static int paging_log_dirty_enable(struct domain *d, bool log_global)
 {
     int ret;
 
-- 
2.34.1

From: Roger Pau Monne <roger.pau@citrix.com>
Subject: x86/hap: do not switch on log dirty for VRAM tracking

XEN_DMOP_track_dirty_vram possibly calls into paging_log_dirty_enable
when using HAP mode, and it can interact badly with other ongoing
paging domctls, as XEN_DMOP_track_dirty_vram is not holding the domctl
lock.

This was detected as a result of the following assert triggering when
doing repeated migrations of a HAP HVM domain with a stubdom:

Assertion 'd->arch.paging.log_dirty.allocs == 0' failed at paging.c:198
----[ Xen-4.17-unstable  x86_64  debug=y  Not tainted ]----
CPU:    34
RIP:    e008:[<ffff82d040314b3b>] arch/x86/mm/paging.c#paging_free_log_dirty_bitmap+0x606/0x6
RFLAGS: 0000000000010206   CONTEXT: hypervisor (d0v23)
[...]
Xen call trace:
   [<ffff82d040314b3b>] R arch/x86/mm/paging.c#paging_free_log_dirty_bitmap+0x606/0x63a
   [<ffff82d040279f96>] S xsm/flask/hooks.c#domain_has_perm+0x5a/0x67
   [<ffff82d04031577f>] F paging_domctl+0x251/0xd41
   [<ffff82d04031640c>] F paging_domctl_continuation+0x19d/0x202
   [<ffff82d0403202fa>] F pv_hypercall+0x150/0x2a7
   [<ffff82d0403a729d>] F lstar_enter+0x12d/0x140

Such assert triggered because the stubdom used
XEN_DMOP_track_dirty_vram while dom0 was in the middle of executing
XEN_DOMCTL_SHADOW_OP_OFF, and so log dirty become enabled while
retiring the old structures, thus leading to new entries being
populated in already clear slots.

Fix this by not enabling log dirty for VRAM tracking, similar to what
is done when using shadow instead of HAP. Call
p2m_enable_hardware_log_dirty when enabling VRAM tracking in order to
get some hardware assistance if available. As a side effect the memory
pressure on the p2m pool should go down if only VRAM tracking is
enabled, as the dirty bitmap is no longer allocated.

Note that paging_log_dirty_range (used to get the dirty bitmap for
VRAM tracking) doesn't use the log dirty bitmap, and instead relies on
checking whether each gfn on the range has been switched from
p2m_ram_logdirty to p2m_ram_rw in order to account for dirty pages.

This is CVE-2022-26356 / XSA-397.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -144,9 +144,6 @@ void paging_log_dirty_range(struct domai
                             unsigned long nr,
                             uint8_t *dirty_bitmap);
 
-/* enable log dirty */
-int paging_log_dirty_enable(struct domain *d, bool_t log_global);
-
 /* log dirty initialization */
 void paging_log_dirty_init(struct domain *d, const struct log_dirty_ops *ops);
 
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -69,13 +69,6 @@ int hap_track_dirty_vram(struct domain *
     {
         int size = (nr + BITS_PER_BYTE - 1) / BITS_PER_BYTE;
 
-        if ( !paging_mode_log_dirty(d) )
-        {
-            rc = paging_log_dirty_enable(d, 0);
-            if ( rc )
-                goto out;
-        }
-
         rc = -ENOMEM;
         dirty_bitmap = vzalloc(size);
         if ( !dirty_bitmap )
@@ -107,6 +100,10 @@ int hap_track_dirty_vram(struct domain *
 
             paging_unlock(d);
 
+            domain_pause(d);
+            p2m_enable_hardware_log_dirty(d);
+            domain_unpause(d);
+
             if ( oend > ostart )
                 p2m_change_type_range(d, ostart, oend,
                                       p2m_ram_logdirty, p2m_ram_rw);
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -209,7 +209,7 @@ static int paging_free_log_dirty_bitmap(
     return rc;
 }
 
-int paging_log_dirty_enable(struct domain *d, bool_t log_global)
+static int paging_log_dirty_enable(struct domain *d, bool_t log_global)
 {
     int ret;
 
From: Roger Pau Monne <roger.pau@citrix.com>
Subject: x86/hap: do not switch on log dirty for VRAM tracking

XEN_DMOP_track_dirty_vram possibly calls into paging_log_dirty_enable
when using HAP mode, and it can interact badly with other ongoing
paging domctls, as XEN_DMOP_track_dirty_vram is not holding the domctl
lock.

This was detected as a result of the following assert triggering when
doing repeated migrations of a HAP HVM domain with a stubdom:

Assertion 'd->arch.paging.log_dirty.allocs == 0' failed at paging.c:198
----[ Xen-4.17-unstable  x86_64  debug=y  Not tainted ]----
CPU:    34
RIP:    e008:[<ffff82d040314b3b>] arch/x86/mm/paging.c#paging_free_log_dirty_bitmap+0x606/0x6
RFLAGS: 0000000000010206   CONTEXT: hypervisor (d0v23)
[...]
Xen call trace:
   [<ffff82d040314b3b>] R arch/x86/mm/paging.c#paging_free_log_dirty_bitmap+0x606/0x63a
   [<ffff82d040279f96>] S xsm/flask/hooks.c#domain_has_perm+0x5a/0x67
   [<ffff82d04031577f>] F paging_domctl+0x251/0xd41
   [<ffff82d04031640c>] F paging_domctl_continuation+0x19d/0x202
   [<ffff82d0403202fa>] F pv_hypercall+0x150/0x2a7
   [<ffff82d0403a729d>] F lstar_enter+0x12d/0x140

Such assert triggered because the stubdom used
XEN_DMOP_track_dirty_vram while dom0 was in the middle of executing
XEN_DOMCTL_SHADOW_OP_OFF, and so log dirty become enabled while
retiring the old structures, thus leading to new entries being
populated in already clear slots.

Fix this by not enabling log dirty for VRAM tracking, similar to what
is done when using shadow instead of HAP. Call
p2m_enable_hardware_log_dirty when enabling VRAM tracking in order to
get some hardware assistance if available. As a side effect the memory
pressure on the p2m pool should go down if only VRAM tracking is
enabled, as the dirty bitmap is no longer allocated.

Note that paging_log_dirty_range (used to get the dirty bitmap for
VRAM tracking) doesn't use the log dirty bitmap, and instead relies on
checking whether each gfn on the range has been switched from
p2m_ram_logdirty to p2m_ram_rw in order to account for dirty pages.

This is CVE-2022-26356 / XSA-397.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -160,9 +160,6 @@ void paging_log_dirty_range(struct domai
                             unsigned long nr,
                             uint8_t *dirty_bitmap);
 
-/* enable log dirty */
-int paging_log_dirty_enable(struct domain *d, bool log_global);
-
 /* log dirty initialization */
 void paging_log_dirty_init(struct domain *d, const struct log_dirty_ops *ops);
 
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -69,13 +69,6 @@ int hap_track_dirty_vram(struct domain *
     {
         int size = (nr + BITS_PER_BYTE - 1) / BITS_PER_BYTE;
 
-        if ( !paging_mode_log_dirty(d) )
-        {
-            rc = paging_log_dirty_enable(d, false);
-            if ( rc )
-                goto out;
-        }
-
         rc = -ENOMEM;
         dirty_bitmap = vzalloc(size);
         if ( !dirty_bitmap )
@@ -107,6 +100,10 @@ int hap_track_dirty_vram(struct domain *
 
             paging_unlock(d);
 
+            domain_pause(d);
+            p2m_enable_hardware_log_dirty(d);
+            domain_unpause(d);
+
             if ( oend > ostart )
                 p2m_change_type_range(d, ostart, oend,
                                       p2m_ram_logdirty, p2m_ram_rw);
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -209,7 +209,7 @@ static int paging_free_log_dirty_bitmap(
     return rc;
 }
 
-int paging_log_dirty_enable(struct domain *d, bool log_global)
+static int paging_log_dirty_enable(struct domain *d, bool log_global)
 {
     int ret;
 
From: Roger Pau Monne <roger.pau@citrix.com>
Subject: x86/hap: do not switch on log dirty for VRAM tracking

XEN_DMOP_track_dirty_vram possibly calls into paging_log_dirty_enable
when using HAP mode, and it can interact badly with other ongoing
paging domctls, as XEN_DMOP_track_dirty_vram is not holding the domctl
lock.

This was detected as a result of the following assert triggering when
doing repeated migrations of a HAP HVM domain with a stubdom:

Assertion 'd->arch.paging.log_dirty.allocs == 0' failed at paging.c:198
----[ Xen-4.17-unstable  x86_64  debug=y  Not tainted ]----
CPU:    34
RIP:    e008:[<ffff82d040314b3b>] arch/x86/mm/paging.c#paging_free_log_dirty_bitmap+0x606/0x6
RFLAGS: 0000000000010206   CONTEXT: hypervisor (d0v23)
[...]
Xen call trace:
   [<ffff82d040314b3b>] R arch/x86/mm/paging.c#paging_free_log_dirty_bitmap+0x606/0x63a
   [<ffff82d040279f96>] S xsm/flask/hooks.c#domain_has_perm+0x5a/0x67
   [<ffff82d04031577f>] F paging_domctl+0x251/0xd41
   [<ffff82d04031640c>] F paging_domctl_continuation+0x19d/0x202
   [<ffff82d0403202fa>] F pv_hypercall+0x150/0x2a7
   [<ffff82d0403a729d>] F lstar_enter+0x12d/0x140

Such assert triggered because the stubdom used
XEN_DMOP_track_dirty_vram while dom0 was in the middle of executing
XEN_DOMCTL_SHADOW_OP_OFF, and so log dirty become enabled while
retiring the old structures, thus leading to new entries being
populated in already clear slots.

Fix this by not enabling log dirty for VRAM tracking, similar to what
is done when using shadow instead of HAP. Call
p2m_enable_hardware_log_dirty when enabling VRAM tracking in order to
get some hardware assistance if available. As a side effect the memory
pressure on the p2m pool should go down if only VRAM tracking is
enabled, as the dirty bitmap is no longer allocated.

Note that paging_log_dirty_range (used to get the dirty bitmap for
VRAM tracking) doesn't use the log dirty bitmap, and instead relies on
checking whether each gfn on the range has been switched from
p2m_ram_logdirty to p2m_ram_rw in order to account for dirty pages.

This is CVE-2022-26356 / XSA-397.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -162,9 +162,6 @@ void paging_log_dirty_range(struct domai
                             unsigned long nr,
                             uint8_t *dirty_bitmap);
 
-/* enable log dirty */
-int paging_log_dirty_enable(struct domain *d, bool log_global);
-
 /* log dirty initialization */
 void paging_log_dirty_init(struct domain *d, const struct log_dirty_ops *ops);
 
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -69,13 +69,6 @@ int hap_track_dirty_vram(struct domain *
     {
         unsigned int size = DIV_ROUND_UP(nr_frames, BITS_PER_BYTE);
 
-        if ( !paging_mode_log_dirty(d) )
-        {
-            rc = paging_log_dirty_enable(d, false);
-            if ( rc )
-                goto out;
-        }
-
         rc = -ENOMEM;
         dirty_bitmap = vzalloc(size);
         if ( !dirty_bitmap )
@@ -107,6 +100,10 @@ int hap_track_dirty_vram(struct domain *
 
             paging_unlock(d);
 
+            domain_pause(d);
+            p2m_enable_hardware_log_dirty(d);
+            domain_unpause(d);
+
             if ( oend > ostart )
                 p2m_change_type_range(d, ostart, oend,
                                       p2m_ram_logdirty, p2m_ram_rw);
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -211,7 +211,7 @@ static int paging_free_log_dirty_bitmap(
     return rc;
 }
 
-int paging_log_dirty_enable(struct domain *d, bool log_global)
+static int paging_log_dirty_enable(struct domain *d, bool log_global)
 {
     int ret;