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

[PATCH for-4.14] x86/livepatch: Make livepatching compatible with CET Shadow Stacks



Just like the alternatives infrastructure, the livepatch infrastructure
disables CR0.WP to perform patching, which is not permitted with CET active.

Modify arch_livepatch_{quiesce,revive}() to disable CET before disabling WP,
and reset the dirty bits on all virtual regions before re-enabling CET.  One
complication is that arch_livepatch_revive() has to fix up the top of the
shadow stack.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
CC: Pawel Wieczorkiewicz <wipawel@xxxxxxxxx>
CC: Paul Durrant <paul@xxxxxxx>

For 4.14.  This is a bug in a 4.14 feature, with a very low risk to non-CET
usecases.

A better longterm plan (but definitely 4.15 material now) would be to create a
separate set of writeable mappings to perform the patching on, at which point
we don't need to disable CET, play with WP, or retroactively clear dirty bits.

Do we ever write into .rodata?  AFAICT, we introduce new fuctions for
references to new .rodata, rather than modifying existing .rodata.  If however
we do modify .rodata, then the virtual regions need extending with information
about .rodata so we can zap those dirty bits as well.
---
 xen/arch/x86/livepatch.c         | 28 ++++++++++++++++++++++++++++
 xen/common/virtual_region.c      | 13 +++++++++++++
 xen/include/xen/virtual_region.h |  1 +
 3 files changed, 42 insertions(+)

diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 901fad96bf..10231a4e40 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -12,6 +12,7 @@
 #include <xen/livepatch.h>
 #include <xen/sched.h>
 #include <xen/vm_event.h>
+#include <xen/virtual_region.h>
 
 #include <asm/fixmap.h>
 #include <asm/nmi.h>
@@ -58,6 +59,10 @@ int arch_livepatch_safety_check(void)
 
 int arch_livepatch_quiesce(void)
 {
+    /* If Shadow Stacks are in use, disable CR4.CET so we can modify CR0.WP. */
+    if ( cpu_has_xen_shstk )
+        write_cr4(read_cr4() & ~X86_CR4_CET);
+
     /* Disable WP to allow changes to read-only pages. */
     write_cr0(read_cr0() & ~X86_CR0_WP);
 
@@ -68,6 +73,29 @@ void arch_livepatch_revive(void)
 {
     /* Reinstate WP. */
     write_cr0(read_cr0() | X86_CR0_WP);
+
+    /* Clobber dirty bits and reinstate CET, if applicable. */
+    if ( IS_ENABLED(CONFIG_XEN_SHSTK) && cpu_has_xen_shstk )
+    {
+        unsigned long tmp;
+
+        reset_virtual_region_perms();
+
+        write_cr4(read_cr4() | X86_CR4_CET);
+
+        /*
+         * Fix up the return address on the shadow stack, which currently
+         * points at arch_livepatch_quiesce()'s caller.
+         *
+         * Note: this is somewhat fragile, and depends on both
+         * arch_livepatch_{quiesce,revive}() being called from the same
+         * function, which is currently the case.
+         */
+        asm volatile ("rdsspq %[ssp];"
+                      "wrssq %[addr], (%[ssp]);"
+                      : [ssp] "=&r" (tmp)
+                      : [addr] "r" (__builtin_return_address(0)));
+    }
 }
 
 int arch_livepatch_verify_func(const struct livepatch_func *func)
diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c
index aa23918bce..84d993d8f8 100644
--- a/xen/common/virtual_region.c
+++ b/xen/common/virtual_region.c
@@ -4,6 +4,7 @@
 
 #include <xen/init.h>
 #include <xen/kernel.h>
+#include <xen/mm.h>
 #include <xen/rcupdate.h>
 #include <xen/spinlock.h>
 #include <xen/virtual_region.h>
@@ -91,6 +92,18 @@ void unregister_virtual_region(struct virtual_region *r)
     remove_virtual_region(r);
 }
 
+void reset_virtual_region_perms(void)
+{
+    const struct virtual_region *region;
+
+    rcu_read_lock(&rcu_virtual_region_lock);
+    list_for_each_entry_rcu( region, &virtual_region_list, list )
+        modify_xen_mappings((unsigned long)region->start,
+                            ROUNDUP((unsigned long)region->end, PAGE_SIZE),
+                            PAGE_HYPERVISOR_RX);
+    rcu_read_unlock(&rcu_virtual_region_lock);
+}
+
 void __init unregister_init_virtual_region(void)
 {
     BUG_ON(system_state != SYS_STATE_active);
diff --git a/xen/include/xen/virtual_region.h b/xen/include/xen/virtual_region.h
index e5e58ed96b..ba408eb87a 100644
--- a/xen/include/xen/virtual_region.h
+++ b/xen/include/xen/virtual_region.h
@@ -33,6 +33,7 @@ void setup_virtual_regions(const struct exception_table_entry 
*start,
 void unregister_init_virtual_region(void);
 void register_virtual_region(struct virtual_region *r);
 void unregister_virtual_region(struct virtual_region *r);
+void reset_virtual_region_perms(void);
 
 #endif /* __XEN_VIRTUAL_REGION_H__ */
 
-- 
2.11.0




 


Rackspace

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