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] [day] [month] [year] [list]
Date:	Sat, 15 Nov 2014 10:45:19 -0800
From:	Andy Lutomirski <luto@...capital.net>
To:	Andi Kleen <andi@...stfloor.org>
Cc:	"Luck, Tony" <tony.luck@...el.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...en8.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Request for help: what did I do wrong with idtentry?

On Sat, Nov 15, 2014 at 10:28 AM, Andi Kleen <andi@...stfloor.org> wrote:
>> I'm not
>> completely thrilled with what it does to double_fault, though.  If we
>> somehow get a double fault caused by an interrupt hitting userspace
>> with a bad kernel_stack, then we'll end up page faulting in the
>> double_fault prologue.  I'm not convinced that this is worth worrying
>> about.  It would be easy enough to fix, though, even if it would
>> further uglify the code.
>
> If you're "cleaning up" good and working code the functionality should
> be the same as before. The old code handled this situation fine.
> So your new code should handle this too.

First, this failure mode should be almost impossible.  We'd really
have to screw up to have the kernel stack point to a bad address.
(This isn't the stack *pointer* being bad -- it's the value in the
TSS.)

If this happens, the existing code will die (no recovery possible
unlike with normal OOPSes).  The new code will log a kernel-mode page
fault on the DF stack (as shown on the stack trace, assuming that
logic works), complain some more in do_exit, and make some sort of
effort to recover, which might even work.

In other words, I'd be happy to "fix" it, but I'm not entirely
convinced that this change should count as a regression in the first
place.

If we go for the fix-it approach, we could add a fixup in sync_regs
and probe the kernel_stack or we could add a paranoid=2 mode for
double_fault.

>
> In general yes handling all the corner cases makes code ugly.
> That is how the existing code got how it became.

Most of those corner cases are at least in code paths that are
supposed to work.  This particular corner case is in a handler that's
just trying to print something useful rather than silently rebooting,
and it should still work well enough to print something useful.

--Andy
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ