On Mon, Jun 15, 2020 at 03:46:00PM -0700, Andy Lutomirski wrote: > In some sense, #UD and #PF are fundamentally different. #PF wants to > be able to schedule in the kernel. #UD wants to be as minimal as > possible in the kernel but probably still wants to do the nmi_enter() > dance in case it's an RCU warning and the warning handler code wants > to use RCU. > > One solution would be to get rid of ud2 for warnings and replace it > with CALL warning_thunk :) But I guess I'm okay with your patch. Well, the raisin we use UD2 is because it's only 2 bytes, which makes for nice and compact code. Ideally we'd have a single byte #UD instruction, but alas. However, I realized that there's another analogy with #PF that does transfer to #UD. For #PF we state that in-kernel #PF only happens when RCU is already watching -- by virtue of us being careful in noinstr. But similarly we can state we only have UD2 when we want to call WARN/BUG and can forgo exception entry. That would then result in something like this... --- diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index af75109485c2..8fe57b07a03b 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -216,40 +216,35 @@ static inline void handle_invalid_op(struct pt_regs *regs) ILL_ILLOPN, error_get_trap_addr(regs)); } -DEFINE_IDTENTRY_RAW(exc_invalid_op) +static noinstr bool handle_bug(struct pt_regs *regs) { - bool rcu_exit; + bool handled = false; /* - * Handle BUG/WARN like NMIs instead of like normal idtentries: - * if we bugged/warned in a bad RCU context, for example, the last - * thing we want is to BUG/WARN again in the idtentry code, ad - * infinitum. + * All lies, just get the WARN/BUG out. */ - if (!user_mode(regs) && is_valid_bugaddr(regs->ip)) { - enum bug_trap_type type; + instrumentation_begin(); + if (is_valid_bugaddr(regs->ip) && + report_bug(regs->ip, regs) == BUG_TRAP_TYPE_WARN) { + regs->ip += LEN_UD2; + handled = true; + } + instrumentation_end(); - nmi_enter(); - instrumentation_begin(); - trace_hardirqs_off_finish(); - type = report_bug(regs->ip, regs); - if (regs->flags & X86_EFLAGS_IF) - trace_hardirqs_on_prepare(); - instrumentation_end(); - nmi_exit(); + return handled; +} - if (type == BUG_TRAP_TYPE_WARN) { - /* Skip the ud2. */ - regs->ip += LEN_UD2; - return; - } +DEFINE_IDTENTRY_RAW(exc_invalid_op) +{ + bool rcu_exit; - /* - * 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. - */ - } + /* + * We use UD2 as a short encoding for 'CALL __WARN', as such + * handle it before exception entry to avoid recursive WARN + * in case exception entry is the one triggering WARNs. + */ + if (!user_mode(regs) && handle_bug(regs)) + return; rcu_exit = idtentry_enter_cond_rcu(regs); instrumentation_begin();
![]() |