PCI: Protect pci_driver->sriov_configure() usage with device_lock()
authorJakub Kicinski <jakub.kicinski@netronome.com>
Mon, 22 May 2017 22:50:23 +0000 (15:50 -0700)
committerBjorn Helgaas <bhelgaas@google.com>
Wed, 14 Jun 2017 22:41:19 +0000 (17:41 -0500)
Every method in struct device_driver or structures derived from it like
struct pci_driver MUST provide exclusion vs the driver's ->remove() method,
usually by using device_lock().

Protect use of pci_driver->sriov_configure() by holding the device lock
while calling it.

The PCI core sets the pci_dev->driver pointer in local_pci_probe() before
calling ->probe() and only clears it after ->remove().  This means driver's
->sriov_configure() callback will happily race with probe() and remove(),
most likely leading to BUGs, since drivers don't expect this.

Remove the iov lock completely, since we remove the last user.

[bhelgaas: changelog, thanks to Christoph for locking rule]
Link: http://lkml.kernel.org/r/20170522225023.14010-1-jakub.kicinski@netronome.com
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
drivers/pci/iov.c
drivers/pci/pci-sysfs.c
drivers/pci/pci.h

index d9dc7363ac77def7cd81f06646ae76774d92f743..120485d6f352bf017199aee85b78507b3f2c38b7 100644 (file)
@@ -461,8 +461,6 @@ found:
        else
                iov->dev = dev;
 
-       mutex_init(&iov->lock);
-
        dev->sriov = iov;
        dev->is_physfn = 1;
        rc = compute_max_vf_buses(dev);
@@ -491,8 +489,6 @@ static void sriov_release(struct pci_dev *dev)
        if (dev != dev->sriov->dev)
                pci_dev_put(dev->sriov->dev);
 
-       mutex_destroy(&dev->sriov->lock);
-
        kfree(dev->sriov);
        dev->sriov = NULL;
 }
index 31e99613a12e4d9737d082ac0c5f3be19bbdae7d..7755559558df223fa2a24db9cb5a3e63b4836b4f 100644 (file)
@@ -472,7 +472,6 @@ static ssize_t sriov_numvfs_store(struct device *dev,
                                  const char *buf, size_t count)
 {
        struct pci_dev *pdev = to_pci_dev(dev);
-       struct pci_sriov *iov = pdev->sriov;
        int ret;
        u16 num_vfs;
 
@@ -483,7 +482,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
        if (num_vfs > pci_sriov_get_totalvfs(pdev))
                return -ERANGE;
 
-       mutex_lock(&iov->dev->sriov->lock);
+       device_lock(&pdev->dev);
 
        if (num_vfs == pdev->sriov->num_VFs)
                goto exit;
@@ -518,7 +517,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
                         num_vfs, ret);
 
 exit:
-       mutex_unlock(&iov->dev->sriov->lock);
+       device_unlock(&pdev->dev);
 
        if (ret < 0)
                return ret;
index f8113e5b98129026e6c100b7b8f803a74507a442..93f4044b8f4bbb2e91ed8f3685f6c9bad325c68d 100644 (file)
@@ -272,7 +272,6 @@ struct pci_sriov {
        u16 driver_max_VFs;     /* max num VFs driver supports */
        struct pci_dev *dev;    /* lowest numbered PF */
        struct pci_dev *self;   /* this PF */
-       struct mutex lock;      /* lock for setting sriov_numvfs in sysfs */
        resource_size_t barsz[PCI_SRIOV_NUM_BARS];      /* VF BAR size */
        bool drivers_autoprobe; /* auto probing of VFs by driver */
 };