linux-kernel - [PATCH] HID: mcp2221: free I2C bus after failed transactions

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: <0302dd6a-3f04-c340-5b6d-5003c6d08026@vigem.de>
Date: Fri, 9 Jan 2026 08:00:14 +0100 (CET)
From: Sven Zühlsdorf <sven.zuehlsdorf@...em.de>
To: Rishi Gupta <gupt21@...il.com>, Jiri Kosina <jikos@...nel.org>, 
    Benjamin Tissoires <bentiss@...nel.org>, linux-i2c@...r.kernel.org, 
    linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] HID: mcp2221: free I2C bus after failed transactions

The conversion of unnecessary speed reconfigurations to actual error cleanup
has missed the timeout case in mcp_i2c_smbus_read. While we give up on
waiting for the requested data, the chip is still executing the transaction.
Attempts to initiate further transactions will be rejected with
"I2C engine busy" (translated to -EAGAIN by the driver). This can be
reproduced by attempting a read from a nonexistent device, as e.g. i2cdetect
does by default for the 0x30-0x3f and 0x50-0x5f address ranges.
Explicitly canceling the current transaction fixes this.

In addition, when initiating an I2C transaction and the chip accepts the
command, i.e. doesn't return "I2C engine busy"/-EAGAIN, we still need to
explicitly free the bus, if we detect an error condition in the HID
report's I2C engine state.

Fixes: 02a46753601a ("HID: mcp2221: Don't set bus speed on every transfer")
Signed-off-by: Sven Zühlsdorf <sven.zuehlsdorf@...em.de>
---
  drivers/hid/hid-mcp2221.c | 19 ++++++++++++++++---
  1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index 33603b019f975..50237b31c781c 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -274,8 +274,11 @@ static int mcp_i2c_write(struct mcp2221 *mcp,
  		memcpy(&mcp->txbuf[4], &msg->buf[idx], len);

  		ret = mcp_send_data_req_status(mcp, mcp->txbuf, len + 4);
-		if (ret)
+		if (ret) {
+			if (ret != -EAGAIN)
+				mcp_cancel_last_cmd(mcp);
  			return ret;
+		}

  		usleep_range(980, 1000);

@@ -332,8 +335,11 @@ static int mcp_i2c_smbus_read(struct mcp2221 *mcp,
  	}

  	ret = mcp_send_data_req_status(mcp, mcp->txbuf, 4);
-	if (ret)
+	if (ret) {
+		if (ret != -EAGAIN)
+			mcp_cancel_last_cmd(mcp);
  		return ret;
+	}

  	mcp->rxbuf_idx = 0;

@@ -353,6 +359,10 @@ static int mcp_i2c_smbus_read(struct mcp2221 *mcp,
  				usleep_range(90, 100);
  				retries++;
  			} else {
+				/* Too many retries.
+				 * Give up and free the bus.
+				 */
+				mcp_cancel_last_cmd(mcp);
  				return ret;
  			}
  		} else {
@@ -454,8 +464,11 @@ static int mcp_smbus_write(struct mcp2221 *mcp, u16 addr,
  	}

  	ret = mcp_send_data_req_status(mcp, mcp->txbuf, data_len);
-	if (ret)
+	if (ret) {
+		if (ret != -EAGAIN)
+			mcp_cancel_last_cmd(mcp);
  		return ret;
+	}

  	if (last_status) {
  		usleep_range(980, 1000);
-- 
2.52.0