Messages in this thread Patch in this message | | | Subject | Re: [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal | From | Tyrel Datwyler <> | Date | Fri, 16 Apr 2021 13:21:09 -0700 |
| |
On 4/16/21 12:15 AM, Daniel Axtens wrote: > Hi Tyrel, > >> The pci_bus->bridge reference may no longer be valid after >> pci_bus_remove() resulting in passing a bad value to device_unregister() >> for the associated bridge device. >> >> Store the host_bridge reference in a separate variable prior to >> pci_bus_remove(). >> > The patch certainly seems to do what you say. I'm not really up on the > innards of PCI, so I'm struggling to figure out by what code path > pci_bus_remove() might invalidate pci_bus->bridge? A quick look at > pci_remove_bus was not very illuminating but I didn't chase down every > call it made.
remove_phb_dynamic() |--> pci_remove_bus(bus) |--> device_unregister(&bus->dev) |--> put_device(dev) |--> device_release(kobj) |--> dev->class->dev_release(dev) == release_pci_bus(dev) |--> kfree(bus)
We have the above call chain that takes place in the when put_device() triggers the kobject ref count to go zero. The kobject_release function in this case is device_release() which in turn calls dev->class->dev_release(dev). For a pci_bus the class is appropriately pcibus_class whose dev_release() callback points to release_pci_bus(). This in turn calls kfree() on the bus. Which means we can no longer safely dereference any fields of the pci_bus struct.
-Tyrel
> > Kind regards, > Daniel > >> Fixes: 7340056567e3 ("powerpc/pci: Reorder pci bus/bridge unregistration during PHB removal") >> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com> >> --- >> arch/powerpc/platforms/pseries/pci_dlpar.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c >> index f9ae17e8a0f4..a8f9140a24fa 100644 >> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c >> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c >> @@ -50,6 +50,7 @@ EXPORT_SYMBOL_GPL(init_phb_dynamic); >> int remove_phb_dynamic(struct pci_controller *phb) >> { >> struct pci_bus *b = phb->bus; >> + struct pci_host_bridge *host_bridge = to_pci_host_bridge(b->bridge); >> struct resource *res; >> int rc, i; >> >> @@ -76,7 +77,8 @@ int remove_phb_dynamic(struct pci_controller *phb) >> /* Remove the PCI bus and unregister the bridge device from sysfs */ >> phb->bus = NULL; >> pci_remove_bus(b); >> - device_unregister(b->bridge); >> + host_bridge->bus = NULL; >> + device_unregister(&host_bridge->dev); >> >> /* Now release the IO resource */ >> if (res->flags & IORESOURCE_IO) >> -- >> 2.27.0
| |