public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] platform/chrome: cros_ec_proto: Lock device when updating MKBP version
@ 2024-07-30 10:44 Patryk Duda
  2024-07-30 12:12 ` Tzung-Bi Shih
  0 siblings, 1 reply; 2+ messages in thread
From: Patryk Duda @ 2024-07-30 10:44 UTC (permalink / raw)
  To: Guenter Roeck, Tzung-Bi Shih, Benson Leung
  Cc: chrome-platform, linux-kernel, chromeos-krk-upstreaming,
	Patryk Duda

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@vger.kernel.org
Signed-off-by: Patryk Duda <patrykd@google.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.

V2 -> V3
- Moved call to mutex_unlock() below call to
  cros_ec_get_host_command_version_mask().

 drivers/platform/chrome/cros_ec_proto.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index f776fd42244f..73f75958e15c 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -813,9 +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);
+		mutex_unlock(&ec_dev->lock);
 		if (ret < 0 || ver_mask == 0)
 			/*
 			 * Do not change the MKBP supported version if we can't
-- 
2.46.0.rc1.232.g9752f9e123-goog


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v3] platform/chrome: cros_ec_proto: Lock device when updating MKBP version
  2024-07-30 10:44 [PATCH v3] platform/chrome: cros_ec_proto: Lock device when updating MKBP version Patryk Duda
@ 2024-07-30 12:12 ` Tzung-Bi Shih
  0 siblings, 0 replies; 2+ messages in thread
From: Tzung-Bi Shih @ 2024-07-30 12:12 UTC (permalink / raw)
  To: Patryk Duda
  Cc: Guenter Roeck, Benson Leung, chrome-platform, linux-kernel,
	chromeos-krk-upstreaming

On Tue, Jul 30, 2024 at 10:44:25AM +0000, Patryk Duda wrote:
> 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.
> 
> [...]

Applied to

    https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git chrome-platform-6.11

Thanks!

[1/1] platform/chrome: cros_ec_proto: Lock device when updating MKBP version
      commit: df615907f1bf907260af01ccb904d0e9304b5278

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-07-30 12:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-30 10:44 [PATCH v3] platform/chrome: cros_ec_proto: Lock device when updating MKBP version Patryk Duda
2024-07-30 12:12 ` Tzung-Bi Shih

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox