linux-kernel - Re: [PATCH] HID: i2c-hid: fix potential buffer overflow in i2c_hid_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <or6uoheepcbcx4sm36g7z3ccevhjn6iv2ddztx6mpw4qgbdoyo@n3yfbkwlye5i>
Date: Wed, 7 Jan 2026 15:20:13 +0100
From: Benjamin Tissoires <bentiss@...nel.org>
To: Kwok Kin Ming <kenkinming2002@...il.com>
Cc: jikos@...nel.org, linux-input@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] HID: i2c-hid: fix potential buffer overflow in
 i2c_hid_get_report()

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@...il.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
>