Skip to content

Commit

Permalink
Revert "PCI: Use to_pci_driver() instead of pci_dev->driver"
Browse files Browse the repository at this point in the history 
This reverts commit 2a4d940.

Robert reported a NULL pointer dereference caused by the PCI core
(local_pci_probe()) calling the i2c_designware_pci driver's
.runtime_resume() method before the .probe() method.  i2c_dw_pci_resume()
depends on initialization done by i2c_dw_pci_probe().

Prior to 2a4d940 ("PCI: Use to_pci_driver() instead of
pci_dev->driver"), pci_pm_runtime_resume() avoided calling the
.runtime_resume() method because pci_dev->driver had not been set yet.

2a4d940 and b5f9c64 ("PCI: Remove struct pci_dev->driver"),
removed pci_dev->driver, replacing it by device->driver, which *has* been
set by this time, so pci_pm_runtime_resume() called the .runtime_resume()
method when it previously had not.

Fixes: 2a4d940 ("PCI: Use to_pci_driver() instead of pci_dev->driver")
Link: https://lore.kernel.org/linux-i2c/CAP145pgdrdiMAT7=-iB1DMgA7t_bMqTcJL4N0=6u8kNY3EU0dw@mail.gmail.com/
Reported-by: Robert Święcki <[email protected]>
Tested-by: Robert Święcki <[email protected]>
Signed-off-by: Bjorn Helgaas <[email protected]>
  • Loading branch information
@bjorn-helgaas
bjorn-helgaas committed Nov 11, 2021
1 parent 68da4e0 commit e0217c5
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 45 deletions.
24 changes: 9 additions & 15 deletions drivers/pci/iov.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,13 @@ static ssize_t sriov_vf_total_msix_show(struct device *dev,
char *buf)
{
struct pci_dev *pdev = to_pci_dev(dev);
struct pci_driver *pdrv;
u32 vf_total_msix = 0;

device_lock(dev);
pdrv = to_pci_driver(dev->driver);
if (!pdrv || !pdrv->sriov_get_vf_total_msix)
if (!pdev->driver || !pdev->driver->sriov_get_vf_total_msix)
goto unlock;

vf_total_msix = pdrv->sriov_get_vf_total_msix(pdev);
vf_total_msix = pdev->driver->sriov_get_vf_total_msix(pdev);
unlock:
device_unlock(dev);
return sysfs_emit(buf, "%u\n", vf_total_msix);
Expand All @@ -185,7 +183,6 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
{
struct pci_dev *vf_dev = to_pci_dev(dev);
struct pci_dev *pdev = pci_physfn(vf_dev);
struct pci_driver *pdrv;
int val, ret = 0;

if (kstrtoint(buf, 0, &val) < 0)
Expand All @@ -195,14 +192,13 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
return -EINVAL;

device_lock(&pdev->dev);
pdrv = to_pci_driver(dev->driver);
if (!pdrv || !pdrv->sriov_set_msix_vec_count) {
if (!pdev->driver || !pdev->driver->sriov_set_msix_vec_count) {
ret = -EOPNOTSUPP;
goto err_pdev;
}

device_lock(&vf_dev->dev);
if (to_pci_driver(vf_dev->dev.driver)) {
if (vf_dev->driver) {
/*
* A driver is already attached to this VF and has configured
* itself based on the current MSI-X vector count. Changing
Expand All @@ -212,7 +208,7 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
goto err_dev;
}

ret = pdrv->sriov_set_msix_vec_count(vf_dev, val);
ret = pdev->driver->sriov_set_msix_vec_count(vf_dev, val);

err_dev:
device_unlock(&vf_dev->dev);
Expand Down Expand Up @@ -379,7 +375,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_driver *pdrv;
int ret = 0;
u16 num_vfs;

Expand All @@ -395,23 +390,22 @@ static ssize_t sriov_numvfs_store(struct device *dev,
goto exit;

/* is PF driver loaded */
pdrv = to_pci_driver(dev->driver);
if (!pdrv) {
if (!pdev->driver) {
pci_info(pdev, "no driver bound to device; cannot configure SR-IOV\n");
ret = -ENOENT;
goto exit;
}

/* is PF driver loaded w/callback */
if (!pdrv->sriov_configure) {
if (!pdev->driver->sriov_configure) {
pci_info(pdev, "driver does not support SR-IOV configuration via sysfs\n");
ret = -ENOENT;
goto exit;
}

if (num_vfs == 0) {
/* disable VFs */
ret = pdrv->sriov_configure(pdev, 0);
ret = pdev->driver->sriov_configure(pdev, 0);
goto exit;
}

Expand All @@ -423,7 +417,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
goto exit;
}

ret = pdrv->sriov_configure(pdev, num_vfs);
ret = pdev->driver->sriov_configure(pdev, num_vfs);
if (ret < 0)
goto exit;

Expand Down
33 changes: 16 additions & 17 deletions drivers/pci/pci-driver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ static int pci_device_probe(struct device *dev)
static void pci_device_remove(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
struct pci_driver *drv = pci_dev->driver;

if (drv->remove) {
pm_runtime_get_sync(dev);
Expand Down Expand Up @@ -493,7 +493,7 @@ static void pci_device_remove(struct device *dev)
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
struct pci_driver *drv = pci_dev->driver;

pm_runtime_resume(dev);

Expand Down Expand Up @@ -589,7 +589,7 @@ static int pci_pm_reenable_device(struct pci_dev *pci_dev)
static int pci_legacy_suspend(struct device *dev, pm_message_t state)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
struct pci_driver *drv = pci_dev->driver;

if (drv && drv->suspend) {
pci_power_t prev = pci_dev->current_state;
Expand Down Expand Up @@ -630,7 +630,7 @@ static int pci_legacy_suspend_late(struct device *dev, pm_message_t state)
static int pci_legacy_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
struct pci_driver *drv = pci_dev->driver;

pci_fixup_device(pci_fixup_resume, pci_dev);

Expand All @@ -649,7 +649,7 @@ static void pci_pm_default_suspend(struct pci_dev *pci_dev)

static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
{
struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
struct pci_driver *drv = pci_dev->driver;
bool ret = drv && (drv->suspend || drv->resume);

/*
Expand Down Expand Up @@ -1242,11 +1242,11 @@ static int pci_pm_runtime_suspend(struct device *dev)
int error;

/*
* If the device has no driver, we leave it in D0, but it may go to
* D3cold when the bridge above it runtime suspends. Save its
* config space in case that happens.
* If pci_dev->driver is not set (unbound), we leave the device in D0,
* but it may go to D3cold when the bridge above it runtime suspends.
* Save its config space in case that happens.
*/
if (!to_pci_driver(dev->driver)) {
if (!pci_dev->driver) {
pci_save_state(pci_dev);
return 0;
}
Expand Down Expand Up @@ -1303,7 +1303,7 @@ static int pci_pm_runtime_resume(struct device *dev)
*/
pci_restore_standard_config(pci_dev);

if (!to_pci_driver(dev->driver))
if (!pci_dev->driver)
return 0;

pci_fixup_device(pci_fixup_resume_early, pci_dev);
Expand All @@ -1322,13 +1322,14 @@ static int pci_pm_runtime_resume(struct device *dev)

static int pci_pm_runtime_idle(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;

/*
* If the device has no driver, it should always remain in D0
* regardless of the runtime PM status
* If pci_dev->driver is not set (unbound), the device should
* always remain in D0 regardless of the runtime PM status
*/
if (!to_pci_driver(dev->driver))
if (!pci_dev->driver)
return 0;

if (!pm)
Expand Down Expand Up @@ -1435,10 +1436,8 @@ static struct pci_driver pci_compat_driver = {
*/
struct pci_driver *pci_dev_driver(const struct pci_dev *dev)
{
struct pci_driver *drv = to_pci_driver(dev->dev.driver);

if (drv)
return drv;
if (dev->driver)
return dev->driver;
else {
int i;
for (i = 0; i <= PCI_ROM_RESOURCE; i++)
Expand Down
17 changes: 8 additions & 9 deletions drivers/pci/pci.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5123,14 +5123,13 @@ EXPORT_SYMBOL_GPL(pci_dev_unlock);

static void pci_dev_save_and_disable(struct pci_dev *dev)
{
struct pci_driver *drv = to_pci_driver(dev->dev.driver);
const struct pci_error_handlers *err_handler =
drv ? drv->err_handler : NULL;
dev->driver ? dev->driver->err_handler : NULL;

/*
* drv->err_handler->reset_prepare() is protected against races
* with ->remove() by the device lock, which must be held by the
* caller.
* dev->driver->err_handler->reset_prepare() is protected against
* races with ->remove() by the device lock, which must be held by
* the caller.
*/
if (err_handler && err_handler->reset_prepare)
err_handler->reset_prepare(dev);
Expand All @@ -5155,15 +5154,15 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)

static void pci_dev_restore(struct pci_dev *dev)
{
struct pci_driver *drv = to_pci_driver(dev->dev.driver);
const struct pci_error_handlers *err_handler =
drv ? drv->err_handler : NULL;
dev->driver ? dev->driver->err_handler : NULL;

pci_restore_state(dev);

/*
* drv->err_handler->reset_done() is protected against races with
* ->remove() by the device lock, which must be held by the caller.
* dev->driver->err_handler->reset_done() is protected against
* races with ->remove() by the device lock, which must be held by
* the caller.
*/
if (err_handler && err_handler->reset_done)
err_handler->reset_done(dev);
Expand Down
8 changes: 4 additions & 4 deletions drivers/pci/pcie/err.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ static int report_error_detected(struct pci_dev *dev,
const struct pci_error_handlers *err_handler;

device_lock(&dev->dev);
pdrv = to_pci_driver(dev->dev.driver);
pdrv = dev->driver;
if (!pci_dev_set_io_state(dev, state) ||
!pdrv ||
!pdrv->err_handler ||
Expand Down Expand Up @@ -98,7 +98,7 @@ static int report_mmio_enabled(struct pci_dev *dev, void *data)
const struct pci_error_handlers *err_handler;

device_lock(&dev->dev);
pdrv = to_pci_driver(dev->dev.driver);
pdrv = dev->driver;
if (!pdrv ||
!pdrv->err_handler ||
!pdrv->err_handler->mmio_enabled)
Expand All @@ -119,7 +119,7 @@ static int report_slot_reset(struct pci_dev *dev, void *data)
const struct pci_error_handlers *err_handler;

device_lock(&dev->dev);
pdrv = to_pci_driver(dev->dev.driver);
pdrv = dev->driver;
if (!pdrv ||
!pdrv->err_handler ||
!pdrv->err_handler->slot_reset)
Expand All @@ -139,7 +139,7 @@ static int report_resume(struct pci_dev *dev, void *data)
const struct pci_error_handlers *err_handler;

device_lock(&dev->dev);
pdrv = to_pci_driver(dev->dev.driver);
pdrv = dev->driver;
if (!pci_dev_set_io_state(dev, pci_channel_io_normal) ||
!pdrv ||
!pdrv->err_handler ||
Expand Down

0 comments on commit e0217c5

Please sign in to comment.