Re: [PATCH] pci: fix next_ari_fn scan

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 15, 2010 at 10:24:51AM -0800, Jesse Barnes wrote:
> Any update on this one Yinghai or Matthew?  I agree with Matthew's
> cleanup, the no dev check should probably be in scan_slot rather than
> each potential callee.

Here's what I came up with.  I decided to put the check in both to be
robust against the case where the ARI capability is incorrect and tells
us about a device that doesn't exist.

---

Make pci_scan_slot more robust

Yinghai pointed out that the new pci_scan_slot() crashes when called
on an ARI-capable slot that is empty.  Fix this by exiting early from
pci_scan_slot if there is no device in the slot.

Also make next_ari_func() robust against devices not existing in case
the ARI capability is corrupt.  ARI also requires that the devices be
listed in order, so if we find a function listed that is out of order,
stop scanning to prevent loops.

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 11824d7..a06887f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1222,11 +1222,19 @@ EXPORT_SYMBOL(pci_scan_single_device);
 static unsigned next_ari_fn(struct pci_dev *dev, unsigned fn)
 {
 	u16 cap;
-	unsigned pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
+	unsigned pos, next_fn;
+
+	if (!dev)
+		return 0;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
 	if (!pos)
 		return 0;
 	pci_read_config_word(dev, pos + 4, &cap);
-	return cap >> 8;
+	next_fn = cap >> 8;
+	if (next_fn <= fn)
+		return 0;
+	return next_fn;
 }
 
 static unsigned next_trad_fn(struct pci_dev *dev, unsigned fn)
@@ -1271,12 +1279,14 @@ int pci_scan_slot(struct pci_bus *bus, int devfn)
 		return 0; /* Already scanned the entire slot */
 
 	dev = pci_scan_single_device(bus, devfn);
-	if (dev && !dev->is_added)	/* new device? */
+	if (!dev)
+		return 0;
+	if (!dev->is_added)
 		nr++;
 
 	if (pci_ari_enabled(bus))
 		next_fn = next_ari_fn;
-	else if (dev && dev->multifunction)
+	else if (dev->multifunction)
 		next_fn = next_trad_fn;
 
 	for (fn = next_fn(dev, 0); fn > 0; fn = next_fn(dev, fn)) {

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux