[PATCH] HID: hid-goodix-spi: validate report size to prevent stack buffer overflow

[PATCH] HID: hid-goodix-spi: validate report size to prevent stack buffer overflow

[Tianchu Chen]

From: Tianchu Chen <flynnnchen@tencent.com>

goodix_hid_set_raw_report() builds a protocol frame in a 128-byte stack
buffer (tmp_buf), writing an 11-12 byte header followed by the
caller-supplied report data.  The HID core caps report size at
HID_MAX_BUFFER_SIZE (16384) by default, while the driver does not set
hid_ll_driver.max_buffer_size and performs no bounds checking before
copying the payload:

    memcpy(tmp_buf + tx_len, buf, len);

A hidraw SET_REPORT ioctl with a report larger than ~116 bytes
overflows the stack buffer.

Add a size check after constructing the header, rejecting reports that
would exceed the buffer capacity.

Discovered by Atuin - Automated Vulnerability Discovery Engine.

Fixes: 75e16c8ce283 ("HID: hid-goodix: Add Goodix HID-over-SPI driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
---
 drivers/hid/hid-goodix-spi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-goodix-spi.c b/drivers/hid/hid-goodix-spi.c
index 80c0288a3..288cb827e 100644
--- a/drivers/hid/hid-goodix-spi.c
+++ b/drivers/hid/hid-goodix-spi.c
@@ -520,6 +520,9 @@ static int goodix_hid_set_raw_report(struct hid_device *hid,
 	memcpy(tmp_buf + tx_len, args, args_len);
 	tx_len += args_len;
 
+	if (tx_len + len > sizeof(tmp_buf))
+		return -EINVAL;
+
 	memcpy(tmp_buf + tx_len, buf, len);
 	tx_len += len;
 
-- 
2.51.0

Re: [PATCH] HID: hid-goodix-spi: validate report size to prevent stack buffer overflow

[Dmitry Torokhov]

On Fri, May 29, 2026 at 01:42:47PM +0000, Tianchu Chen wrote:
> From: Tianchu Chen <flynnnchen@tencent.com>
> 
> goodix_hid_set_raw_report() builds a protocol frame in a 128-byte stack
> buffer (tmp_buf), writing an 11-12 byte header followed by the
> caller-supplied report data.  The HID core caps report size at
> HID_MAX_BUFFER_SIZE (16384) by default, while the driver does not set
> hid_ll_driver.max_buffer_size and performs no bounds checking before
> copying the payload:
> 
>     memcpy(tmp_buf + tx_len, buf, len);
> 
> A hidraw SET_REPORT ioctl with a report larger than ~116 bytes
> overflows the stack buffer.
> 
> Add a size check after constructing the header, rejecting reports that
> would exceed the buffer capacity.
> 
> Discovered by Atuin - Automated Vulnerability Discovery Engine.
> 
> Fixes: 75e16c8ce283 ("HID: hid-goodix: Add Goodix HID-over-SPI driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> ---
>  drivers/hid/hid-goodix-spi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hid/hid-goodix-spi.c b/drivers/hid/hid-goodix-spi.c
> index 80c0288a3..288cb827e 100644
> --- a/drivers/hid/hid-goodix-spi.c
> +++ b/drivers/hid/hid-goodix-spi.c
> @@ -520,6 +520,9 @@ static int goodix_hid_set_raw_report(struct hid_device *hid,
>  	memcpy(tmp_buf + tx_len, args, args_len);
>  	tx_len += args_len;
>  
> +	if (tx_len + len > sizeof(tmp_buf))
> +		return -EINVAL;
> +

We can also consider returning -E2BIG here.

>  	memcpy(tmp_buf + tx_len, buf, len);
>  	tx_len += len;
>  

In any case:

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I think we can ignore Sashiko's ramblings on this patch, it needs some
instructions detailing order of operations/timing of callbacks in HID
subsystem.

Thanks.

-- 
Dmitry