[PATCH v4] hwmon: (pt5161l) Fix bugs in pt5161l_read_block_data()

Pradhan, Sanman posted 1 patch 2 months, 1 week ago
drivers/hwmon/pt5161l.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v4] hwmon: (pt5161l) Fix bugs in pt5161l_read_block_data()
Posted by Pradhan, Sanman 2 months, 1 week ago
From: Sanman Pradhan <psanman@juniper.net>

Fix two bugs in pt5161l_read_block_data():

1. Buffer overrun: The local buffer rbuf is declared as u8 rbuf[24],
   but i2c_smbus_read_block_data() can return up to
   I2C_SMBUS_BLOCK_MAX (32) bytes. The i2c-core copies the data into
   the caller's buffer before the return value can be checked, so
   the post-read length validation does not prevent a stack overrun
   if a device returns more than 24 bytes. Resize the buffer to
   I2C_SMBUS_BLOCK_MAX.

2. Unexpected positive return on length mismatch: When all three
   retries are exhausted because the device returns data with an
   unexpected length, i2c_smbus_read_block_data() returns a positive
   byte count. The function returns this directly, and callers treat
   any non-negative return as success, processing stale or incomplete
   buffer contents. Return -EIO when retries are exhausted with a
   positive return value, preserving the negative error code on I2C
   failure.

Fixes: 1b2ca93cd0592 ("hwmon: Add driver for Astera Labs PT5161L retimer")
Cc: stable@vger.kernel.org
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
v4:
 - Submit as standalone patch, no code changes
v3:
 - No changes
v2:
 - Also fix unexpected positive return when retries are
   exhausted due to length mismatch

 drivers/hwmon/pt5161l.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/pt5161l.c b/drivers/hwmon/pt5161l.c
index 20e3cfa625f1..89d4da8aa4c0 100644
--- a/drivers/hwmon/pt5161l.c
+++ b/drivers/hwmon/pt5161l.c
@@ -121,7 +121,7 @@ static int pt5161l_read_block_data(struct pt5161l_data *data, u32 address,
 	int ret, tries;
 	u8 remain_len = len;
 	u8 curr_len;
-	u8 wbuf[16], rbuf[24];
+	u8 wbuf[16], rbuf[I2C_SMBUS_BLOCK_MAX];
 	u8 cmd = 0x08; /* [7]:pec_en, [4:2]:func, [1]:start, [0]:end */
 	u8 config = 0x00; /* [6]:cfg_type, [4:1]:burst_len, [0]:address bit16 */
 
@@ -151,7 +151,7 @@ static int pt5161l_read_block_data(struct pt5161l_data *data, u32 address,
 				break;
 		}
 		if (tries >= 3)
-			return ret;
+			return ret < 0 ? ret : -EIO;
 
 		memcpy(val, rbuf, curr_len);
 		val += curr_len;
-- 
2.34.1

Re: [PATCH v4] hwmon: (pt5161l) Fix bugs in pt5161l_read_block_data()
Posted by Guenter Roeck 2 months, 1 week ago
On Fri, Apr 10, 2026 at 12:25:55AM +0000, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
> 
> Fix two bugs in pt5161l_read_block_data():
> 
> 1. Buffer overrun: The local buffer rbuf is declared as u8 rbuf[24],
>    but i2c_smbus_read_block_data() can return up to
>    I2C_SMBUS_BLOCK_MAX (32) bytes. The i2c-core copies the data into
>    the caller's buffer before the return value can be checked, so
>    the post-read length validation does not prevent a stack overrun
>    if a device returns more than 24 bytes. Resize the buffer to
>    I2C_SMBUS_BLOCK_MAX.
> 
> 2. Unexpected positive return on length mismatch: When all three
>    retries are exhausted because the device returns data with an
>    unexpected length, i2c_smbus_read_block_data() returns a positive
>    byte count. The function returns this directly, and callers treat
>    any non-negative return as success, processing stale or incomplete
>    buffer contents. Return -EIO when retries are exhausted with a
>    positive return value, preserving the negative error code on I2C
>    failure.
> 
> Fixes: 1b2ca93cd0592 ("hwmon: Add driver for Astera Labs PT5161L retimer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>

Applied.

Thanks,
Guenter