[PATCH] x86/pv: Only use the guest frame in pv_map_ldt_shadow_page()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

pv_map_ldt_shadow_page() takes the whole guest PTE, adds _PAGE_RW to it, then
installs it into Xen's pagetables.  It has had this behaviour ever since LDT
support was added in 2003.

However, it allows the guest to control the software available bits and
cacheability.  This happens to be benign right now, but is bad form.

Use only the guest frame, and construct the mapping as regular RW frame, and
notably includes NX.  This is how the GDT logic already works.

Fixes: 005c2723972f ("Finished virtualisation of x86 LDT")
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Teddy Astie <teddy.astie@xxxxxxxxxx>
CC: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

For 4.22.  The security team have deemed this not an XSA, but it came
alarmingly close.

This was found originally in an LLM review of the ASI series, where a real
vulnerability had been introduced by using one of the software available bits
to mean "please free this page on unmap".  The LLM did not the issues with
attributes (the guest could almost load a Shadow Stack mapping, saved only by
the forced addition of _PAGE_RW), and the cacheability (saved only because of
how conflicting MTRR and PAT values resolve).

An interesting commit is 928a6621db20 ("Fix bug in new LDT shadow mapping
code", 2003) which did restrict to the guest frame only, but without inserting
_PAGE_PRESENT or any other attributes, so got reverted in the following commit
6841936e9256.
---
 xen/arch/x86/pv/mm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/pv/mm.c b/xen/arch/x86/pv/mm.c
index 3f2e9dedcde3..5378299b8cef 100644
--- a/xen/arch/x86/pv/mm.c
+++ b/xen/arch/x86/pv/mm.c
@@ -53,7 +53,7 @@ bool pv_map_ldt_shadow_page(unsigned int offset)
     struct vcpu *curr = current;
     struct domain *currd = curr->domain;
     struct page_info *page;
-    l1_pgentry_t gl1e, *pl1e;
+    l1_pgentry_t gl1e, *pl1e, nl1e;
     unsigned long linear = curr->arch.pv.ldt_base + offset;
 
     BUG_ON(in_irq());
@@ -88,9 +88,9 @@ bool pv_map_ldt_shadow_page(unsigned int offset)
     }
 
     pl1e = &pv_ldt_ptes(curr)[offset >> PAGE_SHIFT];
-    l1e_add_flags(gl1e, _PAGE_RW);
+    nl1e = l1e_from_pfn(l1e_get_pfn(gl1e), __PAGE_HYPERVISOR_RW);
 
-    l1e_write(pl1e, gl1e);
+    l1e_write(pl1e, nl1e);
 
     return true;
 }

base-commit: e3aa330017c533cc312ee4751b8387d988682efc
-- 
2.39.5