On Sat, Nov 15, 2014 at 10:28 AM, Andi Kleen <a...@firstfloor.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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
