Re: [RFC][PATCH 4/5 v2] x86: Keep current stack in NMI breakpoints

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 2011-12-14 at 08:43 -0500, Mathieu Desnoyers wrote:

> What happens to the following sequence ?
> 
> - Hit a breakpoint.
> - Execute an interrupt handler nesting over breakpoint handler (made
>   possible by preempt_conditional_sti(regs) in do_int3()).
>   (or take any kind of fault that switch the current stack)
> - NMI fires, not detecting that it is nested over a breakpoint handler,
>   thus potentially corrupting the DEBUG stack.

Hmm, interesting, because if the interrupt hits a breakpoint, it too
will cause the corruption of the breakpoint stack. I guess this is what
the funky code is trying to protect in the breakpoint assembly, which it
still currently has a race condition too.

> 
> Instead of trying to detect if we nest on a stack to find out if we need
> to change the IDT, I would recommend to unconditionally switch the int3
> IDT to use the current stack upon outermost NMI entry, and set it back
> to its usual behavior upon outermost NMI exit.

I really hate to add overhead to all cases for the most unlikely case.
What would be better, is to fix it in the most unlikely case code, which
would be in the breakpoint handler.

I've been pushing that I don't want the NMI ugliness to spread to other
parts of the code, but this does not have to do with just NMIs. Now we
are having the breakpoint ugliness spread to NMI and vice versa. The
real solution would be to change the do_int3() code.

We could add another IDT table for breakpoints, and before calling the
preempt_conditional_sti(), we update it. This breakpoint IDT will not
change the stack for interrupts. We can still allow NMIs to change, as
that is taken care of. This not only fixes the issue you brought up, but
it also fixes the race that exists today, and we can remove the ugly
code in the int3 assembly.

-- Steve