Skip to content

Commit 11a1f4b

Browse files
l1kkwilczynski
authored andcommitted
PCI/DPC: Fix use-after-free on concurrent DPC and hot-removal
Keith reports a use-after-free when a DPC event occurs concurrently to hot-removal of the same portion of the hierarchy: The dpc_handler() awaits readiness of the secondary bus below the Downstream Port where the DPC event occurred. To do so, it polls the config space of the first child device on the secondary bus. If that child device is concurrently removed, accesses to its struct pci_dev cause the kernel to oops. That's because pci_bridge_wait_for_secondary_bus() neglects to hold a reference on the child device. Before v6.3, the function was only called on resume from system sleep or on runtime resume. Holding a reference wasn't necessary back then because the pciehp IRQ thread could never run concurrently. (On resume from system sleep, IRQs are not enabled until after the resume_noirq phase. And runtime resume is always awaited before a PCI device is removed.) However starting with v6.3, pci_bridge_wait_for_secondary_bus() is also called on a DPC event. Commit 53b54ad ("PCI/DPC: Await readiness of secondary bus after reset"), which introduced that, failed to appreciate that pci_bridge_wait_for_secondary_bus() now needs to hold a reference on the child device because dpc_handler() and pciehp may indeed run concurrently. The commit was backported to v5.10+ stable kernels, so that's the oldest one affected. Add the missing reference acquisition. Abridged stack trace: BUG: unable to handle page fault for address: 00000000091400c0 CPU: 15 PID: 2464 Comm: irq/53-pcie-dpc 6.9.0 RIP: pci_bus_read_config_dword+0x17/0x50 pci_dev_wait() pci_bridge_wait_for_secondary_bus() dpc_reset_link() pcie_do_recovery() dpc_handler() Fixes: 53b54ad ("PCI/DPC: Await readiness of secondary bus after reset") Closes: https://lore.kernel.org/r/[email protected]/ Link: https://lore.kernel.org/linux-pci/8e4bcd4116fd94f592f2bf2749f168099c480ddf.1718707743.git.lukas@wunner.de Reported-by: Keith Busch <[email protected]> Tested-by: Keith Busch <[email protected]> Signed-off-by: Lukas Wunner <[email protected]> Signed-off-by: Krzysztof Wilczyński <[email protected]> Reviewed-by: Keith Busch <[email protected]> Reviewed-by: Mika Westerberg <[email protected]> Cc: [email protected] # v5.10+
1 parent 1613e60 commit 11a1f4b

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

drivers/pci/pci.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4753,7 +4753,7 @@ static int pci_bus_max_d3cold_delay(const struct pci_bus *bus)
47534753
*/
47544754
int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
47554755
{
4756-
struct pci_dev *child;
4756+
struct pci_dev *child __free(pci_dev_put) = NULL;
47574757
int delay;
47584758

47594759
if (pci_dev_is_disconnected(dev))
@@ -4782,8 +4782,8 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
47824782
return 0;
47834783
}
47844784

4785-
child = list_first_entry(&dev->subordinate->devices, struct pci_dev,
4786-
bus_list);
4785+
child = pci_dev_get(list_first_entry(&dev->subordinate->devices,
4786+
struct pci_dev, bus_list));
47874787
up_read(&pci_bus_sem);
47884788

47894789
/*

0 commit comments

Comments
 (0)