Messages in this thread | | | Subject | Re: [RFC][PATCH 4/5 v2] x86: Keep current stack in NMI breakpoints | From | Steven Rostedt <> | Date | Wed, 14 Dec 2011 11:19:10 -0500 |
| |
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
| |