[PATCH] Off-by-one error in CP2112 HID driver affecting SMBus block read output

[Luis Yanes <melus0@yahoo.es>]

Affected file: drivers/hid/hid-cp2112.c
Kernel version: v5.15.195 (verified) - master (suspected)
Tool used: i2cget -y 8 0xb 0x78 s <-- reading a battery gauge chip

Observed behavior:
Incorrect trailing last byte in 32 bytes SMBus block read response.

Expected output:
0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xaa
0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x55

Actual SMBus block data read output:
0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xaa
0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x00

I2C block data read output (from i2cget -y 8 0xb 0x78 i):
0x20 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
0xaa 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff

Description:
When performing an SMBus block read using the CP2112 HID USB-to-SMBus
bridge with the i2cget utility, the last byte of the response for a
32 bytes block read is, in this case, always 0x00 instead of the actual
value. In this sample case shown above the actual last byte is 0x55 but
the driver returns 0x00 (uninitialized value).
This happens consistently for any 32 bytes block read on any register.
Shorter than 32 bytes block transfers are unaffected.

Suspected cause:
For what I could understand checking the source code this seems an
off-by-one error in the read request and buffer handling, limited by the
I2C_SMBUS_BLOCK_MAX 32 bytes read value and the response parsing logic
within the CP2112 driver since that with a custom hacked module
(see patch below) I could read that last byte.

Steps to reproduce:
1. Connect a CP2112 device and a proper slave device to read from.
2. Use i2cget to perform a known 32 bytes block read with this device:
   i.e: i2cget -y 8 0xb 0x78 s
3. Compare the output against the expected data from the target
   SMBus device. The last byte probably could appear as any other
   uninitialized buffer value, I guess.
   From 'i2cget -y 8 0xb 0x78 i' the initial 0x20 is the block
   length and the last 32th byte is completely missing.

Impact:
This bug produces incorrect output and will lead to data corruption
or data misinterpretation when reading from SMBus devices using this
driver.

Suggested fix:
Review the buffer handling and read length in the CP2112 driver SMBus
and I2C block read and write implementations.
I have not checked if write operations are affected, but probably would
be a good idea to verify that the 32 bytes block transfers are handled
properly.

Additional info:
The issue is reproducible with different CP2112 devices (silicon rev F04)
and v5.15.x stock kernel builds. Probably also affects newer versions
since for what I could understand, despite the additional checking
changes added to the driver the logic seems basically the same.

Patch: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/plain/drivers/hid/hid-cp2112.c?h=v5.15.195
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -703,7 +703,7 @@
 	case I2C_SMBUS_BLOCK_DATA:
 		if (I2C_SMBUS_READ == read_write) {
 			count = cp2112_write_read_req(buf, addr,
-						      I2C_SMBUS_BLOCK_MAX,
+						      I2C_SMBUS_BLOCK_MAX + 1,
 						      command, NULL, 0);
 		} else {
 			count = cp2112_write_req(buf, addr, command,
@@ -796,7 +796,7 @@
 		memcpy(data->block + 1, buf, read_length);
 		break;
 	case I2C_SMBUS_BLOCK_DATA:
-		if (read_length > I2C_SMBUS_BLOCK_MAX) {
+		if (read_length > I2C_SMBUS_BLOCK_MAX + 1) {
 			ret = -EPROTO;
 			goto power_normal;
 		}

--