linux-kernel - [PATCH] PCI/sysfs: enforce single creation of sysfs entry for pdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20251013223720.8157-1-ansuelsmth@gmail.com>
Date: Tue, 14 Oct 2025 00:37:16 +0200
From: Christian Marangi <ansuelsmth@...il.com>
To: Bjorn Helgaas <bhelgaas@...gle.com>,
	linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org
Cc: Christian Marangi <ansuelsmth@...il.com>,
	stable@...r.kernel.org,
	Krzysztof Hałasa <khalasa@...p.pl>
Subject: [PATCH] PCI/sysfs: enforce single creation of sysfs entry for pdev

In some specific scenario it's possible that the
pci_create_resource_files() gets called multiple times and the created
entry actually gets wrongly deleted with extreme case of having a NULL
pointer dereference when the PCI is removed.

This mainly happen due to bad timing where the PCI bus is adding PCI
devices and at the same time the sysfs code is adding the entry causing
double execution of the pci_create_resource_files function and kernel
WARNING.

To be more precise there is a race between the late_initcall of
pci-sysfs with pci_sysfs_init and PCI bus.c pci_bus_add_device that also
call pci_create_sysfs_dev_files.

With correct amount of ""luck"" (or better say bad luck)
pci_create_sysfs_dev_files in bus.c might be called with pci_sysfs_init
is executing the loop.

This has been reported multiple times and on multiple system, like imx6
system, ipq806x systems...

To address this, imlement multiple improvement to the implementation:
1. Add a bool to pci_dev to flag when sysfs entry are created
   (sysfs_init)
2. Implement a simple completion to wait pci_sysfs_init execution.
3. Permit additional call of pci_create_sysfs_dev_files only after
   pci_sysfs_init has finished.

With such logic in place, we address al kind of timing problem with
minimal change to any driver.

A notice worth to mention is that the remove function are not affected
by this as the pci_remove_resource_files have enough check in place to
always work and it's always called by pci_stop_dev.

Cc: stable@...r.kernel.org
Reported-by: Krzysztof Hałasa <khalasa@...p.pl>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=215515
Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
---
 drivers/pci/pci-sysfs.c | 34 +++++++++++++++++++++++++++++-----
 include/linux/pci.h     |  1 +
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 71a36f57ef57..cab3aa27f947 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -14,6 +14,7 @@
  */

 #include <linux/bitfield.h>
+#include <linux/completion.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/pci.h>
@@ -37,6 +38,7 @@
 #endif

 static int sysfs_initialized;	/* = 0 */
+static DECLARE_COMPLETION(sysfs_init_completion);

 /* show configuration fields */
 #define pci_config_attr(field, format_string)				\
@@ -1652,12 +1654,32 @@ static const struct attribute_group pci_dev_resource_resize_group = {
 	.is_visible = resource_resize_is_visible,
 };

+static int __pci_create_sysfs_dev_files(struct pci_dev *pdev)
+{
+	int ret;
+
+	ret = pci_create_resource_files(pdev);
+	if (ret)
+		return ret;
+
+	/* on success set sysfs correctly created */
+	pdev->sysfs_init = true;
+	return 0;
+}
+
 int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
 {
 	if (!sysfs_initialized)
 		return -EACCES;

-	return pci_create_resource_files(pdev);
+	/* sysfs entry already created */
+	if (pdev->sysfs_init)
+		return 0;
+
+	/* wait for pci_sysfs_init */
+	wait_for_completion(&sysfs_init_completion);
+
+	return __pci_create_sysfs_dev_files(pdev);
 }

 /**
@@ -1678,21 +1700,23 @@ static int __init pci_sysfs_init(void)
 {
 	struct pci_dev *pdev = NULL;
 	struct pci_bus *pbus = NULL;
-	int retval;
+	int retval = 0;

 	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;
+			goto exit;
 		}
 	}

 	while ((pbus = pci_find_next_bus(pbus)))
 		pci_create_legacy_files(pbus);

-	return 0;
+exit:
+	complete_all(&sysfs_init_completion);
+	return retval;
 }
 late_initcall(pci_sysfs_init);

diff --git a/include/linux/pci.h b/include/linux/pci.h
index f3f6d6dee3ae..f417a0528f01 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -480,6 +480,7 @@ struct pci_dev {
 	unsigned int	non_mappable_bars:1;	/* BARs can't be mapped to user-space  */
 	pci_dev_flags_t dev_flags;
 	atomic_t	enable_cnt;	/* pci_enable_device has been called */
+	bool		sysfs_init;	/* sysfs entry has been created */

 	spinlock_t	pcie_cap_lock;		/* Protects RMW ops in capability accessors */
 	u32		saved_config_space[16]; /* Config space saved at suspend time */
-- 
2.51.0