[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] Revert "x86/mm: drop guest_get_eff_l1e()"


  • To: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Fri, 11 Dec 2020 14:16:15 +0000
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Manuel Bouyer <bouyer@xxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 11 Dec 2020 14:16:57 +0000
  • Ironport-sdr: K/hnHrSfABfZTpsNhA/pGdRyqzpZOXPhq4DhGpjVbeyslbvyAzd555Hm/LJfPGmox3HTuwdg0B 9Z7WudJnerU5NJImmkowFvFY77OTACjec7QwG9FK1GKfACUzuWs6Kw5tgFybJ5KnSq9+xkYqGQ 3eNrwh9MDHkpxez4suqGwhE9g4GVv5goSUca0JDb8jKTjGN2Zu/u2HPt2susyjNerec37Mlf6C UaqviWFTx5e2fN8PMELHvoyP4H5A6jbq/b+Fh8k+LHb0sO6Utz6oBIy/FezKO3dDTtVt/6GyI2 OO4=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

This reverts commit 9ff9705647646aa937b5f5c1426a64c69a62b3bd.

The change is only correct in the original context of XSA-286, where Xen's use
of the linear pagetables were dropped.  However, performance problems
interfered with that plan, and XSA-286 was fixed differently.

This broke Xen's lazy faulting of the LDT for 64bit PV guests when an access
was first encountered in user context.  Xen would proceed to read the
registered LDT virtual address out of the user pagetables, not the kernel
pagetables.

Given the nature of the bug, it would have also interfered with the IO
permisison bitmap functionality of userspace, which similarly needs to read
data using the kernel pagetables.

Reported-by: Manuel Bouyer <bouyer@xxxxxxxxxxxxxxx>
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Tested-by: Manuel Bouyer <bouyer@xxxxxxxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Manuel Bouyer <bouyer@xxxxxxxxxxxxxxx>

There is also a bug with Xen's IRET handling, but that has been broken forever
and is much more complicated to fix.  I'll put it on my TODO list, but no idea
when I'll get around to addressing it.
---
 xen/arch/x86/pv/mm.c            | 21 +++++++++++++++++++++
 xen/arch/x86/pv/mm.h            |  7 ++-----
 xen/arch/x86/pv/ro-page-fault.c |  2 +-
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/pv/mm.c b/xen/arch/x86/pv/mm.c
index 5d74d11cba..14cb0f2d4e 100644
--- a/xen/arch/x86/pv/mm.c
+++ b/xen/arch/x86/pv/mm.c
@@ -56,6 +56,27 @@ l1_pgentry_t *map_guest_l1e(unsigned long linear, mfn_t 
*gl1mfn)
 }
 
 /*
+ * Read the guest's l1e that maps this address, from the kernel-mode
+ * page tables.
+ */
+static l1_pgentry_t guest_get_eff_kern_l1e(unsigned long linear)
+{
+    struct vcpu *curr = current;
+    const bool user_mode = !(curr->arch.flags & TF_kernel_mode);
+    l1_pgentry_t l1e;
+
+    if ( user_mode )
+        toggle_guest_pt(curr);
+
+    l1e = guest_get_eff_l1e(linear);
+
+    if ( user_mode )
+        toggle_guest_pt(curr);
+
+    return l1e;
+}
+
+/*
  * Map a guest's LDT page (covering the byte at @offset from start of the LDT)
  * into Xen's virtual range.  Returns true if the mapping changed, false
  * otherwise.
diff --git a/xen/arch/x86/pv/mm.h b/xen/arch/x86/pv/mm.h
index 2a21859dd4..b1b66e46c8 100644
--- a/xen/arch/x86/pv/mm.h
+++ b/xen/arch/x86/pv/mm.h
@@ -5,11 +5,8 @@ l1_pgentry_t *map_guest_l1e(unsigned long linear, mfn_t 
*gl1mfn);
 
 int new_guest_cr3(mfn_t mfn);
 
-/*
- * Read the guest's l1e that maps this address, from the kernel-mode
- * page tables.
- */
-static inline l1_pgentry_t guest_get_eff_kern_l1e(unsigned long linear)
+/* Read a PV guest's l1e that maps this linear address. */
+static inline l1_pgentry_t guest_get_eff_l1e(unsigned long linear)
 {
     l1_pgentry_t l1e;
 
diff --git a/xen/arch/x86/pv/ro-page-fault.c b/xen/arch/x86/pv/ro-page-fault.c
index 8d0007ede5..7f6fbc92fb 100644
--- a/xen/arch/x86/pv/ro-page-fault.c
+++ b/xen/arch/x86/pv/ro-page-fault.c
@@ -342,7 +342,7 @@ int pv_ro_page_fault(unsigned long addr, struct 
cpu_user_regs *regs)
     bool mmio_ro;
 
     /* Attempt to read the PTE that maps the VA being accessed. */
-    pte = guest_get_eff_kern_l1e(addr);
+    pte = guest_get_eff_l1e(addr);
 
     /* We are only looking for read-only mappings */
     if ( ((l1e_get_flags(pte) & (_PAGE_PRESENT | _PAGE_RW)) != _PAGE_PRESENT) )
-- 
2.11.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.