linux-kernel - [PATCH 5.9 019/391] firmware: arm_scmi: Fix locking in notifications

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20201103203349.219858293@linuxfoundation.org>
Date:   Tue,  3 Nov 2020 21:31:10 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org,
        Cristian Marussi <cristian.marussi@....com>,
        Sudeep Holla <sudeep.holla@....com>,
        Sasha Levin <sashal@...nel.org>
Subject: [PATCH 5.9 019/391] firmware: arm_scmi: Fix locking in notifications

From: Cristian Marussi <cristian.marussi@....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@....com>
Signed-off-by: Sudeep Holla <sudeep.holla@....com>
Signed-off-by: Sasha Levin <sashal@...nel.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