[PATCH] HID: i2c-hid: fix potential buffer overflow in i2c_hid_get_report()

[PATCH] HID: i2c-hid: fix potential buffer overflow in i2c_hid_get_report()

[Kwok Kin Ming]

`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

Re: [PATCH] HID: i2c-hid: fix potential buffer overflow in i2c_hid_get_report()

[Benjamin Tissoires]

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
> 

Re: [PATCH] HID: i2c-hid: fix potential buffer overflow in i2c_hid_get_report()

[kenkinming2002]

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

Re: [PATCH] HID: i2c-hid: fix potential buffer overflow in i2c_hid_get_report()

[Benjamin Tissoires]

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

Re: [PATCH] HID: i2c-hid: fix potential buffer overflow in i2c_hid_get_report()

[Benjamin Tissoires]

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>