[Openipmi-developer] bug report: ipmi probe on powernv platform results in use-after-free

[Nilay S. <ni...@li...>]

Hi,

On powernv platform we have been observing following UAF warning during system boot in IPMI
driver code.

========================================================================================

ipmi-powernv ibm,opal:ipmi: IPMI message handler: Unable to get the device id: -5
ipmi-powernv ibm,opal:ipmi: IPMI SMI registration failed (-5)
ipmi-powernv ibm,opal:ipmi: probe with driver ipmi-powernv failed with error -5
------------[ cut here ]------------
refcount_t: addition on 0; use-after-free.
WARNING: lib/refcount.c:25 at refcount_warn_saturate+0x194/0x230, CPU#24: kworker/24:2/1498
Modules linked in: ipmi_powernv ipmi_devintf ipmi_msghandler dm_multipath usbhid scsi_dh_rdac scsi_dh_alua fuse autofs4
CPU: 24 UID: 0 PID: 1498 Comm: kworker/24:2 Not tainted 7.2.0 #18 PREEMPTLAZY
Hardware name: 8335-GTC POWER9 0x4e1203 opal:v7.1-150-g06912e3ba-root-dirty-87f12a4 PowerNV
Workqueue: events smi_work [ipmi_msghandler]
[...]
[...]
NIP [c0000000008f1364] refcount_warn_saturate+0x194/0x230
LR [c0000000008f1360] refcount_warn_saturate+0x190/0x230
Call Trace:
	 refcount_warn_saturate+0x190/0x230 (unreliable)
	 smi_work+0x550/0x560 [ipmi_msghandler]
	 process_one_work+0x19c/0x4d0
	 worker_thread+0x1ec/0x3e0
	 kthread+0x194/0x1b0
	 start_kernel_thread+0x14/0x18
---[ end trace 0000000000000000 ]---
------------[ cut here ]------------
refcount_t: underflow; use-after-free.
WARNING: lib/refcount.c:28 at refcount_warn_saturate+0x214/0x230, CPU#24: kworker/24:2/1498
Modules linked in: ipmi_powernv ipmi_devintf ipmi_msghandler dm_multipath usbhid scsi_dh_rdac scsi_dh_alua fuse autofs4
CPU: 24 UID: 0 PID: 1498 Comm: kworker/24:2 Tainted: G        W           7.2.0 #18 PREEMPTLAZY
Tainted: [W]=WARN
Hardware name: 8335-GTC POWER9 0x4e1203 opal:v7.1-150-g06912e3ba-root-dirty-87f12a4 PowerNV
Workqueue: events smi_work [ipmi_msghandler]
NIP:  c0000000008f13e4 LR: c0000000008f13e0 CTR: 0000000000000000
REGS: c00000006bd1fb10 TRAP: 0700   Tainted: G        W            (7.2.0)
[...]
[...]
NIP [c0000000008f13e4] refcount_warn_saturate+0x214/0x230
LR [c0000000008f13e0] refcount_warn_saturate+0x210/0x230
Call Trace:
	 refcount_warn_saturate+0x210/0x230 (unreliable)
	 smi_work+0x524/0x560 [ipmi_msghandler]
	 process_one_work+0x19c/0x4d0
	 worker_thread+0x1ec/0x3e0
	 kthread+0x194/0x1b0
	 start_kernel_thread+0x14/0x18
---[ end trace 0000000000000000 ]---
====================================================================================================

Bisecting the code point us towards commit 62cd145453d5 ("ipmi:msghandler: Handle error
returns from the SMI sender").

Further analyzing the above commit 62cd145453d5, it seems that it correctly propagates
an error returned by the SMI sender, but its error path unconditionally queues intf->smi_work.
If the sender fails during SMI registration (on powernv it actually does), the
propagated error causes registration to abort and the struct ipmi_smi to be freed while
the queued work can still execute (after ipmi_smi is freed), resulting in a use-after-free.

One possible way to address the lifetime issue may be to take a reference on intf before
queuing smi_work() and release it when the work completes, although I'm not familiar
enough with the intended lifetime rules of struct ipmi_smi to say whether that is the
appropriate fix.

Please let me know if you need any further information or if there are additional
tests/debugging that I could help run on the powernv platform.

Thanks,
--Nilay