[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen stable-4.9] gnttab: fix pin count / page reference race
commit 174a5690708a874b889f9436e82632b1187faea7 Author: Jan Beulich <jbeulich@xxxxxxxx> AuthorDate: Tue Oct 24 16:06:14 2017 +0200 Commit: Jan Beulich <jbeulich@xxxxxxxx> CommitDate: Tue Oct 24 16:06:14 2017 +0200 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 CVE-2017-15597 / XSA-236. Reported-by: Pawel Wieczorkiewicz <wipawel@xxxxxxxxx> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> master commit: e008f7619dcd6d549727c9635b3f9f3c7ee483ed master date: 2017-10-24 16:01:33 +0200 --- xen/common/grant_table.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index 68c94b6..8955e5c 100644 --- 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(struct gnttab_copy_buf *buf) 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(struct gnttab_copy_buf *buf) 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, -- generated by git-patchbot for /home/xen/git/xen.git#stable-4.9 _______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxx https://lists.xenproject.org/xen-changelog
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |