PCI devices are created via pci_scan_slot() and similar, and are promptly configured for runtime PM (pci_pm_init()). They are initially prevented from suspending by way of pm_runtime_forbid(); however, it's expected that user space may override this via sysfs [1].
Now, sometime after initial scan, a PCI device receives its BAR configuration (pci_assign_unassigned_bus_resources(), etc.).
If a PCI device is allowed to suspend between pci_scan_slot() and pci_assign_unassigned_bus_resources(), then pci-driver.c will save/restore incorrect BAR configuration for the device, and the device may cease to function.
This behavior races with user space, since user space may enable runtime PM [1] as soon as it sees the device, which may be before BAR configuration.
Prevent suspending in this intermediate state by holding a runtime PM reference until the device is fully initialized and ready for probe().
[1] echo auto > /sys/bus/pci/devices/.../power/control
Cc: stable@vger.kernel.org Signed-off-by: Brian Norris briannorris@chromium.org ---
drivers/pci/bus.c | 7 +++++++ drivers/pci/pci.c | 6 ++++++ 2 files changed, 13 insertions(+)
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index f26aec6ff588..227a8898acac 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -14,6 +14,7 @@ #include <linux/of.h> #include <linux/of_platform.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/proc_fs.h> #include <linux/slab.h>
@@ -375,6 +376,12 @@ void pci_bus_add_device(struct pci_dev *dev) put_device(&pdev->dev); }
+ /* + * Now that resources are assigned, drop the reference we grabbed in + * pci_pm_init(). + */ + pm_runtime_put_noidle(&dev->dev); + if (!dn || of_device_is_available(dn)) pci_dev_allow_binding(dev);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b14dd064006c..06a901214f2c 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3226,6 +3226,12 @@ void pci_pm_init(struct pci_dev *dev) pci_pm_power_up_and_verify_state(dev); pm_runtime_forbid(&dev->dev); pm_runtime_set_active(&dev->dev); + /* + * We cannot allow a device to suspend before its resources are + * configured. Otherwise, we may allow saving/restoring unexpected BAR + * configuration. + */ + pm_runtime_get_noresume(&dev->dev); pm_runtime_enable(&dev->dev); }
On Fri, Oct 17, 2025 at 1:28 AM Brian Norris briannorris@chromium.org wrote:
PCI devices are created via pci_scan_slot() and similar, and are promptly configured for runtime PM (pci_pm_init()). They are initially prevented from suspending by way of pm_runtime_forbid(); however, it's expected that user space may override this via sysfs [1].
Now, sometime after initial scan, a PCI device receives its BAR configuration (pci_assign_unassigned_bus_resources(), etc.).
If a PCI device is allowed to suspend between pci_scan_slot() and pci_assign_unassigned_bus_resources(), then pci-driver.c will save/restore incorrect BAR configuration for the device, and the device may cease to function.
This behavior races with user space, since user space may enable runtime PM [1] as soon as it sees the device, which may be before BAR configuration.
Prevent suspending in this intermediate state by holding a runtime PM reference until the device is fully initialized and ready for probe().
[1] echo auto > /sys/bus/pci/devices/.../power/control
Cc: stable@vger.kernel.org Signed-off-by: Brian Norris briannorris@chromium.org
drivers/pci/bus.c | 7 +++++++ drivers/pci/pci.c | 6 ++++++ 2 files changed, 13 insertions(+)
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index f26aec6ff588..227a8898acac 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -14,6 +14,7 @@ #include <linux/of.h> #include <linux/of_platform.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/proc_fs.h> #include <linux/slab.h>
@@ -375,6 +376,12 @@ void pci_bus_add_device(struct pci_dev *dev) put_device(&pdev->dev); }
/** Now that resources are assigned, drop the reference we grabbed in* pci_pm_init().*/pm_runtime_put_noidle(&dev->dev);if (!dn || of_device_is_available(dn)) pci_dev_allow_binding(dev);diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b14dd064006c..06a901214f2c 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3226,6 +3226,12 @@ void pci_pm_init(struct pci_dev *dev) pci_pm_power_up_and_verify_state(dev); pm_runtime_forbid(&dev->dev); pm_runtime_set_active(&dev->dev);
/** We cannot allow a device to suspend before its resources are* configured. Otherwise, we may allow saving/restoring unexpected BAR* configuration.*/pm_runtime_get_noresume(&dev->dev); pm_runtime_enable(&dev->dev);
So runtime PM should not be enabled here, should it?
}
--
Hi Rafael,
On Fri, Oct 17, 2025 at 11:45:14AM +0200, Rafael J. Wysocki wrote:
On Fri, Oct 17, 2025 at 1:28 AM Brian Norris briannorris@chromium.org wrote:
PCI devices are created via pci_scan_slot() and similar, and are promptly configured for runtime PM (pci_pm_init()). They are initially prevented from suspending by way of pm_runtime_forbid(); however, it's expected that user space may override this via sysfs [1].
Now, sometime after initial scan, a PCI device receives its BAR configuration (pci_assign_unassigned_bus_resources(), etc.).
If a PCI device is allowed to suspend between pci_scan_slot() and pci_assign_unassigned_bus_resources(), then pci-driver.c will save/restore incorrect BAR configuration for the device, and the device may cease to function.
This behavior races with user space, since user space may enable runtime PM [1] as soon as it sees the device, which may be before BAR configuration.
Prevent suspending in this intermediate state by holding a runtime PM reference until the device is fully initialized and ready for probe().
[1] echo auto > /sys/bus/pci/devices/.../power/control
Cc: stable@vger.kernel.org Signed-off-by: Brian Norris briannorris@chromium.org
drivers/pci/bus.c | 7 +++++++ drivers/pci/pci.c | 6 ++++++ 2 files changed, 13 insertions(+)
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index f26aec6ff588..227a8898acac 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -14,6 +14,7 @@ #include <linux/of.h> #include <linux/of_platform.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/proc_fs.h> #include <linux/slab.h>
@@ -375,6 +376,12 @@ void pci_bus_add_device(struct pci_dev *dev) put_device(&pdev->dev); }
/** Now that resources are assigned, drop the reference we grabbed in* pci_pm_init().*/pm_runtime_put_noidle(&dev->dev);if (!dn || of_device_is_available(dn)) pci_dev_allow_binding(dev);diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b14dd064006c..06a901214f2c 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3226,6 +3226,12 @@ void pci_pm_init(struct pci_dev *dev) pci_pm_power_up_and_verify_state(dev); pm_runtime_forbid(&dev->dev); pm_runtime_set_active(&dev->dev);
/** We cannot allow a device to suspend before its resources are* configured. Otherwise, we may allow saving/restoring unexpected BAR* configuration.*/pm_runtime_get_noresume(&dev->dev); pm_runtime_enable(&dev->dev);So runtime PM should not be enabled here, should it?
Hmm, I suppose not. Does that imply it would be a better solution to simply defer pm_runtime_enable() to pci_bus_add_device() or some similar point? I'll give that a shot, since that seems like a simpler and cleaner solution.
Thanks, Brian
}
--
linux-stable-mirror@lists.linaro.org
-
Brian Norris -
Rafael J. Wysocki