[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    [oss-security] Xen Security Advisory 277 v2 - x86: incorrect error handling for guest p2m page remov
From:       Xen.org security team <security () xen ! org>
Date:       2018-11-20 13:26:26
Message-ID: E1gP62s-0000dz-Ff () xenbits ! xenproject ! org
[Download RAW message or body]

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

                    Xen Security Advisory XSA-277
                              version 2

       x86: incorrect error handling for guest p2m page removals

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

Public release.

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

The internal function querying a domain's p2m table grabs the p2m lock
by default, so that the answer to the query remains true until the
caller can act on that information; it is up to the caller then to
release the lock.  Unfortunately, certain failure paths don't release
the lock.

IMPACT
======

A malicious or buggy guest may cause a deadlock, resulting in a DoS
(Denial of Service) affecting the entire host.

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

Xen 4.11 and onward are vulnerable.

Only x86 systems are vulnerable.  ARM systems are not vulnerable.

Only systems running untrusted HVM or PVH guests are vulnerable.
Systems running only PV guests are not vulnerable.

MITIGATION
==========

Running only PV guests will avoid this vulnerability.

CREDITS
=======

This issue was discovered by Paul Durrant of Citrix.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

xsa277.patch           xen-unstable, Xen 4.11.x

$ sha256sum xsa277*
576cdc05975e43698624b88f7290119dd702b3db8f30f3219754d992d7fef0c6  xsa277.meta
c9025e1daaec4081a61f1ed7b96e69cfe8e35bdd5b4fcc0fadc98f71c2e243e2  xsa277.patch
$

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

Deployment of the patches and/or 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.

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

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/4UyVfoK9kFAlv0C2kMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZ3W4H/0lfQ3hxNjmYa9soWCkXCFWrRHEt5G11dtL3GE1B
E4GbiAWdownHQjhA3okO9yQKDzwY68+hvVZ7YOUNSQ00tZ8j/RWldDZLhbp9JrjI
QMriPefk8X6ZVnF6velUZI2dpOIX6NFBZHxPXUKV8A+e9/+OS7e9CEWrSaprHcbt
MTHv5evulxl8sPXyVa8e2m2YSdEFU6ylfVyH3m5u3cKBpvbSLFKyQN+MNX8rTmAn
+ga3Vj9zehIlDl22nTXCcQHbj75JK0RsDCcH1Glicqm3LZlZ2GXYNe/OiPdLTmwP
8UN8HJhDB2d6w8x4/TV2ad8UGqCJghkxJkqs2RJJdtz8VSo=
=CFtL
-----END PGP SIGNATURE-----

["xsa277.meta" (application/octet-stream)]
["xsa277.patch" (application/octet-stream)]

From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/mm: Put the gfn on all paths after get_gfn_query()

c/s 7867181b2 "x86/PoD: correctly handle non-order-0 decrease-reservation
requests" introduced an early exit in guest_remove_page() for unexpected p2m
types.  However, get_gfn_query() internally takes the p2m lock, and must be
matched with a put_gfn() call later.

Fix the erroneous comment beside the declaration of get_gfn_query().

This is XSA-277.

Reported-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 987395f..26b7123 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -305,7 +305,11 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
 #ifdef CONFIG_X86
     mfn = get_gfn_query(d, gmfn, &p2mt);
     if ( unlikely(p2mt == p2m_invalid) || unlikely(p2mt == p2m_mmio_dm) )
+    {
+        put_gfn(d, gmfn);
+
         return -ENOENT;
+    }
 
     if ( unlikely(p2m_is_paging(p2mt)) )
     {
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index ac33f50..6d849a5 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -448,10 +448,7 @@ static inline mfn_t __nonnull(3) get_gfn_type(
     return get_gfn_type_access(p2m_get_hostp2m(d), gfn, t, &a, q, NULL);
 }
 
-/* Syntactic sugar: most callers will use one of these. 
- * N.B. get_gfn_query() is the _only_ one guaranteed not to take the
- * p2m lock; none of the others can be called with the p2m or paging
- * lock held. */
+/* Syntactic sugar: most callers will use one of these. */
 #define get_gfn(d, g, t)         get_gfn_type((d), (g), (t), P2M_ALLOC)
 #define get_gfn_query(d, g, t)   get_gfn_type((d), (g), (t), 0)
 #define get_gfn_unshare(d, g, t) get_gfn_type((d), (g), (t), \


[prev in list] [next in list] [prev in thread] [next in thread]