[PATCH 3/3] PCI/sysfs: Fix sysfs init race condition

From: Alexander Stein
Date: Thu Apr 27 2023 - 10:29:24 EST


sysfs attribute files for PCIe devices (pci_create_sysfs_dev_files) can be
created by two paths:
1. pci_sysfs_init()
2. pci_bus_add_device() (drivers/pci/bus.c)

There is a race during startup where an asynchronous PCIe host probe races
against the pci_sysfs_init() late_initcall. In this case the PCIe devices
are already added to the bus, for_each_pci_dev() will see them, but
pci_bus_add_device() has not yet finished, so both code paths try to add
the sysfs attributes.

Fix this by waiting on a workqueue until sysfs has been initialized.
pci_sysfs_init() needs the internal function without the check that
sysfs_initialized has been set to 1.
__pci_create_sysfs_dev_files still needs to remove resource files,
which might have been created during pci_sysfs_init initcall.

Signed-off-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
---
drivers/pci/pci-sysfs.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 7d4733773633..3067d55f981c 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -29,9 +29,11 @@
#include <linux/stat.h>
#include <linux/topology.h>
#include <linux/vgaarb.h>
+#include <linux/wait.h>
#include "pci.h"

static int sysfs_initialized; /* = 0 */
+static DECLARE_WAIT_QUEUE_HEAD(sysfs_wq);

/* show configuration fields */
#define pci_config_attr(field, format_string) \
@@ -997,8 +999,7 @@ static void __pci_create_legacy_files(struct pci_bus *b)
*/
void pci_create_legacy_files(struct pci_bus *b)
{
- if (!sysfs_initialized)
- return;
+ wait_event(sysfs_wq, sysfs_initialized);

__pci_create_legacy_files(b);
}
@@ -1501,13 +1502,18 @@ static const struct attribute_group pci_dev_resource_resize_group = {

int __must_check __pci_create_sysfs_dev_files(struct pci_dev *pdev)
{
+ /*
+ * sysfs attributes might already be created by pci_sysfs_init(),
+ * delete them here just in case
+ */
+ pci_remove_resource_files(pdev);
return pci_create_resource_files(pdev);
}

int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
{
- if (!sysfs_initialized)
- return -EACCES;
+ /* Wait until sysfs has been initialized */
+ wait_event(sysfs_wq, sysfs_initialized);

return __pci_create_sysfs_dev_files(pdev);
}
@@ -1520,8 +1526,8 @@ int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
*/
void pci_remove_sysfs_dev_files(struct pci_dev *pdev)
{
- if (!sysfs_initialized)
- return;
+ /* Wait until sysfs has been initialized */
+ wait_event(sysfs_wq, sysfs_initialized);

pci_remove_resource_files(pdev);
}
@@ -1532,9 +1538,8 @@ static int __init pci_sysfs_init(void)
struct pci_bus *pbus = NULL;
int retval;

- sysfs_initialized = 1;
for_each_pci_dev(pdev) {
- retval = pci_create_sysfs_dev_files(pdev);
+ retval = __pci_create_sysfs_dev_files(pdev);
if (retval) {
pci_dev_put(pdev);
return retval;
@@ -1542,7 +1547,9 @@ static int __init pci_sysfs_init(void)
}

while ((pbus = pci_find_next_bus(pbus)))
- pci_create_legacy_files(pbus);
+ __pci_create_legacy_files(pbus);
+ sysfs_initialized = 1;
+ wake_up_all(&sysfs_wq);

return 0;
}
--
2.34.1