|
| 1 | +From d33941523a8379e30070374b133b28a2077dcef8 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Christian Marangi <ansuelsmth@gmail.com> |
| 3 | +Date: Mon, 13 Oct 2025 20:45:25 +0200 |
| 4 | +Subject: [PATCH] PCI/sysfs: enforce single creation of sysfs entry for pdev |
| 5 | +MIME-Version: 1.0 |
| 6 | +Content-Type: text/plain; charset=UTF-8 |
| 7 | +Content-Transfer-Encoding: 8bit |
| 8 | + |
| 9 | +In some specific scenario it's possible that the |
| 10 | +pci_create_resource_files() gets called multiple times and the created |
| 11 | +entry actually gets wrongly deleted with extreme case of having a NULL |
| 12 | +pointer dereference when the PCI is removed. |
| 13 | + |
| 14 | +This mainly happen due to bad timing where the PCI bus is adding PCI |
| 15 | +devices and at the same time the sysfs code is adding the entry causing |
| 16 | +double execution of the pci_create_resource_files function and kernel |
| 17 | +WARNING. |
| 18 | + |
| 19 | +To be more precise there is a race between the late_initcall of |
| 20 | +pci-sysfs with pci_sysfs_init and PCI bus.c pci_bus_add_device that also |
| 21 | +call pci_create_sysfs_dev_files. |
| 22 | + |
| 23 | +With correct amount of ""luck"" (or better say bad luck) |
| 24 | +pci_create_sysfs_dev_files in bus.c might be called with pci_sysfs_init |
| 25 | +is executing the loop. |
| 26 | + |
| 27 | +This has been reported multiple times and on multiple system, like imx6 |
| 28 | +system, ipq806x systems... |
| 29 | + |
| 30 | +To address this, imlement multiple improvement to the implementation: |
| 31 | +1. Add a bool to pci_dev to flag when sysfs entry are created |
| 32 | + (sysfs_init) |
| 33 | +2. Implement a simple completion to wait pci_sysfs_init execution. |
| 34 | +3. Permit additional call of pci_create_sysfs_dev_files only after |
| 35 | + pci_sysfs_init has finished. |
| 36 | + |
| 37 | +With such logic in place, we address al kind of timing problem with |
| 38 | +minimal change to any driver. |
| 39 | + |
| 40 | +A notice worth to mention is that the remove function are not affected |
| 41 | +by this as the pci_remove_resource_files have enough check in place to |
| 42 | +always work and it's always called by pci_stop_dev. |
| 43 | + |
| 44 | +Cc: stable@vger.kernel.org |
| 45 | +Reported-by: Krzysztof Hałasa <khalasa@piap.pl> |
| 46 | +Closes: https://bugzilla.kernel.org/show_bug.cgi?id=215515 |
| 47 | +Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> |
| 48 | +--- |
| 49 | + drivers/pci/pci-sysfs.c | 34 +++++++++++++++++++++++++++++----- |
| 50 | + include/linux/pci.h | 1 + |
| 51 | + 2 files changed, 30 insertions(+), 5 deletions(-) |
| 52 | + |
| 53 | +--- a/drivers/pci/pci-sysfs.c |
| 54 | ++++ b/drivers/pci/pci-sysfs.c |
| 55 | +@@ -13,6 +13,7 @@ |
| 56 | + */ |
| 57 | + |
| 58 | + #include <linux/bitfield.h> |
| 59 | ++#include <linux/completion.h> |
| 60 | + #include <linux/kernel.h> |
| 61 | + #include <linux/sched.h> |
| 62 | + #include <linux/pci.h> |
| 63 | +@@ -36,6 +37,7 @@ |
| 64 | + #endif |
| 65 | + |
| 66 | + static int sysfs_initialized; /* = 0 */ |
| 67 | ++static DECLARE_COMPLETION(sysfs_init_completion); |
| 68 | + |
| 69 | + /* show configuration fields */ |
| 70 | + #define pci_config_attr(field, format_string) \ |
| 71 | +@@ -1533,12 +1535,32 @@ static const struct attribute_group pci_ |
| 72 | + .is_visible = resource_resize_is_visible, |
| 73 | + }; |
| 74 | + |
| 75 | ++static int __pci_create_sysfs_dev_files(struct pci_dev *pdev) |
| 76 | ++{ |
| 77 | ++ int ret; |
| 78 | ++ |
| 79 | ++ ret = pci_create_resource_files(pdev); |
| 80 | ++ if (ret) |
| 81 | ++ return ret; |
| 82 | ++ |
| 83 | ++ /* on success set sysfs correctly created */ |
| 84 | ++ pdev->sysfs_init = true; |
| 85 | ++ return 0; |
| 86 | ++} |
| 87 | ++ |
| 88 | + int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev) |
| 89 | + { |
| 90 | + if (!sysfs_initialized) |
| 91 | + return -EACCES; |
| 92 | + |
| 93 | +- return pci_create_resource_files(pdev); |
| 94 | ++ /* sysfs entry already created */ |
| 95 | ++ if (pdev->sysfs_init) |
| 96 | ++ return 0; |
| 97 | ++ |
| 98 | ++ /* wait for pci_sysfs_init */ |
| 99 | ++ wait_for_completion(&sysfs_init_completion); |
| 100 | ++ |
| 101 | ++ return __pci_create_sysfs_dev_files(pdev); |
| 102 | + } |
| 103 | + |
| 104 | + /** |
| 105 | +@@ -1559,21 +1581,23 @@ static int __init pci_sysfs_init(void) |
| 106 | + { |
| 107 | + struct pci_dev *pdev = NULL; |
| 108 | + struct pci_bus *pbus = NULL; |
| 109 | +- int retval; |
| 110 | ++ int retval = 0; |
| 111 | + |
| 112 | + sysfs_initialized = 1; |
| 113 | + for_each_pci_dev(pdev) { |
| 114 | +- retval = pci_create_sysfs_dev_files(pdev); |
| 115 | ++ retval = __pci_create_sysfs_dev_files(pdev); |
| 116 | + if (retval) { |
| 117 | + pci_dev_put(pdev); |
| 118 | +- return retval; |
| 119 | ++ goto exit; |
| 120 | + } |
| 121 | + } |
| 122 | + |
| 123 | + while ((pbus = pci_find_next_bus(pbus))) |
| 124 | + pci_create_legacy_files(pbus); |
| 125 | + |
| 126 | +- return 0; |
| 127 | ++exit: |
| 128 | ++ complete_all(&sysfs_init_completion); |
| 129 | ++ return retval; |
| 130 | + } |
| 131 | + late_initcall(pci_sysfs_init); |
| 132 | + |
| 133 | +--- a/include/linux/pci.h |
| 134 | ++++ b/include/linux/pci.h |
| 135 | +@@ -484,6 +484,7 @@ struct pci_dev { |
| 136 | + unsigned int rom_attr_enabled:1; /* Display of ROM attribute enabled? */ |
| 137 | + pci_dev_flags_t dev_flags; |
| 138 | + atomic_t enable_cnt; /* pci_enable_device has been called */ |
| 139 | ++ bool sysfs_init; /* sysfs entry has been created */ |
| 140 | + |
| 141 | + spinlock_t pcie_cap_lock; /* Protects RMW ops in capability accessors */ |
| 142 | + u32 saved_config_space[16]; /* Config space saved at suspend time */ |
0 commit comments