[PATCH] i2c: i801: Clamp adapter timeout to prevent system lockup

Mingyu Wang posted 1 patch 1 day, 2 hours ago
There is a newer version of this series
drivers/i2c/busses/i2c-i801.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
[PATCH] i2c: i801: Clamp adapter timeout to prevent system lockup
Posted by Mingyu Wang 1 day, 2 hours ago
Userspace applications with access to /dev/i2c-* can use the I2C_TIMEOUT
ioctl to manipulate the adap->timeout value. In the i2c-i801 driver,
this user-controlled timeout is used directly in both the interrupt
waiting paths (wait_for_completion_timeout) and the polling loops
(time_after(jiffies, timeout)).

If a malicious or arbitrarily large timeout value is injected from
userspace, and the hardware (or an emulated device in a fuzzing/VM
environment) fails to respond, the driver will block for the entirety
of that requested timeout while holding the i2c adapter's rt_mutex.

In IRQ mode, the task sleeps in wait_for_completion_timeout() in the
TASK_UNINTERRUPTIBLE (D) state for the entire duration, directly triggering
the khungtaskd watchdog if the timeout exceeds 120 seconds. In polling
mode, the endless usleep_range() loop keeps the rt_mutex locked permanently.
Consequently, any other processes attempting to acquire the i2c adapter
lock will be starved in TASK_UNINTERRUPTIBLE, which inevitably triggers
Hung Task panics as well.

Fix this by introducing i801_get_timeout(), which clamps the timeout
to a sensible maximum (HZ, i.e., 1 second). A 1-second timeout is more
than sufficient for typical SMBus block transactions while strictly
bounding the maximum blocking time under hardware failure or excessive
userspace injection.

Fixes: 1de93d5d5217 ("i2c: i801: Replace waitqueue with completion API")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
 drivers/i2c/busses/i2c-i801.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index b29c99ed3883..f60ecf1071ff 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -345,10 +345,19 @@ MODULE_PARM_DESC(disable_features, "Disable selected driver features:\n"
 	"\t\t  0x10  don't use interrupts\n"
 	"\t\t  0x20  disable SMBus Host Notify ");
 
+/*
+ * Bound the maximum wait time to prevent system lockup if the
+ * userspace passes an arbitrarily large timeout via ioctl.
+ */
+static inline unsigned long i801_get_timeout(struct i801_priv *priv)
+{
+	return min_t(unsigned long, priv->adapter.timeout, HZ);
+}
+
 /* Wait for BUSY being cleared and either INTR or an error flag being set */
 static int i801_wait_intr(struct i801_priv *priv)
 {
-	unsigned long timeout = jiffies + priv->adapter.timeout;
+	unsigned long timeout = jiffies + i801_get_timeout(priv);
 	int status, busy;
 
 	do {
@@ -366,7 +375,7 @@ static int i801_wait_intr(struct i801_priv *priv)
 /* Wait for either BYTE_DONE or an error flag being set */
 static int i801_wait_byte_done(struct i801_priv *priv)
 {
-	unsigned long timeout = jiffies + priv->adapter.timeout;
+	unsigned long timeout = jiffies + i801_get_timeout(priv);
 	int status;
 
 	do {
@@ -497,13 +506,12 @@ static int i801_check_post(struct i801_priv *priv, int status)
 static int i801_transaction(struct i801_priv *priv, int xact)
 {
 	unsigned long result;
-	const struct i2c_adapter *adap = &priv->adapter;
 
 	if (priv->features & FEATURE_IRQ) {
 		reinit_completion(&priv->done);
 		iowrite8(xact | SMBHSTCNT_INTREN | SMBHSTCNT_START,
 		       SMBHSTCNT(priv));
-		result = wait_for_completion_timeout(&priv->done, adap->timeout);
+		result = wait_for_completion_timeout(&priv->done, i801_get_timeout(priv));
 		return result ? priv->status : -ETIMEDOUT;
 	}
 
@@ -677,7 +685,6 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 	int smbcmd;
 	int status;
 	unsigned long result;
-	const struct i2c_adapter *adap = &priv->adapter;
 
 	if (command == I2C_SMBUS_BLOCK_PROC_CALL)
 		return -EOPNOTSUPP;
@@ -706,7 +713,7 @@ static int i801_block_transaction_byte_by_byte(struct i801_priv *priv,
 
 		reinit_completion(&priv->done);
 		iowrite8(priv->cmd | SMBHSTCNT_START, SMBHSTCNT(priv));
-		result = wait_for_completion_timeout(&priv->done, adap->timeout);
+		result = wait_for_completion_timeout(&priv->done, i801_get_timeout(priv));
 		return result ? priv->status : -ETIMEDOUT;
 	}
 
-- 
2.34.1