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>] [day] [month] [year] [list]
Message-ID: <20240729133404.451210-1-patrykd@google.com>
Date: Mon, 29 Jul 2024 13:34:04 +0000
From: Patryk Duda <patrykd@...gle.com>
To: Guenter Roeck <groeck@...omium.org>, Tzung-Bi Shih <tzungbi@...nel.org>, 
	Benson Leung <bleung@...omium.org>
Cc: chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	chromeos-krk-upstreaming@...gle.com, Patryk Duda <patrykd@...gle.com>
Subject: [PATCH v2] platform/chrome: cros_ec_proto: Lock device when updating
 MKBP version

The cros_ec_get_host_command_version_mask() function requires that the
caller must have ec_dev->lock mutex before calling it. This requirement
was not met and as a result it was possible that two commands were sent
to the device at the same time.

The problem was observed while using UART backend which doesn't use any
additional locks, unlike SPI backend which locks the controller until
response is received.

Fixes: f74c7557ed0d ("platform/chrome: cros_ec_proto: Update version on GET_NEXT_EVENT failure")
Cc: stable@...r.kernel.org
Signed-off-by: Patryk Duda <patrykd@...gle.com>
---
The f74c7557ed0d patch was tested with Fingerprint MCU (FPMCU) that uses
SPI to communicate with chipset. At that time, UART FPMCU had the same
version of GET_NEXT_EVENT command in RO and RW, so the MKBP version was
never updated in this case.

Version 3 of GET_NEXT_EVENT command was recently added to CrosEC. As a
result MKBP version is also updated when UART FPMCU is used which
exposed this problem.

Best regards,
Patryk

V1 -> V2
- Added missing mutex_unlock() on
  cros_ec_get_host_command_version_mask() failure

 drivers/platform/chrome/cros_ec_proto.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index f776fd42244f..f278fde15504 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -813,10 +813,11 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev,
 	if (ret == -ENOPROTOOPT) {
 		dev_dbg(ec_dev->dev,
 			"GET_NEXT_EVENT returned invalid version error.\n");
+		mutex_lock(&ec_dev->lock);
 		ret = cros_ec_get_host_command_version_mask(ec_dev,
 							EC_CMD_GET_NEXT_EVENT,
 							&ver_mask);
-		if (ret < 0 || ver_mask == 0)
+		if (ret < 0 || ver_mask == 0) {
 			/*
 			 * Do not change the MKBP supported version if we can't
 			 * obtain supported version correctly. Please note that
@@ -824,11 +825,14 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev,
 			 * EC_RES_INVALID_VERSION which means that the command
 			 * is present.
 			 */
+			mutex_unlock(&ec_dev->lock);
 			return -ENOPROTOOPT;
+		}
 
 		ec_dev->mkbp_event_supported = fls(ver_mask);
 		dev_dbg(ec_dev->dev, "MKBP support version changed to %u\n",
 			ec_dev->mkbp_event_supported - 1);
+		mutex_unlock(&ec_dev->lock);
 
 		/* Try to get next event with new MKBP support version set. */
 		ret = get_next_event(ec_dev);
-- 
2.46.0.rc1.232.g9752f9e123-goog


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ