| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20201013133109.49821-1-cristian.marussi@arm.com>
Date: Tue, 13 Oct 2020 14:31:09 +0100
From: Cristian Marussi <cristian.marussi@....com>
To: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Cc: sudeep.holla@....com, cristian.marussi@....com
Subject: [PATCH] firmware: arm_scmi: fix notifications locking
When a protocol registers its events the notification core takes care to
re-scan the hashtable of pending event handlers and activate all the
possibly existent handlers that refer to any of the events just registered
by the new protocol; when a pending handler becomes active the core takes
also care to ask the SCMI platform to enable the corresponding events'
notifications in the SCMI firmware.
If, for whatever reason, the enable fails such invalid event handler must
be finally removed and freed but it must be treated as an active handler
like it has just become: ensure to use the scmi_put_active_handler() helper
which handles properly the needed additional mutexing.
Failing to properly acquire all the needed mutexes exposes a race that
leads to the following splat being observed:
[ 212.840876] ------------[ cut here ]------------
[ 212.845569] refcount_t: underflow; use-after-free.
[ 212.850544] WARNING: CPU: 0 PID: 388 at lib/refcount.c:28 refcount_warn_saturate+0xf8/0x148
[ 212.858913] Modules linked in: dummy_scmi_consumer(-) scmi_perf [last unloaded: scmi_cpufreq]
[ 212.867478] CPU: 0 PID: 388 Comm: rmmod Tainted: G W 5.9.0-rc1-00020-g9c4395e7867d-dirty #4
[ 212.877153] Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Jun 30 2020
[ 212.887963] pstate: 40000005 (nZcv daif -PAN -UAO BTYPE=--)
[ 212.893554] pc : refcount_warn_saturate+0xf8/0x148
[ 212.898361] lr : refcount_warn_saturate+0xf8/0x148
[ 212.903160] sp : ffff80001298bc00
[ 212.906480] x29: ffff80001298bc00 x28: ffff00097470aac0
[ 212.911807] x27: 0000000000000000 x26: 0000000000000000
[ 212.917135] x25: ffff00097357fc80 x24: 0000000000000002
[ 212.922462] x23: ffff00097357fca8 x22: 0000000000000003
[ 212.927790] x21: ffff000974474688 x20: ffff000971e04f00
[ 212.933117] x19: 0000000000000001 x18: 0000000000000010
[ 212.938444] x17: 0000000000000000 x16: 0000000000000000
[ 212.943771] x15: ffffffffffffffff x14: ffff800012269948
[ 212.949098] x13: ffff80009298b947 x12: ffff80001298b94f
[ 212.954426] x11: 0001000000000000 x10: 0000000000000a10
[ 212.959753] x9 : ffff80001039fe88 x8 : ffff00097470b530
[ 212.965080] x7 : 00000000ffffffff x6 : ffff00097ef1b200
[ 212.970407] x5 : ffff00097ef1b200 x4 : 0000000000000000
[ 212.975734] x3 : ffff00097ef2a398 x2 : 0000000000000001
[ 212.981060] x1 : 4b16c3af5d721600 x0 : 0000000000000000
[ 212.986388] Call trace:
[ 212.988847] refcount_warn_saturate+0xf8/0x148
[ 212.993308] scmi_put_handler_unlocked.isra.10+0x204/0x208
[ 212.998811] scmi_put_handler+0x50/0xa0
[ 213.002659] scmi_unregister_notifier+0x1bc/0x240
[ 213.007385] scmi_notify_tester_remove+0x4c/0x68 [dummy_scmi_consumer]
[ 213.013931] scmi_dev_remove+0x54/0x68
[ 213.017695] device_release_driver_internal+0x114/0x1e8
[ 213.022934] driver_detach+0x58/0xe8
[ 213.026520] bus_remove_driver+0x88/0xe0
[ 213.030454] driver_unregister+0x38/0x68
[ 213.034389] scmi_driver_unregister+0x1c/0x28
[ 213.038763] scmi_drv_exit+0x1c/0xae0 [dummy_scmi_consumer]
[ 213.044354] __arm64_sys_delete_module+0x1a4/0x268
[ 213.049160] el0_svc_common.constprop.3+0x94/0x178
[ 213.053963] do_el0_svc+0x2c/0x98
[ 213.057290] el0_sync_handler+0x148/0x1a8
[ 213.061310] el0_sync+0x158/0x180
[ 213.064632] ---[ end trace 5bff2d25d5820911 ]---
Fixes: e7c215f358a35 ("firmware: arm_scmi: Add notification callbacks-registration")
Signed-off-by: Cristian Marussi <cristian.marussi@....com>
---
Applied and tested on [1] on top of :
commit: 9724722fde8f ("firmware: arm_scmi: Add missing Rx size
re-initialisation")
[1]:https://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git/log/?h=for-next/scmi
---
---
drivers/firmware/arm_scmi/notify.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
index 2754f9d01636..c24e427dce0d 100644
--- a/drivers/firmware/arm_scmi/notify.c
+++ b/drivers/firmware/arm_scmi/notify.c
@@ -1403,15 +1403,21 @@ static void scmi_protocols_late_init(struct work_struct *work)
"finalized PENDING handler - key:%X\n",
hndl->key);
ret = scmi_event_handler_enable_events(hndl);
+ if (ret) {
+ dev_dbg(ni->handle->dev,
+ "purging INVALID handler - key:%X\n",
+ hndl->key);
+ scmi_put_active_handler(ni, hndl);
+ }
} else {
ret = scmi_valid_pending_handler(ni, hndl);
- }
- if (ret) {
- dev_dbg(ni->handle->dev,
- "purging PENDING handler - key:%X\n",
- hndl->key);
- /* this hndl can be only a pending one */
- scmi_put_handler_unlocked(ni, hndl);
+ if (ret) {
+ dev_dbg(ni->handle->dev,
+ "purging PENDING handler - key:%X\n",
+ hndl->key);
+ /* this hndl can be only a pending one */
+ scmi_put_handler_unlocked(ni, hndl);
+ }
}
}
mutex_unlock(&ni->pending_mtx);
--
2.17.1
Powered by blists - more mailing lists