From: Jan Beulich <jbeulich@suse.com>
Date: Thu, 16 Nov 2017 11:18:27 +0000 (+0100)
Subject: x86: don't wrongly trigger linear page table assertion
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=b0be3c2e4c710a02adf17e1b858b0743ce154653;p=xen.git

x86: don't wrongly trigger linear page table assertion

_put_page_type() may do multiple iterations until its cmpxchg()
succeeds. It invokes set_tlbflush_timestamp() on the first
iteration, however. Code inside the function takes care of this, but
- the assertion in _put_final_page_type() would trigger on the second
  iteration if time stamps in a debug build are permitted to be
  sufficiently much wider than the default 6 bits (see WRAP_MASK in
  flushtlb.c),
- it returning -EINTR (for a continuation to be scheduled) would leave
  the page inconsistent state (until the re-invocation completes).
Make the set_tlbflush_timestamp() invocation conditional, bypassing it
(for now) only in the case we really can't tolerate the stamp to be
stored.

This is part of XSA-240.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
---

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 41d13a475c..54374c5ad7 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2408,29 +2408,20 @@ static int _put_page_type(struct page_info *page, bool_t preemptible,
                 break;
             }
 
-            if ( ptpg && PGT_type_equal(x, ptpg->u.inuse.type_info) )
-            {
-                /*
-                 * set_tlbflush_timestamp() accesses the same union
-                 * linear_pt_count lives in. Unvalidated page table pages,
-                 * however, should occur during domain destruction only
-                 * anyway.  Updating of linear_pt_count luckily is not
-                 * necessary anymore for a dying domain.
-                 */
-                ASSERT(page_get_owner(page)->is_dying);
-                ASSERT(page->linear_pt_count < 0);
-                ASSERT(ptpg->linear_pt_count > 0);
-                ptpg = NULL;
-            }
-
             /*
              * Record TLB information for flush later. We do not stamp page
              * tables when running in shadow mode:
              *  1. Pointless, since it's the shadow pt's which must be tracked.
              *  2. Shadow mode reuses this field for shadowed page tables to
              *     store flags info -- we don't want to conflict with that.
+             * Also page_set_tlbflush_timestamp() accesses the same union
+             * linear_pt_count lives in. Pages (including page table ones),
+             * however, don't need their flush time stamp set except when
+             * the last reference is being dropped. For page table pages
+             * this happens in _put_final_page_type().
              */
-            if ( !(shadow_mode_enabled(page_get_owner(page)) &&
+            if ( (!ptpg || !PGT_type_equal(x, ptpg->u.inuse.type_info)) &&
+                 !(shadow_mode_enabled(page_get_owner(page)) &&
                    (page->count_info & PGC_page_table)) )
                 page_set_tlbflush_timestamp(page);
         }