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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1323879550.23971.31.camel@gandalf.stny.rr.com>
Date:	Wed, 14 Dec 2011 11:19:10 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"H. Peter Anvin" <hpa@...or.com>, Andi Kleen <andi@...stfloor.org>
Subject: Re: [RFC][PATCH 4/5 v2] x86: Keep current stack in NMI breakpoints

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/