All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Manuel Bouyer" <bouyer@antioche.eu.org>
Subject: [PATCH] Revert "x86/mm: drop guest_get_eff_l1e()"
Date: Fri, 11 Dec 2020 14:16:15 +0000	[thread overview]
Message-ID: <20201211141615.12489-1-andrew.cooper3@citrix.com> (raw)

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@antioche.eu.org>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Manuel Bouyer <bouyer@antioche.eu.org>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Manuel Bouyer <bouyer@antioche.eu.org>

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



             reply	other threads:[~2020-12-11 15:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-11 14:16 Andrew Cooper [this message]
2020-12-14  8:27 ` [PATCH] Revert "x86/mm: drop guest_get_eff_l1e()" Jan Beulich
2020-12-14 13:21   ` Andrew Cooper
2020-12-14 13:55     ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201211141615.12489-1-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=bouyer@antioche.eu.org \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.