On Tue, Jun 18, 2024 at 12:54:55PM +0200, Lukas Wunner wrote:
> 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 53b54ad074de ("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: 53b54ad074de ("PCI/DPC: Await readiness of secondary bus after reset")
> Reported-by: Keith Busch <kbusch@xxxxxxxxxx>
> Closes: https://lore.kernel.org/r/20240612181625.3604512-3-kbusch@xxxxxxxx/
> Tested-by: Keith Busch <kbusch@xxxxxxxxxx>
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> Reviewed-by: Keith Busch <kbusch@xxxxxxxxxx>
Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
