[PATCH v2] HID: intel-thc-hid: intel-quickspi: validate report size before copy

HyeongJun An posted 1 patch 1 week ago
drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c  | 9 ++++++++-
drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h  | 1 +
.../hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c | 3 +++
3 files changed, 12 insertions(+), 1 deletion(-)
[PATCH v2] HID: intel-thc-hid: intel-quickspi: validate report size before copy
Posted by HyeongJun An 1 week ago
write_cmd_to_txdma() builds an output report in qsdev->report_buf, a heap
buffer allocated in quickspi_alloc_report_buf() to the device-descriptor
derived max_report_len (a few hundred bytes for a touch controller).  It
copies the caller-supplied report into that buffer:

    memcpy(write_buf->content, report_buf, report_buf_len);

The HID core caps a report at HID_MAX_BUFFER_SIZE (16384) by default, and
quickspi_hid_ll_driver does not set max_buffer_size, so the length reaches
the driver unbounded.  A hidraw SET_REPORT/SET_FEATURE ioctl carrying a
report larger than max_report_len therefore overflows report_buf with
attacker-controlled length and content.

Record the report_buf allocation size and reject reports that do not fit
before copying, matching the equivalent guard in the intel-quicki2c
sibling (quicki2c_init_write_buf()) and the hid-goodix-spi fix.

write_cmd_to_txdma() writes the output report header ahead of the content
in the same buffer, so size the allocation to cover the header as well.
That keeps the added bound from rejecting a maximum-sized report.

Fixes: 9d8d51735a3a ("HID: intel-thc-hid: intel-quickspi: Add HIDSPI protocol implementation")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
---
v2: Size report_buf to cover the output report header as well, so the added
    bound cannot reject a valid maximum-sized report (raised by the Sashiko
    AI review of v1).  No other change.

v1: https://lore.kernel.org/all/20260628133717.941389-1-sammiee5311@gmail.com/

 drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c  | 9 ++++++++-
 drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h  | 1 +
 .../hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c | 3 +++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
index 4ae2e1718b30..da5ecfcd0fbf 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -555,7 +555,14 @@ static int quickspi_alloc_report_buf(struct quickspi_device *qsdev)
 	max_report_len = max(le16_to_cpu(qsdev->dev_desc.max_output_len),
 			     le16_to_cpu(qsdev->dev_desc.max_input_len));
 
-	qsdev->report_buf = devm_kzalloc(qsdev->dev, max_report_len, GFP_KERNEL);
+	/*
+	 * write_cmd_to_txdma() writes the output report header ahead of the
+	 * content in this buffer, so it has to hold both.
+	 */
+	qsdev->report_buf_size = HIDSPI_OUTPUT_REPORT_SIZE(max_report_len);
+
+	qsdev->report_buf = devm_kzalloc(qsdev->dev, qsdev->report_buf_size,
+					 GFP_KERNEL);
 	if (!qsdev->report_buf)
 		return -ENOMEM;
 
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
index bf5e18f5a5f4..0ed964bfe3dd 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
@@ -157,6 +157,7 @@ struct quickspi_device {
 	u8 *report_descriptor;
 	u8 *input_buf;
 	u8 *report_buf;
+	u32 report_buf_size;
 	u32 report_len;
 
 	wait_queue_head_t reset_ack_wq;
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
index cb19057f1191..db6054843e77 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
@@ -30,6 +30,9 @@ static int write_cmd_to_txdma(struct quickspi_device *qsdev,
 
 	write_buf = (struct output_report *)qsdev->report_buf;
 
+	if (HIDSPI_OUTPUT_REPORT_SIZE(report_buf_len) > qsdev->report_buf_size)
+		return -EINVAL;
+
 	write_buf->output_hdr.report_type = report_type;
 	write_buf->output_hdr.content_len = cpu_to_le16(report_buf_len);
 	write_buf->output_hdr.content_id = report_id;
-- 
2.43.0
RE: [PATCH v2] HID: intel-thc-hid: intel-quickspi: validate report size before copy
Posted by Xu, Even 5 days, 2 hours ago

> -----Original Message-----
> From: HyeongJun An <sammiee5311@gmail.com>
> Sent: Friday, July 17, 2026 5:16 PM
> To: Xu, Even <even.xu@intel.com>; Sun, Xinpeng <xinpeng.sun@intel.com>; Jiri
> Kosina <jikos@kernel.org>; Benjamin Tissoires <bentiss@kernel.org>
> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org;
> stable@vger.kernel.org; HyeongJun An <sammiee5311@gmail.com>
> Subject: [PATCH v2] HID: intel-thc-hid: intel-quickspi: validate report size before
> copy
> 
> write_cmd_to_txdma() builds an output report in qsdev->report_buf, a heap
> buffer allocated in quickspi_alloc_report_buf() to the device-descriptor derived
> max_report_len (a few hundred bytes for a touch controller).  It copies the caller-
> supplied report into that buffer:
> 
>     memcpy(write_buf->content, report_buf, report_buf_len);
> 
> The HID core caps a report at HID_MAX_BUFFER_SIZE (16384) by default, and
> quickspi_hid_ll_driver does not set max_buffer_size, so the length reaches the
> driver unbounded.  A hidraw SET_REPORT/SET_FEATURE ioctl carrying a report
> larger than max_report_len therefore overflows report_buf with attacker-
> controlled length and content.
> 
> Record the report_buf allocation size and reject reports that do not fit before
> copying, matching the equivalent guard in the intel-quicki2c sibling
> (quicki2c_init_write_buf()) and the hid-goodix-spi fix.
> 
> write_cmd_to_txdma() writes the output report header ahead of the content in
> the same buffer, so size the allocation to cover the header as well.
> That keeps the added bound from rejecting a maximum-sized report.
> 
> Fixes: 9d8d51735a3a ("HID: intel-thc-hid: intel-quickspi: Add HIDSPI protocol
> implementation")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
> ---
> v2: Size report_buf to cover the output report header as well, so the added
>     bound cannot reject a valid maximum-sized report (raised by the Sashiko
>     AI review of v1).  No other change.
> 
> v1: https://lore.kernel.org/all/20260628133717.941389-1-
> sammiee5311@gmail.com/
> 
>  drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c  | 9 ++++++++-
> drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h  | 1 +  .../hid/intel-thc-
> hid/intel-quickspi/quickspi-protocol.c | 3 +++
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> index 4ae2e1718b30..da5ecfcd0fbf 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -555,7 +555,14 @@ static int quickspi_alloc_report_buf(struct
> quickspi_device *qsdev)
>  	max_report_len = max(le16_to_cpu(qsdev->dev_desc.max_output_len),
>  			     le16_to_cpu(qsdev->dev_desc.max_input_len));
> 
> -	qsdev->report_buf = devm_kzalloc(qsdev->dev, max_report_len,
> GFP_KERNEL);
> +	/*
> +	 * write_cmd_to_txdma() writes the output report header ahead of the
> +	 * content in this buffer, so it has to hold both.
> +	 */
> +	qsdev->report_buf_size =
> HIDSPI_OUTPUT_REPORT_SIZE(max_report_len);
> +
> +	qsdev->report_buf = devm_kzalloc(qsdev->dev, qsdev->report_buf_size,
> +					 GFP_KERNEL);
>  	if (!qsdev->report_buf)
>  		return -ENOMEM;
> 
> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
> b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
> index bf5e18f5a5f4..0ed964bfe3dd 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
> @@ -157,6 +157,7 @@ struct quickspi_device {
>  	u8 *report_descriptor;
>  	u8 *input_buf;
>  	u8 *report_buf;
> +	u32 report_buf_size;
>  	u32 report_len;
> 
>  	wait_queue_head_t reset_ack_wq;
> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> index cb19057f1191..db6054843e77 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> @@ -30,6 +30,9 @@ static int write_cmd_to_txdma(struct quickspi_device
> *qsdev,
> 
>  	write_buf = (struct output_report *)qsdev->report_buf;
> 
> +	if (HIDSPI_OUTPUT_REPORT_SIZE(report_buf_len) > qsdev-
> >report_buf_size)
> +		return -EINVAL;
> +

Thanks for the patch!
Reviewed-by: Even Xu <even.xu@intel.com>

>  	write_buf->output_hdr.report_type = report_type;
>  	write_buf->output_hdr.content_len = cpu_to_le16(report_buf_len);
>  	write_buf->output_hdr.content_id = report_id;
> --
> 2.43.0