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

Commit Message

Alexander Stein April 27, 2023, 2:29 p.m. UTC

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 <[e-mail]>
---
 drivers/pci/pci-sysfs.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

Patch

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;
 }