On Mon, Jun 15, 2020 at 02:08:16PM -0700, Andy Lutomirski wrote:
> > All !user exceptions really should be NMI-like. If you want to go
> > overboard, I suppose you can look at IF and have them behave interrupt
> > like when set, but why make things complicated.
>
> This entire rabbit hole opened because of #PF. So we at least need the
> set of exceptions that are permitted to schedule if they came from
> kernel mode to remain schedulable.
What exception, other than #PF, actually needs to schedule from kernel?
> Prior to the giant changes, all the non-IST *exceptions*, but not the
> interrupts, were schedulable from kernel mode, assuming the original
> context could schedule. Right now, interrupts can schedule, too, which
> is nice if we ever want to fully clean up the Xen abomination. I
> suppose we could make it so #PF opts in to special treatment again,
> but we should decide that the result is simpler or otherwise better
> before we do this.
>
> One possible justification would be that the schedulable entry variant
> is more complicated, and most kernel exceptions except the ones with
> fixups are bad news, and we want the oopses to succeed. But page
> faults are probably the most common source of oopses, so this is a bit
> weak, and we really want page faults to work even from nasty contexts.
I think I'd prefer the argument of consistent failure.
Do we ever want #UD to schedule? If not, then why allow it to sometimes
schedule and sometimes fail, better to always fail.
#DB is still a giant trainwreck in this regard as well.
Something like this...
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -216,10 +216,25 @@ static inline void handle_invalid_op(str
ILL_ILLOPN, error_get_trap_addr(regs));
}
-DEFINE_IDTENTRY_RAW(exc_invalid_op)
+static void handle_invalid_op_kernel(struct pt_regs *regs)
+{
+ if (is_valid_bugaddr(regs->ip) &&
+ report_bug(regs->ip, regs) == BUG_TRAP_TYPE_WARN) {
+ /* Skip the ud2. */
+ regs->ip += LEN_UD2;
+ return;
+ }
+
+ handle_invalid_op(regs);
+}
+
+static void handle_invalid_op_user(struct pt_regs *regs)
{
- bool rcu_exit;
+ handle_invalid_op(regs);
+}
+DEFINE_IDTENTRY_RAW(exc_invalid_op)
+{
/*
* Handle BUG/WARN like NMIs instead of like normal idtentries:
* if we bugged/warned in a bad RCU context, for example, the last
@@ -227,38 +242,25 @@ DEFINE_IDTENTRY_RAW(exc_invalid_op)
* infinitum.
*/
if (!user_mode(regs)) {
- enum bug_trap_type type = BUG_TRAP_TYPE_NONE;
-
nmi_enter();
instrumentation_begin();
trace_hardirqs_off_finish();
- if (is_valid_bugaddr(regs->ip))
- type = report_bug(regs->ip, regs);
+ handle_invalid_op_kernel(regs);
if (regs->flags & X86_EFLAGS_IF)
trace_hardirqs_on_prepare();
instrumentation_end();
nmi_exit();
+ } else {
+ bool rcu_exit;
- if (type == BUG_TRAP_TYPE_WARN) {
- /* Skip the ud2. */
- regs->ip += LEN_UD2;
- return;
- }
-
- /*
- * Else, if this was a BUG and report_bug returns or if this
- * was just a normal #UD, we want to continue onward and
- * crash.
- */
+ rcu_exit = idtentry_enter_cond_rcu(regs);
+ instrumentation_begin();
+ handle_invalid_op_user(regs);
+ instrumentation_end();
+ idtentry_exit_cond_rcu(regs, rcu_exit);
}
-
- rcu_exit = idtentry_enter_cond_rcu(regs);
- instrumentation_begin();
- handle_invalid_op(regs);
- instrumentation_end();
- idtentry_exit_cond_rcu(regs, rcu_exit);
}
DEFINE_IDTENTRY(exc_coproc_segment_overrun)
