Re: [Xen-devel] [PATCH] x86: guard more stack pages

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/03/18 11:24, Jan Beulich wrote:
> There's no reason to keep the unused pages (of which there are actually
> two; respective commentary also gets adjusted) mapped.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5563,10 +5563,11 @@ void memguard_unguard_range(void *p, uns
>  
>  void memguard_guard_stack(void *p)
>  {
> -    BUILD_BUG_ON((PRIMARY_STACK_SIZE + PAGE_SIZE) > STACK_SIZE);
> -    p = (void *)((unsigned long)p + STACK_SIZE -
> -                 PRIMARY_STACK_SIZE - PAGE_SIZE);
> -    memguard_guard_range(p, PAGE_SIZE);
> +    /* IST_MAX IST pages + at least 1 guard page + primary stack. */
> +    BUILD_BUG_ON((IST_MAX + 1) * PAGE_SIZE + PRIMARY_STACK_SIZE > 
> STACK_SIZE);
> +
> +    memguard_guard_range(p + IST_MAX * PAGE_SIZE,
> +                         STACK_SIZE - PRIMARY_STACK_SIZE - IST_MAX * 
> PAGE_SIZE);
>  }
>  
>  void memguard_unguard_stack(void *p)

You need an equivalent adjustment in this function.

> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -301,8 +301,8 @@ void subarch_percpu_traps_init(void)
>      unsigned char *stub_page;
>      unsigned int offset;
>  
> -    /* IST_MAX IST pages + 1 syscall page + 1 guard page + primary stack. */
> -    BUILD_BUG_ON((IST_MAX + 2) * PAGE_SIZE + PRIMARY_STACK_SIZE > 
> STACK_SIZE);
> +    /* IST_MAX IST pages + at least 1 guard page + primary stack. */
> +    BUILD_BUG_ON((IST_MAX + 1) * PAGE_SIZE + PRIMARY_STACK_SIZE > 
> STACK_SIZE);
>  
>      stub_page = map_domain_page(_mfn(this_cpu(stubs.mfn)));
>  
> --- a/xen/include/asm-x86/current.h
> +++ b/xen/include/asm-x86/current.h
> @@ -16,9 +16,9 @@
>   *
>   * 7 - Primary stack (with a struct cpu_info at the top)
>   * 6 - Primary stack
> - * 5 - Optionally not preset (MEMORY_GUARD)
> + * 5 - Optionally not present (MEMORY_GUARD)
>   * 4 - unused
> - * 3 - Syscall trampolines
> + * 3 - unused
>   * 2 - MCE IST stack
>   * 1 - NMI IST stack
>   * 0 - Double Fault IST stack

How about:

diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index 4678a0f..58fa703 100644
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -16,9 +16,9 @@
  *
  * 7 - Primary stack (with a struct cpu_info at the top)
  * 6 - Primary stack
- * 5 - Optionally not preset (MEMORY_GUARD)
- * 4 - unused
- * 3 - Syscall trampolines
+ * 5 - unused \
+ * 4 - unused  } Optionally not present (MEMORY_GUARD)
+ * 3 - unused /
  * 2 - MCE IST stack
  * 1 - NMI IST stack
  * 0 - Double Fault IST stack

Which highlights that all 3 unused pages now get unmapped.

With at least the first issue fixed, Reviewed-by: Andrew Cooper
<andrew.cooper3@xxxxxxxxxx>

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