[PATCH v2 0/2] powerpc: fix oops in pcibios_release_device() after pcibios_free_controller()

[Mauricio Faria de Oliveira]

This patchset addresses the problem/suggestion discussed previously [1],
by adding a kref reference counting to the PHB (struct pci_controller):

> It's possible to hit an oops/crash if pcibios_release_device() accesses the
> phb struct and it had been freed earlier -- by pcibios_free_controller() --
> as the memory it pointed to can be reused.

> If after reuse 'phb->controller_ops.release_device' is non-NULL it will be
> called, but it points to an invalid location (that function pointer is not
> set anywhere in the code, so if it's non-NULL, that's not correct), and so
> it hits an oops and the system crashes.

> That problem can happen with the pSeries platform's DLPAR remove operation
> if references to devices are held until after the pcibios_free_controller()
> function runs, and then released - exercising pcibios_release_device() path.

More problem details/call trace are described in the original submission [1].

With the patch applied (tested on 4.8-rc1), the test-case demonstrates that
the PHB is only released/freed after the last reference to the PCI device(s)
is dropped:

  Enable debugging messages:

  # echo 'file pci-common.c +pf; file pci-hotplug.c +pf' > /sys/kernel/debug/dynamic_debug/control
  # echo 8 > /proc/sys/kernel/printk

  Hold references to both PCI devices in the PHB:

  # ls -ld /sys/block/sd* | grep -m1 0021:01:00.0
  <...> /sys/block/sdaa -> ../devices/pci0021:01/0021:01:00.0/<...>
  
  # ls -ld /sys/block/sd* | grep -m1 0021:01:00.1
  <...> /sys/block/sdab -> ../devices/pci0021:01/0021:01:00.1/<...>
  
  # cat > /dev/sdaa & pid1=$!
  # cat > /dev/sdab & pid2=$!
  
  Perform DLPAR remove of the PHB:

  # drmgr -w 5 -d 1 -c phb -s 'PHB 33' -r
  Validating PHB DLPAR capability...yes.
  [  888.776964] pci_hp_remove_devices: PCI: Removing devices on bus 0021:01
  [  888.776983] pci_hp_remove_devices:    Removing 0021:01:00.0...
  ...
  [  893.696431] pci_hp_remove_devices:    Removing 0021:01:00.1...
  ...
  [  908.352717] pci_bus 0021:01: busn_res: [bus 01-ff] is released
  [  908.352744] pcibios_remove_bus: PCI 0021:01, pci_bus c0000001e7d59400, phb c0000001e7d57400
  [  908.352753] controller_put: PCI domain 33, phb c0000001e7d57400
  [  908.352811] pcibios_free_controller: PCI domain 33, phb c0000001e7d57400, phb->is_dynamic 1
  [  908.352820] controller_put: PCI domain 33, phb c0000001e7d57400
  [  908.352832] rpadlpar_io: slot PHB 33 removed
  
  Notice the PHB was not freed yet (controller_free() was not called)

  Drop the last references to the PCI devices:

  # kill -9 $pid1
  [  991.221998] pcibios_release_device: PCI 0021:01:00.0, pci_dev c0000001ee0b7000, phb c0000001e7d57400
  [  991.222005] controller_put: PCI domain 33, phb c0000001e7d57400
  
  # kill -9 $pid2
  [  996.076293] pcibios_release_device: PCI 0021:01:00.1, pci_dev c0000001ee0b3800, phb c0000001e7d57400
  [  996.076299] controller_put: PCI domain 33, phb c0000001e7d57400
  [  996.076303] controller_free: PCI domain: 33, phb c0000001e7d57400, phb->is_dynamic 1

  Notice that only now the PHB was freed.

Note: this patchset currently covers references from struct pci_dev/pci_bus,
which _is_ enough to resolve this particular problem; it does not yet cover
references from struct pci_dn/eeh_pe/eeh_dev (but since those are unchanged
by/unrelated to this patchset, they remain working in the very same manner).

I have gone to great lengths in time studying the relevant code for EEH in
order to implement those too, but am not yet sure of all the details (e.g.,
lifetime of eeh_dev, removal of pci_dn, etc) that need to be considered to
kfree() them - will likely ask Gavin & maintainers for RFC after some time.

Links:
  [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2016-July/145264.html 

Changelog:
  v2: change approach to use krefs (suggestion by benh & mpe).

Mauricio Faria de Oliveira (2):
  powerpc: add refcount to struct pci_controller
  powerpc: update pci_controller.refcount for PCI devices and buses

 arch/powerpc/include/asm/pci-bridge.h | 15 ++++++++
 arch/powerpc/kernel/pci-common.c      | 72 +++++++++++++++++++++++++++++++++--
 arch/powerpc/kernel/pci-hotplug.c     | 29 ++++++++++++++
 arch/powerpc/kernel/pci_of_scan.c     |  1 +
 4 files changed, 114 insertions(+), 3 deletions(-)

-- 
1.8.3.1