File xsa299-5.patch of Package xen
Subject: x86/mm: Rework get_page_and_type_from_mfn conditional
From: George Dunlap george.dunlap@citrix.com Mon Nov 4 15:04:36 2019 +0100
Date: Mon Nov 4 15:04:36 2019 +0100:
Git: 44303c6efe19bff9712cee3bb04906b011e7e3ef
Make it easier to read by declaring the conditions in which we will
retain the ref, rather than the conditions under which we release it.
The only way (page == current->arch.old_guest_table) can be true is if
preemptible is true; so remove this from the query itself, and add an
ASSERT() to that effect on the opposite path.
No functional change intended.
NB that alloc_lN_table() mishandle the "linear pt failure" situation
described in the comment; this will be addressed in a future patch.
This is part of XSA-299.
Reported-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 2aab06d742e13d7a9d248f1fc7f0ec62b295ada1
master date: 2019-10-31 16:13:23 +0100
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 2cf0c33d18..c430f2c52e 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -798,8 +798,43 @@ static int get_page_and_type_from_pagenr(unsigned long page_nr,
rc = __get_page_type(page, type, preemptible);
- if ( unlikely(rc) && !partial_ref &&
- (!preemptible || page != current->arch.old_guest_table) )
+ /*
+ * Retain the refcount if:
+ * - page is fully validated (rc == 0)
+ * - page is not validated (rc < 0) but:
+ * - We came in with a reference (partial_ref)
+ * - page is partially validated but there's been an error
+ * (page == current->arch.old_guest_table)
+ *
+ * The partial_ref-on-error clause is worth an explanation. There
+ * are two scenarios where partial_ref might be true coming in:
+ * - mfn has been partially demoted as type `type`; i.e. has
+ * PGT_partial set
+ * - mfn has been partially demoted as L(type+1) (i.e., a linear
+ * page; e.g. we're being called from get_page_from_l2e with
+ * type == PGT_l1_table, but the mfn is PGT_l2_table)
+ *
+ * If there's an error, in the first case, _get_page_type will
+ * either return -ERESTART, in which case we want to retain the
+ * ref (as the caller will consider it retained), or -EINVAL, in
+ * which case old_guest_table will be set; in both cases, we need
+ * to retain the ref.
+ *
+ * In the second case, if there's an error, _get_page_type() can
+ * *only* return -EINVAL, and *never* set old_guest_table. In
+ * that case we also want to retain the reference, to allow the
+ * page to continue to be torn down (i.e., PGT_partial cleared)
+ * safely.
+ *
+ * Also note that we shouldn't be able to leave with the reference
+ * count retained unless we succeeded, or the operation was
+ * preemptible.
+ */
+ if ( likely(!rc) || partial_ref )
+ /* nothing */;
+ else if ( page == current->arch.old_guest_table )
+ ASSERT(preemptible);
+ else
put_page(page);
return rc;