uptime_show() asks for two bytes and only rejects a negative return:

	u8 time_val[2];

	ret = i2c_smbus_read_i2c_block_data(mcu->client, REG_UPTIME,
					    sizeof(time_val), time_val);
	if (ret < 0)
		return ret;

	return sprintf(buf, "%d\n",
		       (time_val[0]) | (time_val[1] << 8));

i2c_smbus_read_i2c_block_data() returns the number of bytes the transfer
actually produced, which the device supplies and which can be shorter
than the length asked for:

	memcpy(values, &data.block[1], data.block[0]);
	return data.block[0];

time_val is not initialised, so a reply of one byte leaves the high half
of the reported uptime as whatever was on the stack, and a reply of zero
bytes leaks both halves. Either way the value ends up in sysfs.

Require the full two bytes. The other registers this driver reads go
through i2c_smbus_read_byte_data(), which returns the byte itself, so
the existing negative-only checks are right there.

Fixes: 758b62e562f2 ("hwmon: Add sophgo SG2042 external hardware monitor support")
Cc: stable@vger.kernel.org
Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 drivers/hwmon/sg2042-mcu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hwmon/sg2042-mcu.c b/drivers/hwmon/sg2042-mcu.c
index 591f5f572fe4..10292417e766 100644
--- a/drivers/hwmon/sg2042-mcu.c
+++ b/drivers/hwmon/sg2042-mcu.c
@@ -79,6 +79,8 @@ static ssize_t uptime_show(struct device *dev,
 					    sizeof(time_val), time_val);
 	if (ret < 0)
 		return ret;
+	if (ret != sizeof(time_val))
+		return -EIO;
 
 	return sprintf(buf, "%d\n",
 		       (time_val[0]) | (time_val[1] << 8));

base-commit: 2d2338c93da79b3bfe4b6099a931d9468d539952
-- 
2.55.0

On 8/2/26 06:07, Ali Ahmet Memis wrote:
> uptime_show() asks for two bytes and only rejects a negative return:
> 
> 	u8 time_val[2];
> 
> 	ret = i2c_smbus_read_i2c_block_data(mcu->client, REG_UPTIME,
> 					    sizeof(time_val), time_val);
> 	if (ret < 0)
> 		return ret;
> 
> 	return sprintf(buf, "%d\n",
> 		       (time_val[0]) | (time_val[1] << 8));
> 
> i2c_smbus_read_i2c_block_data() returns the number of bytes the transfer
> actually produced, which the device supplies and which can be shorter
> than the length asked for:
> 
> 	memcpy(values, &data.block[1], data.block[0]);
> 	return data.block[0];
> 
> time_val is not initialised, so a reply of one byte leaves the high half
> of the reported uptime as whatever was on the stack, and a reply of zero
> bytes leaks both halves. Either way the value ends up in sysfs.
> 
> Require the full two bytes. The other registers this driver reads go
> through i2c_smbus_read_byte_data(), which returns the byte itself, so
> the existing negative-only checks are right there.
> 

I ended up asking AI:

Does i2c_smbus_read_i2c_block_data() in ./drivers/i2c/i2c-core-smbus.c ever return less than the requested number of bytes unless there is an error ?

Here is what it tells me:

   No, as long as the requested length is <= I2C_SMBUS_BLOCK_MAX (32 bytes), i2c-core-smbus.c will never return fewer
bytes than requested unless a negative error code is returned.

   The only scenario where it returns a positive byte count less than the caller's requested length is if the caller
passes length > 32, because the function clamps the request to I2C_SMBUS_BLOCK_MAX.

   ### Why this is the case

   1. No Count Byte in I2C Block Read (I2C_SMBUS_I2C_BLOCK_DATA):
   Unlike an SMBus Block Read (smbus-protocol.rst), where the slave device sends a length/count byte as the first byte of
  data transferred, an I2C Block Read does not include a length byte from the target
   device. The master reads a fixed number of bytes determined by the caller.
   2. In i2c-core-smbus.c:
     if (length > I2C_SMBUS_BLOCK_MAX)
             length = I2C_SMBUS_BLOCK_MAX;
     data.block[0] = length;
     status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
                             I2C_SMBUS_READ, command,
                             I2C_SMBUS_I2C_BLOCK_DATA, &data);
     if (status < 0)
             return status;

     memcpy(values, &data.block[1], data.block[0]);
     return data.block[0];
     Before calling i2c_smbus_xfer(), data.block[0] is initialized to the requested length (clamped to 32). Upon success
(status >= 0), it returns data.block[0].
   3. In I2C Emulation (i2c-core-smbus.c):
       • The read message is set up for exactly data->block[0] bytes (msg[1].len = data->block[0];).
       • Unlike SMBus Block Data, it does not set I2C_M_RECV_LEN.
       • When copying the buffer back upon completion (i2c-core-smbus.c), data->block[0] is never modified and remains
exactly equal to the requested length.
   4. In Native Adapter Drivers (adapter->algo->smbus_xfer):
       • None of the bus drivers in drivers/i2c/busses/ implementing I2C_SMBUS_I2C_BLOCK_DATA (i2c-i801, i2c-viapro,
i2c-amd8111, i2c-ismt, i2c-mlxbf, etc.) modify data->block[0] to a smaller value on success.
       • If a target device NAKs before the requested length bytes are read, the controller driver fails the transaction
and returns a negative errno (e.g., -ENXIO, -EIO, or -EPROTO).

   Consequently, any successful call such as i2c_smbus_read_i2c_block_data(client, reg, I2C_SMBUS_BLOCK_MAX, buf) will
always return exactly 32 (I2C_SMBUS_BLOCK_MAX) on success.

Please refrain from sending fixes for non-issues.

Thanks,
Guenter

On Sun, Aug 02 2026, Guenter Roeck wrote:

> Please refrain from sending fixes for non-issues.

You are right. Please drop it.

I checked what i2c_smbus_read_i2c_block_data() returns without checking
which protocol it runs, and the two block protocols do not behave the
same. In i2c_smbus_xfer_emulated(), I2C_SMBUS_BLOCK_DATA sets
I2C_M_RECV_LEN and reads back with

	memcpy(data->block, msg[1].buf, msg[1].buf[0] + 1);

so there the device supplies the count and a short reply is a real
thing. I2C_SMBUS_I2C_BLOCK_DATA, which is what this call uses, sets no
I2C_M_RECV_LEN, sends msg[1].len = data->block[0] and reads back with

	memcpy(data->block + 1, msg[1].buf, data->block[0]);

which never touches data->block[0]. An I2C block read carries no length
byte on the wire, so nothing can shorten it, and a transfer that is cut
short fails with an errno rather than returning a smaller count. time_val
is fully written whenever the call succeeds.

I should have read that before sending, rather than reasoning from the
return value alone. Sorry for the noise, and for the stable Cc on top of
it.

--
Ali