Re: [PATCH] x86/xpti: Hide almost all of .text and all .data/.rodata/.bss mappings

[Juergen Gross <jgross@suse.com>]

On 13/02/18 20:45, Andrew Cooper wrote:
> The current XPTI implementation isolates the directmap (and therefore a lot of
> guest data), but a large quantity of CPU0's state (including its stack)
> remains visible.
> 
> Furthermore, an attacker able to read .text is in a vastly superior position
> to normal when it comes to fingerprinting Xen for known vulnerabilities, or
> scanning for ROP/Spectre gadgets.
> 
> Collect together the entrypoints in .text.entry (currently 3x4k frames, but
> can almost certainly be slimmed down), and create a common mapping which is
> inserted into each per-cpu shadow.  The stubs are also inserted into this
> mapping by pointing at the in-use L2.  This allows stubs allocated later (SMP
> boot, or CPU hotplug) to work without further changes to the common mappings.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> 
> RFC, because I don't think the stubs handling is particularly sensible.
> 
> We allocate 4k of virtual address space per CPU, but squash loads of CPUs
> together onto a single MFN.  The stubs ought to be isolated as well (as they
> leak the virtual addresses of each stack), which can be done by allocating an
> MFN per CPU (and simplifies cpu_smpboot_alloc() somewhat).  At this point, we
> can't use a common set of mappings, and will have to clone the single stub and
> .entry.text into each PCPUs copy of the pagetables.
> 
> Also, my plan to cause .text.entry to straddle a 512TB boundary (and therefore
> avoid any further pagetable allocations) has come a little unstuck because of
> CONFIG_BIGMEM.  I'm still working out whether there is a sensible way to
> rearrange the virtual layout for this plan to work.
> ---
>  xen/arch/x86/smpboot.c             | 37 ++++++++++++++++++++++++++++++++-----
>  xen/arch/x86/x86_64/compat/entry.S |  2 ++
>  xen/arch/x86/x86_64/entry.S        |  4 +++-
>  xen/arch/x86/xen.lds.S             |  7 +++++++
>  4 files changed, 44 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
> index 2ebef03..2519141 100644
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -622,6 +622,9 @@ unsigned long alloc_stub_page(unsigned int cpu, unsigned long *mfn)
>          unmap_domain_page(memset(__map_domain_page(pg), 0xcc, PAGE_SIZE));
>      }
>  
> +    /* Confirm that all stubs fit in a single L2 pagetable. */
> +    BUILD_BUG_ON(NR_CPUS * PAGE_SIZE > (1u << L2_PAGETABLE_SHIFT));

So we limit NR_CPUS to be max 512 now?

Maybe you should use STUB_BUF_SIZE instead of PAGE_SIZE?

BTW: Is there any reason we don't use a common stub page mapped to each
per-cpu stack area? The stack address can easily be obtained via %rip
relative addressing then (see my patch for the per-vcpu stacks:
https://lists.xen.org/archives/html/xen-devel/2018-02/msg00773.html )


Juergen

> +
>      stub_va = XEN_VIRT_END - (cpu + 1) * PAGE_SIZE;
>      if ( map_pages_to_xen(stub_va, mfn_x(page_to_mfn(pg)), 1,
>                            PAGE_HYPERVISOR_RX | MAP_SMALL_PAGES) )
> @@ -651,9 +654,6 @@ static int clone_mapping(const void *ptr, root_pgentry_t *rpt)
>      l2_pgentry_t *pl2e;
>      l1_pgentry_t *pl1e;
>  
> -    if ( linear < DIRECTMAP_VIRT_START )
> -        return 0;
> -
>      flags = l3e_get_flags(*pl3e);
>      ASSERT(flags & _PAGE_PRESENT);
>      if ( flags & _PAGE_PSE )
> @@ -744,6 +744,9 @@ static __read_mostly int8_t opt_xpti = -1;
>  boolean_param("xpti", opt_xpti);
>  DEFINE_PER_CPU(root_pgentry_t *, root_pgt);
>  
> +static root_pgentry_t common_pgt;
> +extern char _stextentry[], _etextentry[];
> +
>  static int setup_cpu_root_pgt(unsigned int cpu)
>  {
>      root_pgentry_t *rpt;
> @@ -764,8 +767,32 @@ static int setup_cpu_root_pgt(unsigned int cpu)
>          idle_pg_table[root_table_offset(RO_MPT_VIRT_START)];
>      /* SH_LINEAR_PT inserted together with guest mappings. */
>      /* PERDOMAIN inserted during context switch. */
> -    rpt[root_table_offset(XEN_VIRT_START)] =
> -        idle_pg_table[root_table_offset(XEN_VIRT_START)];
> +
> +    /* One-time setup of common_pgt, which maps .text.entry and the stubs. */
> +    if ( unlikely(!root_get_intpte(common_pgt)) )
> +    {
> +        unsigned long stubs_linear = XEN_VIRT_END - 1;
> +        l3_pgentry_t *stubs_main, *stubs_shadow;
> +        char *ptr;
> +
> +        for ( rc = 0, ptr = _stextentry;
> +              !rc && ptr < _etextentry; ptr += PAGE_SIZE )
> +            rc = clone_mapping(ptr, rpt);
> +
> +        if ( rc )
> +            return rc;
> +
> +        stubs_main = l4e_to_l3e(idle_pg_table[l4_table_offset(stubs_linear)]);
> +        stubs_shadow = l4e_to_l3e(rpt[l4_table_offset(stubs_linear)]);
> +
> +        /* Splice into the regular L2 mapping the stubs. */
> +        stubs_shadow[l3_table_offset(stubs_linear)] =
> +            stubs_main[l3_table_offset(stubs_linear)];
> +
> +        common_pgt = rpt[root_table_offset(XEN_VIRT_START)];
> +    }
> +
> +    rpt[root_table_offset(XEN_VIRT_START)] = common_pgt;
>  
>      /* Install direct map page table entries for stack, IDT, and TSS. */
>      for ( off = rc = 0; !rc && off < STACK_SIZE; off += PAGE_SIZE )
> diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
> index 707c746..b001e79 100644
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -13,6 +13,8 @@
>  #include <public/xen.h>
>  #include <irq_vectors.h>
>  
> +        .section .text.entry, "ax", @progbits
> +
>  ENTRY(entry_int82)
>          ASM_CLAC
>          pushq $0
> diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
> index 58f652d..5c5310d 100644
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -14,6 +14,8 @@
>  #include <public/xen.h>
>  #include <irq_vectors.h>
>  
> +        .section .text.entry, "ax", @progbits
> +
>  /* %rbx: struct vcpu */
>  ENTRY(switch_to_kernel)
>          leaq  VCPU_trap_bounce(%rbx),%rdx
> @@ -854,7 +856,7 @@ GLOBAL(autogen_entrypoints)
>          .popsection
>          .endm
>  
> -        .text
> +        .previous
>  autogen_stubs: /* Automatically generated stubs. */
>  
>          vec = 0
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index 0952980..25c6cbc 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -67,6 +67,13 @@ SECTIONS
>         *(.text)
>         *(.text.__x86_indirect_thunk_*)
>         *(.text.page_aligned)
> +
> +       . = ALIGN(PAGE_SIZE);
> +       _stextentry = .;
> +       *(.text.entry)
> +       . = ALIGN(PAGE_SIZE);
> +       _etextentry = .;
> +
>         *(.text.cold)
>         *(.text.unlikely)
>         *(.fixup)
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel