Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Alexander Stein <alexander.stein@ew.tq-group.com>
To:     Bjorn Helgaas <bhelgaas@google.com>
Cc:     Alexander Stein <alexander.stein@ew.tq-group.com>,
        linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
        Korneliusz Osmenda <korneliuszo@gmail.com>,
        Oliver Neukum <oneukum@suse.com>
Subject: [PATCH 3/3] PCI/sysfs: Fix sysfs init race condition
Date:   Thu, 27 Apr 2023 16:29:01 +0200
Message-Id: <20230427142901.3570536-4-alexander.stein@ew.tq-group.com>
In-Reply-To: <20230427142901.3570536-1-alexander.stein@ew.tq-group.com>
References: <20230427142901.3570536-1-alexander.stein@ew.tq-group.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@ew.tq-group.com>
---
 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