Message ID | 20220318034219.1188008-1-mpe@ellerman.id.au (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | powerpc/pseries: Fix use after free in remove_phb_dynamic() | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
> 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
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
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
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; }
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(+)