linux-kernel - Re: mce.c related WARNING: at kernel/timer.c:983 del_timer

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110308185035.GF2499@one.firstfloor.org>
Date:	Tue, 8 Mar 2011 19:50:35 +0100
From:	Andi Kleen <andi@...stfloor.org>
To:	Yong Zhang <yong.zhang0@...il.com>
Cc:	Venkatesh Pallipadi <venki@...gle.com>,
	Andi Kleen <andi@...stfloor.org>,
	Yong Zhang <yong.zhang@...driver.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>
Subject: Re: mce.c related WARNING: at kernel/timer.c:983 del_timer_sync

> >
> > But, the actual reason is likely some MCE parameter change at boot causing
> > mce_restart() which in turn calls on_each_cpu mce_cpu_restart() which calls
> > del_timer_sync().
> 
> Seems we found a real bug.

I don't think it's a real bug actually because the timer cannot run at
the same time in this state. It's an interrupt which runs with irq disabled 
Really the only case where it could lead to deadlock is when the timer
runs with irqs on and the other interrupt with the del_timer_sync
interrupts it. So most likely your new WARN_ON() is catching
lots of innocent code.

That said I don't think we need the del_timer_sync in mce.c either
for the same reason.  The timer is always on the
same CPU, so it cannot run in parallel.

Remove del_timer_sync()s in mce.c

All the del_timers happen on the same CPUs as the actual timers, so
the timer handlers cannot run at the same time. Replace them
with plain del_timer()s.

Signed-off-by: Andi Kleen <ak@...ux.intel.com>

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index d916183..ba7058a 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1774,7 +1774,7 @@ static int mce_resume(struct sys_device *dev)
 
 static void mce_cpu_restart(void *data)
 {
-	del_timer_sync(&__get_cpu_var(mce_timer));
+	del_timer(&__get_cpu_var(mce_timer));
 	if (!mce_available(__this_cpu_ptr(&cpu_info)))
 		return;
 	__mcheck_cpu_init_generic();
@@ -1793,7 +1793,7 @@ static void mce_disable_ce(void *all)
 	if (!mce_available(__this_cpu_ptr(&cpu_info)))
 		return;
 	if (all)
-		del_timer_sync(&__get_cpu_var(mce_timer));
+		del_timer(&__get_cpu_var(mce_timer));
 	cmci_clear();
 }
 
@@ -2075,7 +2075,7 @@ mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 		break;
 	case CPU_DOWN_PREPARE:
 	case CPU_DOWN_PREPARE_FROZEN:
-		del_timer_sync(t);
+		del_timer(t);
 		smp_call_function_single(cpu, mce_disable_cpu, &action, 1);
 		break;
 	case CPU_DOWN_FAILED:

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