| From 278eb30b7c81d43fcff98dc2bc969ca2c14e1fe2 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 13 Oct 2020 14:31:09 +0100 |
| Subject: firmware: arm_scmi: Fix locking in notifications |
| |
| From: Cristian Marussi <cristian.marussi@arm.com> |
| |
| [ Upstream commit c7821c2d9c0dda0adf2bcf88e79b02a19a430be4 ] |
| |
| When a protocol registers its events, the notification core takes care |
| to rescan the hashtable of pending event handlers and activate all the |
| possibly existent handlers referring to any of the events that are just |
| registered by the new protocol. When a pending handler becomes active |
| the core requests and enables the corresponding events in the SCMI |
| firmware. |
| |
| If, for whatever reason, the enable fails, such invalid event handler |
| must be finally removed and freed. Let us ensure to use the |
| scmi_put_active_handler() helper which handles properly the needed |
| additional locking. |
| |
| Failing to properly acquire all the needed mutexes exposes a race that |
| leads to the following splat being observed: |
| |
| WARNING: CPU: 0 PID: 388 at lib/refcount.c:28 refcount_warn_saturate+0xf8/0x148 |
| Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development |
| Platform, BIOS EDK II Jun 30 2020 |
| pstate: 40000005 (nZcv daif -PAN -UAO BTYPE=--) |
| pc : refcount_warn_saturate+0xf8/0x148 |
| lr : refcount_warn_saturate+0xf8/0x148 |
| Call trace: |
| refcount_warn_saturate+0xf8/0x148 |
| scmi_put_handler_unlocked.isra.10+0x204/0x208 |
| scmi_put_handler+0x50/0xa0 |
| scmi_unregister_notifier+0x1bc/0x240 |
| scmi_notify_tester_remove+0x4c/0x68 [dummy_scmi_consumer] |
| scmi_dev_remove+0x54/0x68 |
| device_release_driver_internal+0x114/0x1e8 |
| driver_detach+0x58/0xe8 |
| bus_remove_driver+0x88/0xe0 |
| driver_unregister+0x38/0x68 |
| scmi_driver_unregister+0x1c/0x28 |
| scmi_drv_exit+0x1c/0xae0 [dummy_scmi_consumer] |
| __arm64_sys_delete_module+0x1a4/0x268 |
| el0_svc_common.constprop.3+0x94/0x178 |
| do_el0_svc+0x2c/0x98 |
| el0_sync_handler+0x148/0x1a8 |
| el0_sync+0x158/0x180 |
| |
| Link: https://lore.kernel.org/r/20201013133109.49821-1-cristian.marussi@arm.com |
| Fixes: e7c215f358a35 ("firmware: arm_scmi: Add notification callbacks-registration") |
| Signed-off-by: Cristian Marussi <cristian.marussi@arm.com> |
| Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| 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 4731daaacd19e..4d9f6de3a7fae 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.27.0 |
| |