On Thu, Jul 02, 2020 at 07:57:54PM +0530, Naresh Kamboju wrote:
> I have reported this warning on linux-next and now it is happening on
> linux mainline tree.
> May I know , are we missing a fix patch on linus 's tree ?
>
> - Naresh
> ---
> While running selftest x86 single_step_syscall_32 on
> i386 kernel linux 5.8.0-rc3 kernel warning noticed.
>
> steps to reproduce:
> --------------------------
> perf test
> and
> cd /opt/kselftests/default-in-kernel/x86
> ./single_step_syscall_32
>
> crash dump,
> [ 1324.774385] kselftest: Running tests in x86
> [ 1324.830187] ------------[ cut here ]------------
> [ 1324.834820] IRQs not disabled as expected
> [ 1324.838838] WARNING: CPU: 2 PID: 5365 at
> /usr/src/kernel/arch/x86/entry/common.c:645
> idtentry_exit_cond_rcu+0x92/0xc0
How's this?
DEFINE_IDTENTRY_DEBUG() is DEFINE_IDTENTRY_RAW on x86_64
DEFINE_IDTENTRY on i386
calling exc_debug_*() from DEFINE_IDTENTRY() does a double layer of
idtentry_{enter,exit}*() functions.
Also, handle_debug() is still a trainwreck, we should never get to
cond_local_irq_enable() when !user.
Completely untested...
---
arch/x86/kernel/traps.c | 49 +++++++++++++++++++++++--------------------------
1 file changed, 23 insertions(+), 26 deletions(-)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index b0195771c932..47d6b46e1564 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -806,8 +806,17 @@ static void handle_debug(struct pt_regs *regs, unsigned
long dr6, bool user)
* If DR6 is zero, no point in trying to handle it. The kernel is
* not using INT1.
*/
- if (!user && !dr6)
- return;
+ if (!user) {
+ /*
+ * Catch SYSENTER with TF set and clear DR_STEP. If this hit a
+ * watchpoint at the same time then that will still be handled.
+ */
+ if ((dr6 & DR_STEP) && is_sysenter_singlestep(regs))
+ dr6 &= ~DR_STEP;
+
+ if (!dr6)
+ return;
+ }
/*
* If dr6 has no reason to give us about the origin of this trap,
@@ -859,25 +868,29 @@ static void handle_debug(struct pt_regs *regs, unsigned
long dr6, bool user)
cond_local_irq_disable(regs);
}
+#ifdef CONFIG_X86_64
static __always_inline void exc_debug_kernel(struct pt_regs *regs,
unsigned long dr6)
{
bool irq_state = idtentry_enter_nmi(regs);
instrumentation_begin();
- /*
- * Catch SYSENTER with TF set and clear DR_STEP. If this hit a
- * watchpoint at the same time then that will still be handled.
- */
- if ((dr6 & DR_STEP) && is_sysenter_singlestep(regs))
- dr6 &= ~DR_STEP;
-
handle_debug(regs, dr6, false);
instrumentation_end();
idtentry_exit_nmi(regs, irq_state);
}
+/* IST stack entry */
+DEFINE_IDTENTRY_DEBUG(exc_debug)
+{
+ unsigned long dr6, dr7;
+
+ debug_enter(&dr6, &dr7);
+ exc_debug_kernel(regs, dr6);
+ debug_exit(dr7);
+}
+
static __always_inline void exc_debug_user(struct pt_regs *regs,
unsigned long dr6)
{
@@ -890,17 +903,6 @@ static __always_inline void exc_debug_user(struct pt_regs
*regs,
idtentry_exit_user(regs);
}
-#ifdef CONFIG_X86_64
-/* IST stack entry */
-DEFINE_IDTENTRY_DEBUG(exc_debug)
-{
- unsigned long dr6, dr7;
-
- debug_enter(&dr6, &dr7);
- exc_debug_kernel(regs, dr6);
- debug_exit(dr7);
-}
-
/* User entry, runs on regular task stack */
DEFINE_IDTENTRY_DEBUG_USER(exc_debug)
{
@@ -917,12 +919,7 @@ DEFINE_IDTENTRY_DEBUG(exc_debug)
unsigned long dr6, dr7;
debug_enter(&dr6, &dr7);
-
- if (user_mode(regs))
- exc_debug_user(regs, dr6);
- else
- exc_debug_kernel(regs, dr6);
-
+ handle_debug(regs, dr6, user_mode(regs));
debug_exit(dr7);
}
#endif
