* [PATCH v2] HID: Off-by-one error in CP2112 HID driver affecting SMBus block read output
2025-10-21 17:18 ` [PATCH] Off-by-one error in CP2112 HID driver affecting SMBus block read output Luis Yanes
@ 2025-10-29 17:08 ` Luis Yanes
2026-01-10 10:09 ` [PATCH] " Jiri Kosina
1 sibling, 0 replies; 3+ messages in thread
From: Luis Yanes @ 2025-10-29 17:08 UTC (permalink / raw)
To: linux-input
Just following up on this bug report and proposed fix. I would appreciate feedback on
whether this approach is acceptable or if there is a preferred way to handle this quirk.
After thoroughly reviewing the driver code and rereading AN495 I still don't know why
these chips (tried 5 of them so far) are showing the previously stated observed behaviour,
because the driver logic seems compliant with the AN495 CP2112 interface specification.
But performing an SMBus block read using the CP2112 HID USB-to-SMBus bridge with these
chips the last byte of a 32-byte response is consistently incorrect unless an extra byte
was requested.
A flag could be added at driver load time to enable a quirk fix for affected chips but
the end user first would need to notice or know that their adapter is affected.
I have not been able to figure out a way to detect those affected chips that will produce
corrupted output without notice or warning other than directly checking the serial number.
So have written a quirks patch against v5.15.195 that applies to a given serial number
range or device version and can be enabled for those known to be affected with minimal
impact on the rest.
Alternatively I think that at least some sort of warning notice always should be included.
Signed-off-by: Luis Yanes <melus0@yahoo.es>
--- a/drivers/hid-cp2112.c
+++ b/drivers/hid-cp2112.c
@@ -26,6 +26,14 @@
#include <linux/nls.h>
#include <linux/usb/ch9.h>
#include "hid-ids.h"
+#include <linux/usb.h>
+
+#ifndef CP2112_QUIRKS
+#define CP2112_SN_RANGE_QUIRKS_START "00670000"
+#define CP2112_SN_RANGE_QUIRKS_END "00670FFF"
+#define CP2112_DEVICE_VERSION_QUIRK 2
+#define CP2112_QUIRK_SMBUS_BLOCK33 BIT(0)
+#endif
#define CP2112_REPORT_MAX_LENGTH 64
#define CP2112_GPIO_CONFIG_LENGTH 5
@@ -170,6 +178,7 @@
struct delayed_work gpio_poll_worker;
unsigned long irq_mask;
u8 gpio_prev_state;
+ unsigned long quirks;
};
static int gpio_push_pull = 0xFF;
@@ -642,6 +651,7 @@
__le16 word;
ssize_t count;
size_t read_length = 0;
+ size_t max_length;
unsigned int retries;
int ret;
@@ -702,8 +712,14 @@
break;
case I2C_SMBUS_BLOCK_DATA:
if (I2C_SMBUS_READ == read_write) {
+ read_length = I2C_SMBUS_BLOCK_MAX ;
+ /* Apply quirk: allow one extra byte for CP2112 block read */
+ if (dev->quirks & CP2112_QUIRK_SMBUS_BLOCK33) {
+ read_length += 1;
+ hid_info(hdev, "Quirk: using block read size %lu\n", read_length);
+ }
count = cp2112_write_read_req(buf, addr,
- I2C_SMBUS_BLOCK_MAX,
+ read_length,
command, NULL, 0);
} else {
count = cp2112_write_req(buf, addr, command,
@@ -796,7 +812,14 @@
memcpy(data->block + 1, buf, read_length);
break;
case I2C_SMBUS_BLOCK_DATA:
- if (read_length > I2C_SMBUS_BLOCK_MAX) {
+ max_length = I2C_SMBUS_BLOCK_MAX;
+
+ if (dev->quirks & CP2112_QUIRK_SMBUS_BLOCK33) {
+ max_length += 1;
+ hid_info(hdev, "Quirk: allowing block read up to %lu bytes\n", max_length);
+ }
+
+ if (read_length > max_length) {
ret = -EPROTO;
goto power_normal;
}
@@ -1235,6 +1258,9 @@
struct gpio_irq_chip *girq;
int ret;
+ struct usb_device *udev;
+ char serial[64] = {0};
+
dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
if (!dev)
return -ENOMEM;
@@ -1282,6 +1308,26 @@
hid_info(hdev, "Part Number: 0x%02X Device Version: 0x%02X\n",
buf[1], buf[2]);
+ if (buf[2] == CP2112_DEVICE_VERSION_QUIRK) {
+ dev->quirks |= CP2112_QUIRK_SMBUS_BLOCK33;
+ hid_info(hdev, "Enabled SMBus block+1 quirk for device version 0x%02X\n", buf[2]);
+ }
+
+ udev = interface_to_usbdev(to_usb_interface(hdev->dev.parent));
+
+ if (udev->descriptor.iSerialNumber) {
+ ret = usb_string(udev, udev->descriptor.iSerialNumber, serial, sizeof(serial));
+ if (ret > 0)
+ hid_info(hdev, "CP2112 device serial number: %s\n", serial);
+ }
+
+ if (strlen(serial) == 8 &&
+ strncmp(serial, CP2112_SN_RANGE_QUIRKS_START, 8) >= 0 &&
+ strncmp(serial, CP2112_SN_RANGE_QUIRKS_END, 8) <= 0) {
+ dev->quirks |= CP2112_QUIRK_SMBUS_BLOCK33;
+ hid_info(hdev, "Enabled SMBus block+1 quirk for serial %s\n", serial);
+ }
+
ret = cp2112_hid_get(hdev, CP2112_SMBUS_CONFIG, (u8 *)&config,
sizeof(config), HID_FEATURE_REPORT);
if (ret != sizeof(config)) {
--
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Off-by-one error in CP2112 HID driver affecting SMBus block read output
2025-10-21 17:18 ` [PATCH] Off-by-one error in CP2112 HID driver affecting SMBus block read output Luis Yanes
2025-10-29 17:08 ` [PATCH v2] HID: " Luis Yanes
@ 2026-01-10 10:09 ` Jiri Kosina
1 sibling, 0 replies; 3+ messages in thread
From: Jiri Kosina @ 2026-01-10 10:09 UTC (permalink / raw)
To: Luis Yanes; +Cc: linux-input, Benjamin Tissoires
On Tue, 21 Oct 2025, Luis Yanes wrote:
> 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;
> }
Your explanation and the fix look good to me. Could you please do a proper
submission (with proper shortlog+changelog, SOB line, etc?)?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 3+ messages in thread