LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal
@ 2021-02-11 18:24 Tyrel Datwyler
  2021-04-13 19:04 ` Tyrel Datwyler
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Tyrel Datwyler @ 2021-02-11 18:24 UTC (permalink / raw)
  To: mpe; +Cc: linuxppc-dev, linux-kernel, Tyrel Datwyler

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().

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal
  2021-02-11 18:24 [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal Tyrel Datwyler
@ 2021-04-13 19:04 ` Tyrel Datwyler
  2021-04-16  7:15 ` Daniel Axtens
  2021-04-19  4:00 ` Michael Ellerman
  2 siblings, 0 replies; 5+ messages in thread
From: Tyrel Datwyler @ 2021-04-13 19:04 UTC (permalink / raw)
  To: mpe; +Cc: linuxppc-dev, linux-kernel

On 2/11/21 10:24 AM, Tyrel Datwyler wrote:
> 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().
> 
> Fixes: 7340056567e3 ("powerpc/pci: Reorder pci bus/bridge unregistration during PHB removal")
> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>

Ping?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal
  2021-02-11 18:24 [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal Tyrel Datwyler
  2021-04-13 19:04 ` Tyrel Datwyler
@ 2021-04-16  7:15 ` Daniel Axtens
  2021-04-16 20:21   ` Tyrel Datwyler
  2021-04-19  4:00 ` Michael Ellerman
  2 siblings, 1 reply; 5+ messages in thread
From: Daniel Axtens @ 2021-04-16  7:15 UTC (permalink / raw)
  To: Tyrel Datwyler, mpe; +Cc: Tyrel Datwyler, linuxppc-dev, linux-kernel

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.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal
  2021-04-16  7:15 ` Daniel Axtens
@ 2021-04-16 20:21   ` Tyrel Datwyler
  0 siblings, 0 replies; 5+ messages in thread
From: Tyrel Datwyler @ 2021-04-16 20:21 UTC (permalink / raw)
  To: Daniel Axtens, mpe; +Cc: linuxppc-dev, linux-kernel

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal
  2021-02-11 18:24 [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal Tyrel Datwyler
  2021-04-13 19:04 ` Tyrel Datwyler
  2021-04-16  7:15 ` Daniel Axtens
@ 2021-04-19  4:00 ` Michael Ellerman
  2 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2021-04-19  4:00 UTC (permalink / raw)
  To: mpe, Tyrel Datwyler; +Cc: linuxppc-dev, linux-kernel

On Thu, 11 Feb 2021 12:24:35 -0600, Tyrel Datwyler wrote:
> 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().

Applied to powerpc/next.

[1/1] powerpc/pseries: extract host bridge from pci_bus prior to bus removal
      https://git.kernel.org/powerpc/c/38d0b1c9cec71e6d0f3bddef0bbce41d05a3e796

cheers

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-11 18:24 [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal Tyrel Datwyler
2021-04-13 19:04 ` Tyrel Datwyler
2021-04-16  7:15 ` Daniel Axtens
2021-04-16 20:21   ` Tyrel Datwyler
2021-04-19  4:00 ` Michael Ellerman

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git