drivers/hid/i2c-hid/i2c-hid-core.c | 1 + 1 file changed, 1 insertion(+)
`i2c_hid_xfer` is used to read `recv_len + sizeof(__le16)` bytes of data
into `ihid->rawbuf`.
The former can come from the userspace in the hidraw driver and is only
bounded by HID_MAX_BUFFER_SIZE(16384) by default (unless we also set
`max_buffer_size` field of `struct hid_ll_driver` which we do not).
The latter has size determined at runtime by the maximum size of
different report types you could receive on any particular device and
can be a much smaller value.
Fix this by truncating `recv_len` to `ihid->bufsize - sizeof(__le16)`.
The impact is low since access to hidraw devices requires root.
Signed-off-by: Kwok Kin Ming <kenkinming2002@gmail.com>
---
drivers/hid/i2c-hid/i2c-hid-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 63f46a2e5..5a183af3d 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -286,6 +286,7 @@ static int i2c_hid_get_report(struct i2c_hid *ihid,
* In addition to report data device will supply data length
* in the first 2 bytes of the response, so adjust .
*/
+ recv_len = min(recv_len, ihid->bufsize - sizeof(__le16));
error = i2c_hid_xfer(ihid, ihid->cmdbuf, length,
ihid->rawbuf, recv_len + sizeof(__le16));
if (error) {
--
2.52.0
On Thu, 01 Jan 2026 02:18:26 +0800, Kwok Kin Ming wrote:
> `i2c_hid_xfer` is used to read `recv_len + sizeof(__le16)` bytes of data
> into `ihid->rawbuf`.
>
> The former can come from the userspace in the hidraw driver and is only
> bounded by HID_MAX_BUFFER_SIZE(16384) by default (unless we also set
> `max_buffer_size` field of `struct hid_ll_driver` which we do not).
>
> [...]
Applied to hid/hid.git (for-6.19/upstream-fixes), thanks!
[1/1] HID: i2c-hid: fix potential buffer overflow in i2c_hid_get_report()
https://git.kernel.org/hid/hid/c/2497ff38c530
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
On Jan 01 2026, Kwok Kin Ming wrote:
> `i2c_hid_xfer` is used to read `recv_len + sizeof(__le16)` bytes of data
> into `ihid->rawbuf`.
>
> The former can come from the userspace in the hidraw driver and is only
> bounded by HID_MAX_BUFFER_SIZE(16384) by default (unless we also set
> `max_buffer_size` field of `struct hid_ll_driver` which we do not).
>
> The latter has size determined at runtime by the maximum size of
> different report types you could receive on any particular device and
> can be a much smaller value.
>
> Fix this by truncating `recv_len` to `ihid->bufsize - sizeof(__le16)`.
>
> The impact is low since access to hidraw devices requires root.
>
> Signed-off-by: Kwok Kin Ming <kenkinming2002@gmail.com>
> ---
> drivers/hid/i2c-hid/i2c-hid-core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 63f46a2e5..5a183af3d 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -286,6 +286,7 @@ static int i2c_hid_get_report(struct i2c_hid *ihid,
> * In addition to report data device will supply data length
> * in the first 2 bytes of the response, so adjust .
> */
> + recv_len = min(recv_len, ihid->bufsize - sizeof(__le16));
It makes sense to put this min call here, but it's already present at
line 304 a few lines after. Could you remove that second check (and
unnecessary one after your change).
Cheers,
Benjamin
> error = i2c_hid_xfer(ihid, ihid->cmdbuf, length,
> ihid->rawbuf, recv_len + sizeof(__le16));
> if (error) {
> --
> 2.52.0
>
On Wed, Jan 07, 2026 at 03:20:13PM +0100, Benjamin Tissoires wrote: > It makes sense to put this min call here, but it's already present at > line 304 a few lines after. Could you remove that second check (and > unnecessary one after your change). The min call at line 304 uses ret_count which comes from the first 2 bytes in the device response and indicates the actual size of the returned report descriptor. Notice that importantly ret_count can be strictly smaller than ihid->bufsize because persumably not all reports have the same size. The behavior will change if the caller provides a larger buffer than is necessary. With the min call at line 304, we will return the actual size of the report descriptor (without the 2 bytes length header). Without the min call at 304, we will instead return the size of supplied buffer. Yours sincerely, Ken Kwok
On Jan 08 2026, kenkinming2002@gmail.com wrote: > On Wed, Jan 07, 2026 at 03:20:13PM +0100, Benjamin Tissoires wrote: > > It makes sense to put this min call here, but it's already present at > > line 304 a few lines after. Could you remove that second check (and > > unnecessary one after your change). > > The min call at line 304 uses ret_count which comes from the first 2 > bytes in the device response and indicates the actual size of the > returned report descriptor. Notice that importantly ret_count can be > strictly smaller than ihid->bufsize because persumably not all reports > have the same size. The behavior will change if the caller provides a > larger buffer than is necessary. With the min call at line 304, we will > return the actual size of the report descriptor (without the 2 bytes > length header). Without the min call at 304, we will instead return the > size of supplied buffer. Oh, you're correct. Thanks. Applied (as mentioned by the automated reply). Cheers, Benjamin > > Yours sincerely, > Ken Kwok
© 2016 - 2026 Red Hat, Inc.