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>] [day] [month] [year] [list]
Date:	Wed, 27 Sep 2006 14:31:22 -0600
From:	"Eric W. Biederman" <ebiederm@...ssion.com>
To:	Andrew Morton <akpm@...l.org>
Cc:	<linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
	Tony Luck <tony.luck@...el.com>, Andi Kleen <ak@...e.de>,
	"Eric W. Biederman" <ebiederm@...ssion.com>
Subject: [PATCH 1/5] msi: Simplify msi sanity checks by adding with generic irq code.

Currently msi.c is doing sanity checks that make certain before
an irq is destroyed it has no more users.

By adding irq_has_action I can perform the test is a generic way, instead
of relying on a msi specific data structure.

By performing the core check in dynamic_irq_cleanup I ensure every user
of dynamic irqs has a test present and we don't free resources that are
in use.

In msi.c this allows me to kill the attrib.state member of msi_desc and
all of the assciated code to maintain it.

To keep from freeing data structures when irq cleanup code is called to
soon changing dyanamic_irq_cleanup is insufficient because there are
msi specific data structures that are also not safe to free.

Signed-off-by: Eric W. Biederman <ebiederm@...ssion.com>
---
 drivers/pci/msi.c   |   43 ++++++++-----------------------------------
 drivers/pci/msi.h   |    2 +-
 include/linux/irq.h |    7 +++++++
 kernel/irq/chip.c   |    7 +++++++
 4 files changed, 23 insertions(+), 36 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index da2c6c2..e3ba396 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -188,18 +188,6 @@ static void unmask_MSI_irq(unsigned int 
 
 static unsigned int startup_msi_irq_wo_maskbit(unsigned int irq)
 {
-	struct msi_desc *entry;
-	unsigned long flags;
-
-	spin_lock_irqsave(&msi_lock, flags);
-	entry = msi_desc[irq];
-	if (!entry || !entry->dev) {
-		spin_unlock_irqrestore(&msi_lock, flags);
-		return 0;
-	}
-	entry->msi_attrib.state = 1;	/* Mark it active */
-	spin_unlock_irqrestore(&msi_lock, flags);
-
 	return 0;	/* never anything pending */
 }
 
@@ -212,14 +200,6 @@ static unsigned int startup_msi_irq_w_ma
 
 static void shutdown_msi_irq(unsigned int irq)
 {
-	struct msi_desc *entry;
-	unsigned long flags;
-
-	spin_lock_irqsave(&msi_lock, flags);
-	entry = msi_desc[irq];
-	if (entry && entry->dev)
-		entry->msi_attrib.state = 0;	/* Mark it not active */
-	spin_unlock_irqrestore(&msi_lock, flags);
 }
 
 static void end_msi_irq_wo_maskbit(unsigned int irq)
@@ -671,7 +651,6 @@ static int msi_capability_init(struct pc
 	entry->link.head = irq;
 	entry->link.tail = irq;
 	entry->msi_attrib.type = PCI_CAP_ID_MSI;
-	entry->msi_attrib.state = 0;			/* Mark it not active */
 	entry->msi_attrib.is_64 = is_64bit_address(control);
 	entry->msi_attrib.entry_nr = 0;
 	entry->msi_attrib.maskbit = is_mask_bit_support(control);
@@ -744,7 +723,6 @@ static int msix_capability_init(struct p
  		j = entries[i].entry;
  		entries[i].vector = irq;
 		entry->msi_attrib.type = PCI_CAP_ID_MSIX;
- 		entry->msi_attrib.state = 0;		/* Mark it not active */
 		entry->msi_attrib.is_64 = 1;
 		entry->msi_attrib.entry_nr = j;
 		entry->msi_attrib.maskbit = 1;
@@ -897,12 +875,12 @@ void pci_disable_msi(struct pci_dev* dev
 		spin_unlock_irqrestore(&msi_lock, flags);
 		return;
 	}
-	if (entry->msi_attrib.state) {
+	if (irq_has_action(dev->irq)) {
 		spin_unlock_irqrestore(&msi_lock, flags);
 		printk(KERN_WARNING "PCI: %s: pci_disable_msi() called without "
 		       "free_irq() on MSI irq %d\n",
 		       pci_name(dev), dev->irq);
-		BUG_ON(entry->msi_attrib.state > 0);
+		BUG_ON(irq_has_action(dev->irq));
 	} else {
 		default_irq = entry->msi_attrib.default_irq;
 		spin_unlock_irqrestore(&msi_lock, flags);
@@ -1035,17 +1013,16 @@ void pci_disable_msix(struct pci_dev* de
 
 	temp = dev->irq;
 	if (!msi_lookup_irq(dev, PCI_CAP_ID_MSIX)) {
-		int state, irq, head, tail = 0, warning = 0;
+		int irq, head, tail = 0, warning = 0;
 		unsigned long flags;
 
 		irq = head = dev->irq;
 		dev->irq = temp;			/* Restore pin IRQ */
 		while (head != tail) {
 			spin_lock_irqsave(&msi_lock, flags);
-			state = msi_desc[irq]->msi_attrib.state;
 			tail = msi_desc[irq]->link.tail;
 			spin_unlock_irqrestore(&msi_lock, flags);
-			if (state)
+			if (irq_has_action(irq))
 				warning = 1;
 			else if (irq != head)	/* Release MSI-X irq */
 				msi_free_irq(dev, irq);
@@ -1072,7 +1049,7 @@ void pci_disable_msix(struct pci_dev* de
  **/
 void msi_remove_pci_irq_vectors(struct pci_dev* dev)
 {
-	int state, pos, temp;
+	int pos, temp;
 	unsigned long flags;
 
 	if (!pci_msi_enable || !dev)
@@ -1081,14 +1058,11 @@ void msi_remove_pci_irq_vectors(struct p
 	temp = dev->irq;		/* Save IOAPIC IRQ */
 	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
 	if (pos > 0 && !msi_lookup_irq(dev, PCI_CAP_ID_MSI)) {
-		spin_lock_irqsave(&msi_lock, flags);
-		state = msi_desc[dev->irq]->msi_attrib.state;
-		spin_unlock_irqrestore(&msi_lock, flags);
-		if (state) {
+		if (irq_has_action(dev->irq)) {
 			printk(KERN_WARNING "PCI: %s: msi_remove_pci_irq_vectors() "
 			       "called without free_irq() on MSI irq %d\n",
 			       pci_name(dev), dev->irq);
-			BUG_ON(state > 0);
+			BUG_ON(irq_has_action(dev->irq));
 		} else /* Release MSI irq assigned to this device */
 			msi_free_irq(dev, dev->irq);
 		dev->irq = temp;		/* Restore IOAPIC IRQ */
@@ -1101,11 +1075,10 @@ void msi_remove_pci_irq_vectors(struct p
 		irq = head = dev->irq;
 		while (head != tail) {
 			spin_lock_irqsave(&msi_lock, flags);
-			state = msi_desc[irq]->msi_attrib.state;
 			tail = msi_desc[irq]->link.tail;
 			base = msi_desc[irq]->mask_base;
 			spin_unlock_irqrestore(&msi_lock, flags);
-			if (state)
+			if (irq_has_action(irq))
 				warning = 1;
 			else if (irq != head) /* Release MSI-X irq */
 				msi_free_irq(dev, irq);
diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h
index 435d05a..77823bf 100644
--- a/drivers/pci/msi.h
+++ b/drivers/pci/msi.h
@@ -53,7 +53,7 @@ struct msi_desc {
 	struct {
 		__u8	type	: 5; 	/* {0: unused, 5h:MSI, 11h:MSI-X} */
 		__u8	maskbit	: 1; 	/* mask-pending bit supported ?   */
-		__u8	state	: 1; 	/* {0: free, 1: busy}		  */
+		__u8	unused	: 1;
 		__u8	is_64	: 1;	/* Address size: 0=32bit 1=64bit  */
 		__u8	pos;	 	/* Location of the msi capability */
 		__u16	entry_nr;    	/* specific enabled entry 	  */
diff --git a/include/linux/irq.h b/include/linux/irq.h
index ee8a5ea..86b65ed 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -373,6 +373,13 @@ set_irq_chained_handler(unsigned int irq
 extern int create_irq(void);
 extern void destroy_irq(unsigned int irq);
 
+/* Test to see if a driver has successfully requested an irq */
+static inline int irq_has_action(unsigned int irq)
+{
+	struct irq_desc *desc = irq_desc + irq;
+	return desc->action != NULL;
+}
+
 /* Dynamic irq helper functions */
 extern void dynamic_irq_init(unsigned int irq);
 extern void dynamic_irq_cleanup(unsigned int irq);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index e128b7d..2253898 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -151,6 +151,13 @@ void dynamic_irq_cleanup(unsigned int ir
 
 	desc = irq_desc + irq;
 	spin_lock_irqsave(&desc->lock, flags);
+	if (desc->action) {
+		spin_unlock_irqrestore(&desc->lock, flags);
+		printk(KERN_ERR "Destroying IRQ%d without calling free_irq\n",
+			irq);
+		WARN_ON(1);
+		return;
+	}
 	desc->handle_irq = handle_bad_irq;
 	desc->chip = &no_irq_chip;
 	spin_unlock_irqrestore(&desc->lock, flags);
-- 
1.4.2.rc3.g7e18e-dirty

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ