[PATCH] powerpc/pseries: Fix use after free in remove_phb_dynamic()

[PATCH] powerpc/pseries: Fix use after free in remove_phb_dynamic()

[Michael Ellerman]

In remove_phb_dynamic() we use &phb->io_resource, after we've called
device_unregister(&host_bridge->dev). But the unregister may have freed
phb, because pcibios_free_controller_deferred() is the release function
for the host_bridge.

If there are no outstanding references when we call device_unregister()
then phb will be freed out from under us.

This has gone mainly unnoticed, but with slub_debug and page_poison
enabled it can lead to a crash:

  PID: 7574   TASK: c0000000d492cb80  CPU: 13  COMMAND: "drmgr"
   #0 [c0000000e4f075a0] crash_kexec at c00000000027d7dc
   #1 [c0000000e4f075d0] oops_end at c000000000029608
   #2 [c0000000e4f07650] __bad_page_fault at c0000000000904b4
   #3 [c0000000e4f076c0] do_bad_slb_fault at c00000000009a5a8
   #4 [c0000000e4f076f0] data_access_slb_common_virt at c000000000008b30
   Data SLB Access [380] exception frame:
   R0:  c000000000167250    R1:  c0000000e4f07a00    R2:  c000000002a46100
   R3:  c000000002b39ce8    R4:  00000000000000c0    R5:  00000000000000a9
   R6:  3894674d000000c0    R7:  0000000000000000    R8:  00000000000000ff
   R9:  0000000000000100    R10: 6b6b6b6b6b6b6b6b    R11: 0000000000008000
   R12: c00000000023da80    R13: c0000009ffd38b00    R14: 0000000000000000
   R15: 000000011c87f0f0    R16: 0000000000000006    R17: 0000000000000003
   R18: 0000000000000002    R19: 0000000000000004    R20: 0000000000000005
   R21: 000000011c87ede8    R22: 000000011c87c5a8    R23: 000000011c87d3a0
   R24: 0000000000000000    R25: 0000000000000001    R26: c0000000e4f07cc8
   R27: c00000004d1cc400    R28: c0080000031d00e8    R29: c00000004d23d800
   R30: c00000004d1d2400    R31: c00000004d1d2540
   NIP: c000000000167258    MSR: 8000000000009033    OR3: c000000000e9f474
   CTR: 0000000000000000    LR:  c000000000167250    XER: 0000000020040003
   CCR: 0000000024088420    MQ:  0000000000000000    DAR: 6b6b6b6b6b6b6ba3
   DSISR: c0000000e4f07920     Syscall Result: fffffffffffffff2
   [NIP  : release_resource+56]
   [LR   : release_resource+48]
   #5 [c0000000e4f07a00] release_resource at c000000000167258  (unreliable)
   #6 [c0000000e4f07a30] remove_phb_dynamic at c000000000105648
   #7 [c0000000e4f07ab0] dlpar_remove_slot at c0080000031a09e8 [rpadlpar_io]
   #8 [c0000000e4f07b50] remove_slot_store at c0080000031a0b9c [rpadlpar_io]
   #9 [c0000000e4f07be0] kobj_attr_store at c000000000817d8c
  #10 [c0000000e4f07c00] sysfs_kf_write at c00000000063e504
  #11 [c0000000e4f07c20] kernfs_fop_write_iter at c00000000063d868
  #12 [c0000000e4f07c70] new_sync_write at c00000000054339c
  #13 [c0000000e4f07d10] vfs_write at c000000000546624
  #14 [c0000000e4f07d60] ksys_write at c0000000005469f4
  #15 [c0000000e4f07db0] system_call_exception at c000000000030840
  #16 [c0000000e4f07e10] system_call_vectored_common at c00000000000c168

To avoid it, we can take a reference to the host_bridge->dev until we're
done using phb. Then when we drop the reference the phb will be freed.

Fixes: 2dd9c11b9d4d ("powerpc/pseries: use pci_host_bridge.release_fn() to kfree(phb)")
Reported-by: David Dai <zdai@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/platforms/pseries/pci_dlpar.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
index 90c9d3531694..4ba824568119 100644
--- a/arch/powerpc/platforms/pseries/pci_dlpar.c
+++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
@@ -78,6 +78,9 @@ int remove_phb_dynamic(struct pci_controller *phb)
 
 	pseries_msi_free_domains(phb);
 
+	/* Keep a reference so phb isn't freed yet */
+	get_device(&host_bridge->dev);
+
 	/* Remove the PCI bus and unregister the bridge device from sysfs */
 	phb->bus = NULL;
 	pci_remove_bus(b);
@@ -101,6 +104,7 @@ int remove_phb_dynamic(struct pci_controller *phb)
 	 * the pcibios_free_controller_deferred() callback;
 	 * see pseries_root_bridge_prepare().
 	 */
+	put_device(&host_bridge->dev);
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH] powerpc/pseries: Fix use after free in remove_phb_dynamic()

[Sachin Sant]

> On 18-Mar-2022, at 9:12 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> To avoid it, we can take a reference to the host_bridge->dev until we're
> done using phb. Then when we drop the reference the phb will be freed.
> 
> Fixes: 2dd9c11b9d4d ("powerpc/pseries: use pci_host_bridge.release_fn() to kfree(phb)")
> Reported-by: David Dai <zdai@linux.ibm.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> —
Verified successfully with 5.17.0-rc8-00061-g34e047aa16c0 + patch.

Tested-by: Sachin Sant <sachinp@linux.ibm.com>

Without this patch:

# drmgr -c phb -s "PHB 18" -r -d 5
[  178.107171] rpaphp: RPA HOT Plug PCI Controller Driver version: 0.1
[  178.107429] rpaphp: Slot [U78D8.ND0.FGD003M-P0-C2-C0] registered
[  178.107578] rpaphp: Slot [U78D8.ND0.FGD003M-P0-C3-C0] registered
[  178.107721] rpaphp: Slot [U78D8.ND0.FGD003M-P0-C4-C0] registered
[  178.196960] pci_bus 0012:01: busn_res: [bus 01-ff] is released
[  178.197040] BUG: Unable to handle kernel data access on read at 0x6b6b6b6b6b6b6ba3
[  178.197045] Faulting instruction address: 0xc000000000181068
[  178.197049] Oops: Kernel access of bad area, sig: 11 [#1]
[  178.197051] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
[  178.197056] Modules linked in: rpadlpar_io rpaphp dm_mod nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 rfkill bonding ip_set tls nf_tables nfnetlink sunrpc pseries_rng drm drm_panel_orientation_quirks xfs libcrc32c sr_mod sd_mod cdrom t10_pi sg ibmvscsi scsi_transport_srp ibmveth xts vmx_crypto fuse
[  178.197086] CPU: 15 PID: 10565 Comm: drmgr Not tainted 5.17.0-rc8-00061-g34e047aa16c0 #1
[  178.197091] NIP:  c000000000181068 LR: c000000000181060 CTR: 0000000000000000
[  178.197094] REGS: c00000002af5b700 TRAP: 0380   Not tainted  (5.17.0-rc8-00061-g34e047aa16c0)
[  178.197097] MSR:  8000000000009033 <SF,EE,ME,IR,DR,RI,LE>  CR: 24088220  XER: 20040003
[  178.197105] CFAR: c000000000f60564 IRQMASK: 0 
[  178.197105] GPR00: c000000000181060 c00000002af5b9a0 c000000002a78c00 c000000002b79d50 
[  178.197105] GPR04: 0000000000000000 c00c00000015b348 c00c000000030288 0000000000000000 
[  178.197105] GPR08: 00000000000000ff 0000000000000100 6b6b6b6b6b6b6b6b 0000000000008000 
[  178.197105] GPR12: c000000000260940 c000000a5ffedb00 0000000000000000 0000000107a8f0f0 
[  178.197105] GPR16: 0000000000000006 0000000000000003 0000000000000002 0000000000000004 
[  178.197105] GPR20: 0000000000000005 0000000107a8ede8 0000000107a8c5a8 c000000002abae85 
[  178.197105] GPR24: 0000000000000000 0000000000000001 c00000000cabd078 c00000000c50ac00 
[  178.197105] GPR28: 0000000000000000 c000000a851cac00 c00000000c501c00 c00000000c501d38 
[  178.197139] NIP [c000000000181068] release_resource+0x38/0xf0
[  178.197146] LR [c000000000181060] release_resource+0x30/0xf0
[  178.197149] Call Trace:
[  178.197151] [c00000002af5b9a0] [c0000000008c4814] pci_remove_bus+0xf4/0x110 (unreliable)
[  178.197156] [c00000002af5b9d0] [c00000000011a018] remove_phb_dynamic+0x178/0x190
[  178.197161] [c00000002af5ba50] [c0080000086f09e8] dlpar_remove_slot+0x1d0/0x250 [rpadlpar_io]
[  178.197166] [c00000002af5baf0] [c0080000086f0b5c] remove_slot_store+0xa4/0x160 [rpadlpar_io]
[  178.197170] [c00000002af5bb80] [c00000000088a6dc] kobj_attr_store+0x2c/0x50
[  178.197174] [c00000002af5bba0] [c0000000006b19e4] sysfs_kf_write+0x64/0x80
[  178.197179] [c00000002af5bbc0] [c0000000006b0e0c] kernfs_fop_write_iter+0x1bc/0x2b0
[  178.197183] [c00000002af5bc10] [c0000000005816a4] new_sync_write+0x124/0x1c0
[  178.197187] [c00000002af5bcb0] [c000000000585954] vfs_write+0x2c4/0x390
[  178.197190] [c00000002af5bd00] [c000000000585d24] ksys_write+0x84/0x140
[  178.197194] [c00000002af5bd50] [c000000000037024] system_call_exception+0x254/0x550
[  178.197198] [c00000002af5be10] [c00000000000bfe8] system_call_vectored_common+0xe8/0x278
[  178.197203] --- interrupt: 3000 at 0x7fffa7118774
[  178.197206] NIP:  00007fffa7118774 LR: 0000000000000000 CTR: 0000000000000000
[  178.197208] REGS: c00000002af5be80 TRAP: 3000   Not tainted  (5.17.0-rc8-00061-g34e047aa16c0)
[  178.197211] MSR:  800000000280f033 <SF,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE>  CR: 44084404  XER: 00000000
[  178.197220] IRQMASK: 0 
[  178.197220] GPR00: 0000000000000004 00007ffff50a4ae0 00007fffa7227100 0000000000000006 
[  178.197220] GPR04: 0000000140744930 0000000000000006 0000000000000000 0000000000000001 
[  178.197220] GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
[  178.197220] GPR12: 0000000000000000 00007fffa733b100 0000000000000000 0000000107a8f0f0 
[  178.197220] GPR16: 0000000000000006 0000000000000003 0000000000000002 0000000000000004 
[  178.197220] GPR20: 0000000000000005 0000000107a8ede8 0000000107a8c5a8 0000000107a8d3a0 
[  178.197220] GPR24: 0000000000000006 0000000107a8b3c0 0000000140744930 0000000000000006 
[  178.197220] GPR28: 0000000000000006 00000001407202a0 0000000140744930 0000000000000006 
[  178.197251] NIP [00007fffa7118774] 0x7fffa7118774
[  178.197254] LR [0000000000000000] 0x0
[  178.197255] --- interrupt: 3000
[  178.197257] Instruction dump:
[  178.197259] 7c0802a6 60000000 7c0802a6 fbe1fff8 7c7f1b78 3c620010 38631150 f8010010 
[  178.197265] f821ffd1 48ddf4dd 60000000 e95f0028 <e92a0038> 2c290000 41820028 7c3f4840 
[  178.197273] ---[ end trace 0000000000000000 ]---

With the patch applied both add and remove operation works correctly.

# lsslot -c phb | grep 18
PHB 18    /pci@800000020000012 U78D8.ND0.FGD003M-P0-C2-C0
# drmgr -c phb -s "PHB 18" -r -d 5

########## Mar 20 02:58:53 2022 ##########
drmgr: -c phb -s PHB 18 -r -d 5 
Validating PHB DLPAR capability...yes.
Getting node types 0x00000010
……..
Retrieving hotplug nodes
hp adapter status for U78D8.ND0.FGD003M-P0-C2-C0 is 0
setting hp adapter status to UNCONFIG adapter for U78D8.ND0.FGD003M-P0-C2-C0
hp adapter status for U78D8.ND0.FGD003M-P0-C2-C0 is 0
performing kernel op for PHB 18, file is /sys/bus/pci/slots/control/remove_slot
Removing device-tree node /proc/device-tree/pci@800000020000012
Removing device-tree node /proc/device-tree/interrupt-controller@800000025000012
Releasing drc index 0x20000012
get-sensor for 20000012: 0, 1
Setting isolation state to 'isolate'
Setting allocation state to 'alloc unusable'
get-sensor for 20000012: 0, 2
drc_index 20000012 sensor-state: 2
Resource is not available to the partition.
########## Mar 20 02:58:54 2022 ##########
# lsslot -c phb | grep 18 
#  drmgr -c phb -s "PHB 18" -a -d 5

########## Mar 20 03:00:16 2022 ##########
drmgr: -c phb -s PHB 18 -a -d 5 
Validating PHB DLPAR capability...yes.
Getting node types 0x00000010

…..
get-sensor for 22010012: 0, 0
performing kernel op for PHB 18, file is /sys/bus/pci/slots/control/add_slot
########## Mar 20 03:00:19 2022 ##########
# lsslot -c phb | grep 18
PHB 18    /pci@800000020000012 U78D8.ND0.FGD003M-P0-C2-C0
# 


- Sachin

Re: [PATCH] powerpc/pseries: Fix use after free in remove_phb_dynamic()

[Michael Ellerman]

Sachin Sant <sachinp@linux.ibm.com> writes:
>> On 18-Mar-2022, at 9:12 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> To avoid it, we can take a reference to the host_bridge->dev until we're
>> done using phb. Then when we drop the reference the phb will be freed.
>> 
>> Fixes: 2dd9c11b9d4d ("powerpc/pseries: use pci_host_bridge.release_fn() to kfree(phb)")
>> Reported-by: David Dai <zdai@linux.ibm.com>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> —
> Verified successfully with 5.17.0-rc8-00061-g34e047aa16c0 + patch.
>
> Tested-by: Sachin Sant <sachinp@linux.ibm.com>

Thanks for testing.

cheers

Re: [PATCH] powerpc/pseries: Fix use after free in remove_phb_dynamic()

[Michael Ellerman]

On Fri, 18 Mar 2022 14:42:19 +1100, Michael Ellerman wrote:
> In remove_phb_dynamic() we use &phb->io_resource, after we've called
> device_unregister(&host_bridge->dev). But the unregister may have freed
> phb, because pcibios_free_controller_deferred() is the release function
> for the host_bridge.
> 
> If there are no outstanding references when we call device_unregister()
> then phb will be freed out from under us.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/pseries: Fix use after free in remove_phb_dynamic()
      https://git.kernel.org/powerpc/c/fe2640bd7a62f1f7c3f55fbda31084085075bc30

cheers