[prev in list] [next in list] [prev in thread] [next in thread]
List: xen-announce
Subject: [Xen-announce] Xen Security Advisory 236 (CVE-2017-15597) - pin count / page reference race in grant
From: Xen.org security team <security () xen ! org>
Date: 2017-10-24 13:55:54
Message-ID: E1e6zgQ-0007u1-QN () xenbits ! xenproject ! org
[Download RAW message or body]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Xen Security Advisory CVE-2017-15597 / XSA-236
version 3
pin count / page reference race in grant table code
UPDATES IN VERSION 3
====================
We now once again think that only Xen 4.2 and newer are vulnerable.
Fix grammar typo.
Public release.
ISSUE DESCRIPTION
=================
Grant copying code made an implication that any grant pin would be
accompanied by a suitable page reference. Other portions of code,
however, did not match up with that assumption. When such a grant
copy operation is being done on a grant of a dying domain, the
assumption turns out wrong.
IMPACT
======
A malicious guest administrator can cause hypervisor memory
corruption, most likely resulting in host crash and a Denial of
Service. Privilege escalation and information leaks cannot be ruled
out.
VULNERABLE SYSTEMS
==================
Xen versions from 4.2 onwards are vulnerable. Xen versions 4.1 and
earlier are not vulnerable.
Both x86 and ARM are vulnerable, and on x86 both PV and HVM guests can
trigger the vulnerability.
MITIGATION
==========
Running only guests without para-virtual drivers, and known not to
issue grant table operations can avoid the vulnerability.
CREDITS
=======
This issue was discovered by Pawel Wieczorkiewicz of Amazon.
RESOLUTION
==========
Applying the appropriate attached patch resolves this issue.
xsa236.patch xen-unstable
xsa236-4.9.patch Xen 4.9.x, Xen 4.8.x, Xen 4.7.x, Xen 4.6.x
xsa236-4.5.patch Xen 4.5.x
$ sha256sum xsa236*
2f7736c43b6da7d983cf3edbc10024c4cba9d6d3e5b2b758a07de726a804617d xsa236.meta
f06f01fb4ffcfc7938a2fc6ab73559ebbaac2d448bd36ca538bb07ba510eeb4a xsa236.patch
c98a4b50d021414626cd68002643e9aa0cc6067b98cd5dd995c0140a7933d1ea xsa236-4.5.patch
b6fe5604af26e93184f30127ebbb644f127ecc7116b093c161ca3044b44d2fe9 xsa236-4.9.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-----
Version: GnuPG v1
iQEcBAEBCAAGBQJZ70ZiAAoJEIP+FMlX6CvZlBgH/0cwYrP3/zvc3dNJRtpxyn1J
BkigYP8JBIYW85M7KdZDFBhgXIpuw6x45XZ4qfq6rrz3GOp5oZgZVFIoggHZBzRe
eVCIpjOAXInM7ThsE6pV1Qr/JKe8V6RJumXEgqr5zznWpGmcFChWmobA+BBq64P6
87ALWjXBcuqOyjJnJQwEjk+kHJMnIpocVZk6NqcDeoHoJvRh/Zk4YYc78qm4Lucw
d0yHq5azA9bgt5iJgxUvF74B4r8JxTLmA8sn7Kx280UJGEAkqM7jj1QVQ6sb8fgO
q6RSzBVnuVqLh4E1Dji9KaxcRRVnbrp2FFpBUUWHAVVO4O0GYlu5NxERnnye9v0=
=zI77
-----END PGP SIGNATURE-----
["xsa236.meta" (application/octet-stream)]
["xsa236.patch" (application/octet-stream)]
From: Jan Beulich <jbeulich@suse.com>
Subject: gnttab: fix pin count / page reference race
Dropping page references before decrementing pin counts is a bad idea
if assumptions are being made that a non-zero pin count implies a valid
page. Fix the order of operations in gnttab_copy_release_buf(), but at
the same time also remove the assertion that was found to trigger:
map_grant_ref() also has the potential of causing a race here, and
changing the order of operations there would likely be quite a bit more
involved.
This is XSA-236.
Reported-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2368,9 +2368,20 @@ acquire_grant_for_copy(
td = page_get_owner_and_reference(*page);
/*
* act->pin being non-zero should guarantee the page to have a
- * non-zero refcount and hence a valid owner.
+ * non-zero refcount and hence a valid owner (matching the one on
+ * record), with one exception: If the owning domain is dying we
+ * had better not make implications from pin count (map_grant_ref()
+ * updates pin counts before obtaining page references, for
+ * example).
*/
- ASSERT(td);
+ if ( td != rd || rd->is_dying )
+ {
+ if ( td )
+ put_page(*page);
+ *page = NULL;
+ rc = GNTST_bad_domain;
+ goto unlock_out_clear;
+ }
}
act->pin += readonly ? GNTPIN_hstr_inc : GNTPIN_hstw_inc;
@@ -2480,6 +2491,11 @@ static void gnttab_copy_release_buf(stru
unmap_domain_page(buf->virt);
buf->virt = NULL;
}
+ if ( buf->have_grant )
+ {
+ release_grant_for_copy(buf->domain, buf->ptr.u.ref, buf->read_only);
+ buf->have_grant = 0;
+ }
if ( buf->have_type )
{
put_page_type(buf->page);
@@ -2490,11 +2506,6 @@ static void gnttab_copy_release_buf(stru
put_page(buf->page);
buf->page = NULL;
}
- if ( buf->have_grant )
- {
- release_grant_for_copy(buf->domain, buf->ptr.u.ref, buf->read_only);
- buf->have_grant = 0;
- }
}
static int gnttab_copy_claim_buf(const struct gnttab_copy *op,
["xsa236-4.5.patch" (application/octet-stream)]
From: Jan Beulich <jbeulich@suse.com>
Subject: gnttab: fix pin count / page reference race
Dropping page references before decrementing pin counts is a bad idea
if assumptions are being made that a non-zero pin count implies a valid
page. Fix the order of operations in gnttab_copy_release_buf(), but at
the same time also remove the assertion that was found to trigger:
map_grant_ref() also has the potential of causing a race here, and
changing the order of operations there would likely be quite a bit more
involved.
This is XSA-236.
Reported-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2086,7 +2086,23 @@ __acquire_grant_for_copy(
{
ASSERT(mfn_valid(act->frame));
*page = mfn_to_page(act->frame);
- (void)page_get_owner_and_reference(*page);
+ td = page_get_owner_and_reference(*page);
+ /*
+ * act->pin being non-zero should guarantee the page to have a
+ * non-zero refcount and hence a valid owner (matching the one on
+ * record), with one exception: If the owning domain is dying we
+ * had better not make implications from pin count (map_grant_ref()
+ * updates pin counts before obtaining page references, for
+ * example).
+ */
+ if ( td != rd || rd->is_dying )
+ {
+ if ( td )
+ put_page(*page);
+ *page = NULL;
+ rc = GNTST_bad_domain;
+ goto unlock_out_clear;
+ }
}
act->pin += readonly ? GNTPIN_hstr_inc : GNTPIN_hstw_inc;
@@ -2223,14 +2239,14 @@ __gnttab_copy(
put_page_type(d_pg);
error_out:
- if ( d_pg )
- put_page(d_pg);
- if ( s_pg )
- put_page(s_pg);
if ( have_s_grant )
__release_grant_for_copy(sd, op->source.u.ref, 1);
if ( have_d_grant )
__release_grant_for_copy(dd, op->dest.u.ref, 0);
+ if ( d_pg )
+ put_page(d_pg);
+ if ( s_pg )
+ put_page(s_pg);
if ( sd )
rcu_unlock_domain(sd);
if ( dd )
["xsa236-4.9.patch" (application/octet-stream)]
From: Jan Beulich <jbeulich@suse.com>
Subject: gnttab: fix pin count / page reference race
Dropping page references before decrementing pin counts is a bad idea
if assumptions are being made that a non-zero pin count implies a valid
page. Fix the order of operations in gnttab_copy_release_buf(), but at
the same time also remove the assertion that was found to trigger:
map_grant_ref() also has the potential of causing a race here, and
changing the order of operations there would likely be quite a bit more
involved.
This is XSA-236.
Reported-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2330,9 +2330,20 @@ __acquire_grant_for_copy(
td = page_get_owner_and_reference(*page);
/*
* act->pin being non-zero should guarantee the page to have a
- * non-zero refcount and hence a valid owner.
+ * non-zero refcount and hence a valid owner (matching the one on
+ * record), with one exception: If the owning domain is dying we
+ * had better not make implications from pin count (map_grant_ref()
+ * updates pin counts before obtaining page references, for
+ * example).
*/
- ASSERT(td);
+ if ( td != rd || rd->is_dying )
+ {
+ if ( td )
+ put_page(*page);
+ *page = NULL;
+ rc = GNTST_bad_domain;
+ goto unlock_out_clear;
+ }
}
act->pin += readonly ? GNTPIN_hstr_inc : GNTPIN_hstw_inc;
@@ -2451,6 +2462,11 @@ static void gnttab_copy_release_buf(stru
unmap_domain_page(buf->virt);
buf->virt = NULL;
}
+ if ( buf->have_grant )
+ {
+ __release_grant_for_copy(buf->domain, buf->ptr.u.ref, buf->read_only);
+ buf->have_grant = 0;
+ }
if ( buf->have_type )
{
put_page_type(buf->page);
@@ -2461,11 +2477,6 @@ static void gnttab_copy_release_buf(stru
put_page(buf->page);
buf->page = NULL;
}
- if ( buf->have_grant )
- {
- __release_grant_for_copy(buf->domain, buf->ptr.u.ref, buf->read_only);
- buf->have_grant = 0;
- }
}
static int gnttab_copy_claim_buf(const struct gnttab_copy *op,
[Attachment #7 (text/plain)]
_______________________________________________
Xen-announce mailing list
Xen-announce@lists.xen.org
https://lists.xen.org/xen-announce
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic