linux-kernel - Re: BUG: sleeping function called from invalid context at kernel/mutex.c

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: <alpine.LFD.2.00.1104021653010.22266@localhost6.localdomain6>
Date:	Sat, 2 Apr 2011 21:12:46 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Xiaotian Feng <dfeng@...hat.com>
cc:	LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
	Venkatesh Pallipadi <venki@...gle.com>,
	Suresh Siddha <suresh.b.siddha@...el.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: BUG: sleeping function called from invalid context at
 kernel/mutex.c

On Sat, 2 Apr 2011, Xiaotian Feng wrote:
>     I've got following warnings when I'm trying to resume from suspend, so
> sparse_lock is not safe enough as a mutex for the suspend resume case, can
> we simply convert sparse_lock back to spinlock?

What the heck? We do not just convert a mutex to a spinlock because a
warning triggers. And it was a mutex already in 2.6.38 w/o any such
problems.

And it is safe enough for the resume case as well, it simply cannot go
to sleep because nothing else can hold that mutex in the early resume
code. We take the mutex during early boot as well, but there the might
sleep checks are disabled. And we should do the very same thing in the
early resume code as well. Rafael, PeterZ ???

But that's a different issue and not the real problem.

So now instead of thinking about spinlock it would have been pretty
simple to figure out that the warning is triggered due to commit
d72274e5.

But of course we dont want to revert d72274e5 either, though it's easy
to figure out from the call chain that the fundamental stupidity is in
the hpet resume code.

Why the hell does it call arch_setup_hpet_msi()? That interrupt was
completely setup _BEFORE_ we went into suspend. And I don't see any
code which tears down the interrupt on suspend. So we run through a
full setup for nothing. And when we have interrupt remapping enabled
it even leaks an irte, because it allocates a new one. Utter crap.
init_one_hpet_msi_clockevent() has the same stupid call.

So no, the mutex stays a mutex and we fix crap where the crap is.

The following completely untested patch should solve that. Actually we
should be more clever and do the affinity magic in the new function as
well, so we can get rid of this weird disable/set_affinity/enable
hack, but that's a different story.

Thanks,

	tglx
---
 arch/x86/include/asm/hpet.h    |    2 ++
 arch/x86/kernel/apic/io_apic.c |   11 +++++++++++
 arch/x86/kernel/hpet.c         |    5 +++--
 3 files changed, 16 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/include/asm/hpet.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/hpet.h
+++ linux-2.6/arch/x86/include/asm/hpet.h
@@ -83,11 +83,13 @@ extern void hpet_msi_read(struct hpet_de

 #ifdef CONFIG_PCI_MSI
 extern int arch_setup_hpet_msi(unsigned int irq, unsigned int id);
+extern int arch_start_hpet_msi(unsigned int irq, unsigned int id);
 #else
 static inline int arch_setup_hpet_msi(unsigned int irq, unsigned int id)
 {
 	return -EINVAL;
 }
+static inline int arch_start_hpet_msi(unsigned int irq, unsigned int id) { }
 #endif

 #ifdef CONFIG_HPET_EMULATE_RTC
Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6/arch/x86/kernel/apic/io_apic.c
@@ -3477,6 +3477,17 @@ int arch_setup_hpet_msi(unsigned int irq
 	irq_set_chip_and_handler_name(irq, chip, handle_edge_irq, "edge");
 	return 0;
 }
+
+int arch_start_hpet_msi(unsigned int irq, unsigned int id)
+{
+	struct msi_msg msg;
+	int ret = msi_compose_msg(NULL, irq, &msg, id);
+
+	if (!ret)
+		hpet_msi_write(irq_get_handler_data(irq), &msg);
+	return ret;
+}
+
 #endif

 #endif /* CONFIG_PCI_MSI */
Index: linux-2.6/arch/x86/kernel/hpet.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/hpet.c
+++ linux-2.6/arch/x86/kernel/hpet.c
@@ -370,7 +370,8 @@ static void hpet_set_mode(enum clock_eve
 			hpet_enable_legacy_int();
 		} else {
 			struct hpet_dev *hdev = EVT_TO_HPET_DEV(evt);
-			hpet_setup_msi_irq(hdev->irq);
+
+			arch_start_hpet_msi(hdev->irq, hpet_blockid);
 			disable_irq(hdev->irq);
 			irq_set_affinity(hdev->irq, cpumask_of(hdev->cpu));
 			enable_irq(hdev->irq);
@@ -555,7 +556,7 @@ static void init_one_hpet_msi_clockevent
 	if (!(hdev->flags & HPET_DEV_VALID))
 		return;

-	if (hpet_setup_msi_irq(hdev->irq))
+	if (arch_start_hpet_msi(hdev->irq, hpet_blockid))
 		return;

 	hdev->cpu = cpu;

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