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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.03.18 at 12:51, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 02/03/18 11:24, Jan Beulich wrote:
>> --- 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.

Oops, of course. How did I miss that?

>> --- 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.

That would no longer convey to the reader that one such page
has to remain. How about

@@ -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 - Optionally not present (MEMORY_GUARD)
+ * 4 - Unused; optionally not present (MEMORY_GUARD)
+ * 3 - Unused; optionally not present (MEMORY_GUARD)
  * 2 - MCE IST stack
  * 1 - NMI IST stack
  * 0 - Double Fault IST stack

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

Thanks, but for the above please let me know.

Jan


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