On Fri, Apr 05, 2019 at 05:07:17PM +0200, Thomas Gleixner wrote:
> The debug IST stack is actually two separate debug stacks to handle #DB
> recursion. This is required because the CPU starts always at top of stack
> on exception entry, which means on #DB recursion the second #DB would
> overwrite the stack of the first.
>
> The low level entry code therefore adjusts the top of stack on entry so a
> secondary #DB starts from a different stack page. But the stack pages are
> adjacent without a guard page between them.
>
> Split the debug stack into 3 stacks which are separated by guard pages. The
> 3rd stack is never mapped into the cpu_entry_area and is only there to
> catch triple #DB nesting:
>
> --- top of DB_stack <- Initial stack
> --- end of DB_stack
> guard page
>
> --- top of DB1_stack <- Top of stack after entering first #DB
> --- end of DB1_stack
> guard page
>
> --- top of DB2_stack <- Top of stack after entering second #DB
> --- end of DB2_stack
> guard page
>
> If DB2 would not act as the final guard hole, a second #DB would point the
> top of #DB stack to the stack below #DB1 which would be valid and not catch
> the not so desired triple nesting.
>
> The backing store does not allocate any memory for DB2 and its guard page
> as it is not going to be mapped into the cpu_entry_area.
>
> - Adjust the low level entry code so it adjusts top of #DB with the offset
> between the stacks instead of exception stack size.
>
> - Make the dumpstack code aware of the new stacks.
>
> - Adjust the in_debug_stack() implementation and move it into the NMI code
> where it belongs. As this is NMI hotpath code, it just checks the full
> area between top of DB_stack and bottom of DB1_stack without checking
> for the guard page. That's correct because the NMI cannot hit a
> stackpointer pointing to the guard page between DB and DB1 stack. Even
> if it would, then the NMI operation still is unaffected, but the resume
> of the debug exception on the topmost DB stack will crash by touching
> the guard page.
>
> Suggested-by: Andy Lutomirski <l...@kernel.org>
> Signed-off-by: Thomas Gleixner <t...@linutronix.de>
> ---
...
> +static bool notrace is_debug_stack(unsigned long addr)
> +{
> + struct cea_exception_stacks *cs = __this_cpu_read(cea_exception_stacks);
> + unsigned long top = CEA_ESTACK_TOP(cs, DB);
> + unsigned long bot = CEA_ESTACK_BOT(cs, DB1);
> +
> + if (__this_cpu_read(debug_stack_usage))
> + return true;
> + /*
> + * Note, this covers the guard page between DB and DB1 as well to
> + * avoid two checks. But by all means @addr can never point into
> + * the guard page.
> + */
> + return addr > bot && addr < top;
Isn't this an off by one error? I.e. "return addr >= bot && addr < top".
%rsp == bot is technically still in the DB1 stack even though the next
PUSH/CALL will explode on the guard page.
> +}
> +NOKPROBE_SYMBOL(is_debug_stack);
> #endif
>
> dotraplinkage notrace void
> --- a/arch/x86/mm/cpu_entry_area.c
> +++ b/arch/x86/mm/cpu_entry_area.c
> @@ -98,10 +98,12 @@ static void __init percpu_setup_exceptio
>
> /*
> * The exceptions stack mappings in the per cpu area are protected
> - * by guard pages so each stack must be mapped separately.
> + * by guard pages so each stack must be mapped separately. DB2 is
> + * not mapped; it just exists to catch triple nesting of #DB.
> */
> cea_map_stack(DF);
> cea_map_stack(NMI);
> + cea_map_stack(DB1);
> cea_map_stack(DB);
> cea_map_stack(MCE);
> }
>
>
