diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c index 2383fa6629..99482766b5 100644 --- a/xen/arch/x86/alternative.c +++ b/xen/arch/x86/alternative.c @@ -382,24 +382,28 @@ static int __init cf_check nmi_apply_alternatives( */ if ( !(alt_done & alt_todo) ) { - unsigned long cr0, cr4; - - cr0 = read_cr0(); - cr4 = read_cr4(); - - if ( cr4 & X86_CR4_CET ) - write_cr4(cr4 & ~X86_CR4_CET); - - /* Disable WP to allow patching read-only pages. */ - write_cr0(cr0 & ~X86_CR0_WP); + /* + * Relax perms on .text to be RWX, so we can modify them. + * + * This relaxes perms globally, but we run ahead of bringing APs + * online, so only have our own TLB to worry about. + */ + modify_xen_mappings_lite(XEN_VIRT_START + MB(2), + (unsigned long)&__2M_text_end, + PAGE_HYPERVISOR_RWX); + flush_local(FLUSH_TLB_GLOBAL); _apply_alternatives(__alt_instructions, __alt_instructions_end, alt_done); - write_cr0(cr0); - - if ( cr4 & X86_CR4_CET ) - write_cr4(cr4); + /* + * Reinstate perms on .text to be RX. This also cleans out the dirty + * bits, which matters when CET Shstk is active. + */ + modify_xen_mappings_lite(XEN_VIRT_START + MB(2), + (unsigned long)&__2M_text_end, + PAGE_HYPERVISOR_RX); + flush_local(FLUSH_TLB_GLOBAL); alt_done |= alt_todo; } @@ -454,19 +458,6 @@ static void __init _alternative_instructions(bool force) panic("Timed out waiting for alternatives self-NMI to hit\n"); set_nmi_callback(saved_nmi_callback); - - /* - * When Xen is using shadow stacks, the alternatives clearing CR0.WP and - * writing into the mappings set dirty bits, turning the mappings into - * shadow stack mappings. - * - * While we can execute from them, this would also permit them to be the - * target of WRSS instructions, so reset the dirty after patching. - */ - if ( cpu_has_xen_shstk ) - modify_xen_mappings(XEN_VIRT_START + MB(2), - (unsigned long)&__2M_text_end, - PAGE_HYPERVISOR_RX); } void __init alternative_instructions(void) diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c index f2d783fdc5..a54d991c5f 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -61,46 +61,32 @@ int arch_livepatch_safety_check(void) int noinline 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); + /* + * Relax perms on .text to be RWX, so we can modify them. + * + * This relaxes perms globally, but all other CPUs are waiting on us. + */ + relax_virtual_region_perms(); + flush_local(FLUSH_TLB_GLOBAL); return 0; } void noinline 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. - * - * Any error will result in Xen dying with #CP, and its too late to - * recover in any way. - */ - asm volatile ("rdsspq %[ssp];" - "wrssq %[addr], (%[ssp]);" - : [ssp] "=&r" (tmp) - : [addr] "r" (__builtin_return_address(0))); - } + /* + * Reinstate perms on .text to be RX. This also cleans out the dirty + * bits, which matters when CET Shstk is active. + * + * The other CPUs waiting for us could in principle have re-walked while + * we were patching and cached the reduced perms in their TLB. Therefore, + * we need to do a global TLB flush. + * + * However, we can't use Xen's normal global TLB flush infrastructure, so + * delay the TLB flush to arch_livepatch_post_action(), which is called on + * all CPUs (including us) on the way out of patching. + */ + tighten_virtual_region_perms(); } int arch_livepatch_verify_func(const struct livepatch_func *func) @@ -197,6 +183,8 @@ void noinline arch_livepatch_revert(const struct livepatch_func *func) */ void noinline arch_livepatch_post_action(void) { + /* See arch_livepatch_revive() */ + flush_local(FLUSH_TLB_GLOBAL); } static nmi_callback_t *saved_nmi_callback; diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 36a07ef77e..9741d28cbc 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -91,6 +91,7 @@ #include #include #include +#include #include #include #include @@ -5879,6 +5880,73 @@ int destroy_xen_mappings(unsigned long s, unsigned long e) return modify_xen_mappings(s, e, _PAGE_NONE); } +/* + * Similar to modify_xen_mappings(), but used by the alternatives and + * livepatch in weird contexts. All synchronization, TLB flushing, etc is the + * responsibility of the caller, and *MUST* not be introduced here. + * + * Must be limited to XEN_VIRT_{START,END}, i.e. over l2_xenmap[]. + * Must be called with present flags, and over present mappings. + * It is the callers responsibility to not pass s or e in the middle of + * superpages if changing the permission on the whole superpage is going to be + * a problem. + */ +void init_or_livepatch modify_xen_mappings_lite( + unsigned long s, unsigned long e, unsigned int _nf) +{ + unsigned long v = s, fm, nf; + + /* Set of valid PTE bits which may be altered. */ +#define FLAGS_MASK (_PAGE_NX|_PAGE_DIRTY|_PAGE_ACCESSED|_PAGE_RW|_PAGE_PRESENT) + fm = put_pte_flags(FLAGS_MASK); + nf = put_pte_flags(_nf & FLAGS_MASK); +#undef FLAGS_MASK + + ASSERT(nf & _PAGE_PRESENT); + ASSERT(IS_ALIGNED(s, PAGE_SIZE) && s >= XEN_VIRT_START); + ASSERT(IS_ALIGNED(e, PAGE_SIZE) && e <= XEN_VIRT_END); + + while ( v < e ) + { + l2_pgentry_t *pl2e = &l2_xenmap[l2_table_offset(v)]; + l2_pgentry_t l2e = l2e_read_atomic(pl2e); + unsigned int l2f = l2e_get_flags(l2e); + + ASSERT(l2f & _PAGE_PRESENT); + + if ( l2e_get_flags(l2e) & _PAGE_PSE ) + { + l2e_write_atomic(pl2e, l2e_from_intpte((l2e.l2 & ~fm) | nf)); + + v += 1UL << L2_PAGETABLE_SHIFT; + continue; + } + + /* else descend to l1 */ + { + l1_pgentry_t *pl1t = map_l1t_from_l2e(l2e); + + while ( v < e ) + { + l1_pgentry_t *pl1e = &pl1t[l1_table_offset(v)]; + l1_pgentry_t l1e = l1e_read_atomic(pl1e); + unsigned int l1f = l1e_get_flags(l1e); + + ASSERT(l1f & _PAGE_PRESENT); + + l1e_write_atomic(pl1e, l1e_from_intpte((l1e.l1 & ~fm) | nf)); + + v += 1UL << L1_PAGETABLE_SHIFT; + + if ( l2_table_offset(v) == 0 ) + break; + } + + unmap_domain_page(pl1t); + } + } +} + void __set_fixmap( enum fixed_addresses idx, unsigned long mfn, unsigned long flags) { diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c index 30b0b4ab9c..5f89703f51 100644 --- a/xen/common/virtual_region.c +++ b/xen/common/virtual_region.c @@ -92,16 +92,28 @@ void unregister_virtual_region(struct virtual_region *r) remove_virtual_region(r); } -#if defined(CONFIG_LIVEPATCH) && defined(CONFIG_XEN_SHSTK) -void reset_virtual_region_perms(void) +#if defined(CONFIG_LIVEPATCH) && defined(CONFIG_X86) +void relax_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); + modify_xen_mappings_lite((unsigned long)region->start, + ROUNDUP((unsigned long)region->end, PAGE_SIZE), + PAGE_HYPERVISOR_RWX); + rcu_read_unlock(&rcu_virtual_region_lock); +} + +void tighten_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_lite((unsigned long)region->start, + ROUNDUP((unsigned long)region->end, PAGE_SIZE), + PAGE_HYPERVISOR_RX); rcu_read_unlock(&rcu_virtual_region_lock); } #endif diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index 9d14aed74b..b0dc3ba9c9 100644 --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -100,6 +100,7 @@ int map_pages_to_xen( unsigned int flags); /* Alter the permissions of a range of Xen virtual address space. */ int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags); +void modify_xen_mappings_lite(unsigned long s, unsigned long e, unsigned int flags); int destroy_xen_mappings(unsigned long s, unsigned long e); /* Retrieve the MFN mapped by VA in Xen virtual address space. */ mfn_t xen_map_to_mfn(unsigned long va); diff --git a/xen/include/xen/virtual_region.h b/xen/include/xen/virtual_region.h index ba408eb87a..d053620711 100644 --- a/xen/include/xen/virtual_region.h +++ b/xen/include/xen/virtual_region.h @@ -33,7 +33,9 @@ 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); + +void relax_virtual_region_perms(void); +void tighten_virtual_region_perms(void); #endif /* __XEN_VIRTUAL_REGION_H__ */