6.1-stable review patch. If anyone has any objections, please let me know.
------------------
From: Daniel Stodden daniel.stodden@gmail.com
[ Upstream commit cbf937dcadfd571a434f8074d057b32cd14fbea5 ]
Before 456d8aa37d0f ("PCI/ASPM: Disable ASPM on MFD function removal to avoid use-after-free"), we would free the ASPM link only after the last function on the bus pertaining to the given link was removed.
That was too late. If function 0 is removed before sibling function, link->downstream would point to free'd memory after.
After above change, we freed the ASPM parent link state upon any function removal on the bus pertaining to a given link.
That is too early. If the link is to a PCIe switch with MFD on the upstream port, then removing functions other than 0 first would free a link which still remains parent_link to the remaining downstream ports.
The resulting GPFs are especially frequent during hot-unplug, because pciehp removes devices on the link bus in reverse order.
On that switch, function 0 is the virtual P2P bridge to the internal bus. Free exactly when function 0 is removed -- before the parent link is obsolete, but after all subordinate links are gone.
Link: https://lore.kernel.org/r/e12898835f25234561c9d7de4435590d957b85d9.173492485... Fixes: 456d8aa37d0f ("PCI/ASPM: Disable ASPM on MFD function removal to avoid use-after-free") Signed-off-by: Daniel Stodden dns@arista.com Signed-off-by: Bjorn Helgaas bhelgaas@google.com [kwilczynski: commit log] Signed-off-by: Krzysztof Wilczyński kwilczynski@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/pci/pcie/aspm.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index cf4acea6610d5..20ec2a4aae65f 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -1031,16 +1031,16 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev) parent_link = link->parent;
/* - * link->downstream is a pointer to the pci_dev of function 0. If - * we remove that function, the pci_dev is about to be deallocated, - * so we can't use link->downstream again. Free the link state to - * avoid this. + * Free the parent link state, no later than function 0 (i.e. + * link->downstream) being removed. * - * If we're removing a non-0 function, it's possible we could - * retain the link state, but PCIe r6.0, sec 7.5.3.7, recommends - * programming the same ASPM Control value for all functions of - * multi-function devices, so disable ASPM for all of them. + * Do not free the link state any earlier. If function 0 is a + * switch upstream port, this link state is parent_link to all + * subordinate ones. */ + if (pdev != link->downstream) + goto out; + pcie_config_aspm_link(link, 0); list_del(&link->sibling); free_link_state(link); @@ -1051,6 +1051,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev) pcie_config_aspm_path(parent_link); }
+ out: mutex_unlock(&aspm_lock); up_read(&pci_bus_sem); }