Commit 5ef753ae authored by Oliver O'Halloran's avatar Oliver O'Halloran Committed by Michael Ellerman

powerpc/eeh: Fix race when freeing PDNs

When hot-adding devices we rely on the hotplug driver to create pci_dn's
for the devices under the hotplug slot. Converse, when hot-removing the
driver will remove the pci_dn's that it created. This is a problem because
the pci_dev is still live until it's refcount drops to zero. This can
happen if the driver is slow to tear down it's internal state. Ideally, the
driver would not attempt to perform any config accesses to the device once
it's been marked as removed, but sometimes it happens. As a result, we
might attempt to access the pci_dn for a device that has been torn down and
the kernel may crash as a result.

To fix this, don't free the pci_dn unless the corresponding pci_dev has
been released.  If the pci_dev is still live, then we mark the pci_dn with
a flag that indicates the pci_dev's release function should free it.
Signed-off-by: default avatarOliver O'Halloran <oohall@gmail.com>
Signed-off-by: default avatarMichael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20190903101605.2890-3-oohall@gmail.com
parent 799abe28
...@@ -183,6 +183,7 @@ struct iommu_table; ...@@ -183,6 +183,7 @@ struct iommu_table;
struct pci_dn { struct pci_dn {
int flags; int flags;
#define PCI_DN_FLAG_IOV_VF 0x01 #define PCI_DN_FLAG_IOV_VF 0x01
#define PCI_DN_FLAG_DEAD 0x02 /* Device has been hot-removed */
int busno; /* pci bus number */ int busno; /* pci bus number */
int devfn; /* pci device and function number */ int devfn; /* pci device and function number */
......
...@@ -55,11 +55,18 @@ EXPORT_SYMBOL_GPL(pci_find_bus_by_node); ...@@ -55,11 +55,18 @@ EXPORT_SYMBOL_GPL(pci_find_bus_by_node);
void pcibios_release_device(struct pci_dev *dev) void pcibios_release_device(struct pci_dev *dev)
{ {
struct pci_controller *phb = pci_bus_to_host(dev->bus); struct pci_controller *phb = pci_bus_to_host(dev->bus);
struct pci_dn *pdn = pci_get_pdn(dev);
eeh_remove_device(dev); eeh_remove_device(dev);
if (phb->controller_ops.release_device) if (phb->controller_ops.release_device)
phb->controller_ops.release_device(dev); phb->controller_ops.release_device(dev);
/* free()ing the pci_dn has been deferred to us, do it now */
if (pdn && (pdn->flags & PCI_DN_FLAG_DEAD)) {
pci_dbg(dev, "freeing dead pdn\n");
kfree(pdn);
}
} }
/** /**
......
...@@ -323,6 +323,7 @@ void pci_remove_device_node_info(struct device_node *dn) ...@@ -323,6 +323,7 @@ void pci_remove_device_node_info(struct device_node *dn)
{ {
struct pci_dn *pdn = dn ? PCI_DN(dn) : NULL; struct pci_dn *pdn = dn ? PCI_DN(dn) : NULL;
struct device_node *parent; struct device_node *parent;
struct pci_dev *pdev;
#ifdef CONFIG_EEH #ifdef CONFIG_EEH
struct eeh_dev *edev = pdn_to_eeh_dev(pdn); struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
...@@ -336,12 +337,28 @@ void pci_remove_device_node_info(struct device_node *dn) ...@@ -336,12 +337,28 @@ void pci_remove_device_node_info(struct device_node *dn)
WARN_ON(!list_empty(&pdn->child_list)); WARN_ON(!list_empty(&pdn->child_list));
list_del(&pdn->list); list_del(&pdn->list);
/* Drop the parent pci_dn's ref to our backing dt node */
parent = of_get_parent(dn); parent = of_get_parent(dn);
if (parent) if (parent)
of_node_put(parent); of_node_put(parent);
/*
* At this point we *might* still have a pci_dev that was
* instantiated from this pci_dn. So defer free()ing it until
* the pci_dev's release function is called.
*/
pdev = pci_get_domain_bus_and_slot(pdn->phb->global_number,
pdn->busno, pdn->devfn);
if (pdev) {
/* NB: pdev has a ref to dn */
pci_dbg(pdev, "marked pdn (from %pOF) as dead\n", dn);
pdn->flags |= PCI_DN_FLAG_DEAD;
} else {
dn->data = NULL; dn->data = NULL;
kfree(pdn); kfree(pdn);
}
pci_dev_put(pdev);
} }
EXPORT_SYMBOL_GPL(pci_remove_device_node_info); EXPORT_SYMBOL_GPL(pci_remove_device_node_info);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment