Re: [PATCH 1/2] HID: Intel-thc-hid: Intel-quicki2c: Fix buffer overflow
From: sashiko-bot@kernel.org
Date: 2026-09-22 03:03:14
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] hid: intel-quicki2c: incomplete buffer fix and stale report_len causes -EINVAL for large payloads -- commit 3ae5fe57bf2559497c69a1a1f4feb9ea45f51690 Author: Even Xu [off-list ref] HID: Intel-thc-hid: Intel-quicki2c: Fix buffer overflow The patch modifies buffer allocations in the intel-quicki2c driver to account for packet headers in addition to the payload length. This is intended to prevent buffer overflows when the payload reaches its maximum size.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c index 0d2ad7bc3648a..2537288b50261 100644 --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c@@ -606,7 +606,8 @@ static int quicki2c_alloc_report_buf(struct quicki2c_device *qcdev) */ max_report_len = max(le16_to_cpu(qcdev->dev_desc.max_input_len), SZ_4K); - qcdev->input_buf = devm_kzalloc(qcdev->dev, max_report_len, GFP_KERNEL); + qcdev->input_buf = devm_kzalloc(qcdev->dev, + HIDI2C_PACKET_LEN(max_report_len), GFP_KERNEL); if (!qcdev->input_buf) return -ENOMEM;@@ -616,7 +617,8 @@ static int quicki2c_alloc_report_buf(struct quicki2c_device *qcdev) max_report_len = max(le16_to_cpu(qcdev->dev_desc.max_output_len), max_report_len); - qcdev->report_buf = devm_kzalloc(qcdev->dev, max_report_len, GFP_KERNEL); + qcdev->report_buf = devm_kzalloc(qcdev->dev, + HIDI2C_PACKET_LEN(max_report_len), GFP_KERNEL);
[Severity: Medium]
Does HIDI2C_PACKET_LEN() add enough space for the headers? It appears to only
add 2 bytes, but output operations and SET_REPORT might require up to 9 bytes
of command headers (e.g., cmd_reg, cmd, data_reg, length).
Also, qcdev->report_len is not updated to match the new size, remaining as
max_report_len just below this allocation.
Will this cause maximum-sized payloads to be incorrectly rejected with -EINVAL?
In quicki2c_init_write_buf(), there is a bounds check:
if (buf_len > write_buf_len)
return -EINVAL;
This evaluates the total packet length (including headers) against the
unadjusted report_len (passed as write_buf_len), causing valid maximum-sized
payloads to be rejected.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260922024638.932865-1-even.xu@intel.com?part=1