[PATCH v2] HID: intel-thc-hid: intel-quickspi: validate GET REPORT response size

[PATCH v2] HID: intel-thc-hid: intel-quickspi: validate GET REPORT response size

[Even Xu]

From: HyeongJun An <sammiee5311@gmail.com>

quickspi_handle_input_data() copies GET_FEATURE and
GET_INPUT_REPORT responses into qsdev->report_buf. The response length
is derived from data supplied by the controller, but was not checked
against the destination buffer size.

Adjust qsdev->report_buf_size to store the data length of report buffer,
reject responses that exceed it before copying. This prevents an
oversized response from overflowing report_buf. The rejected response
does not complete the GET REPORT request, so quickspi_get_report()
follows its existing timeout path.

Fixes: 9d8d51735a3a ("HID: intel-thc-hid: intel-quickspi: Add HIDSPI protocol implementation")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
Signed-off-by: Even Xu <even.xu@intel.com>
Tested-by: Rui Zhang <rui1.zhang@intel.com>
---
Change log:
V2:
 - Rebased on top of latest mainline kernel
 - Adjust qsdev->report_buf_size to store the data length of report buffer

---
 .../intel-thc-hid/intel-quickspi/pci-quickspi.c  | 16 ++++++++--------
 .../intel-thc-hid/intel-quickspi/quickspi-dev.h  |  1 +
 .../intel-quickspi/quickspi-protocol.c           | 12 ++++++++++--
 3 files changed, 19 insertions(+), 10 deletions(-)

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 53023e30cf8d..d0aa66a8a4b4 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -562,17 +562,17 @@ 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));
 
-	/*
-	 * 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);
+	qsdev->report_buf = devm_kzalloc(qsdev->dev,
+					 HIDSPI_OUTPUT_REPORT_SIZE(max_report_len), GFP_KERNEL);
 	if (!qsdev->report_buf)
 		return -ENOMEM;
 
+	/*
+	 * Store the max data length of this report buffer, then compare it with output/input
+	 * packet content when write or read into this report buffer to avoid buffer overflow.
+	 */
+	qsdev->report_buf_size = max_report_len;
+
 	return 0;
 }
 
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 fe51ff51d8c0..4362ac568bda 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
@@ -115,6 +115,7 @@ struct acpi_device;
  * @input_buf: store a copy of latest input report data
  * @input_len: the data size of the input report buffer
  * @report_buf: store a copy of latest input/output report packet from set/get feature
+ * @report_buf_size: the data size of input/output buffer
  * @report_len: the length of input/output report packet
  * @reset_ack_wq: workqueue for waiting reset response from device
  * @reset_ack: indicate reset response received or not
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 0df33c44fbff..5de25c36a1e4 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
@@ -30,7 +30,7 @@ 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)
+	if (report_buf_len > qsdev->report_buf_size)
 		return -EINVAL;
 
 	write_buf->output_hdr.report_type = report_type;
@@ -161,6 +161,7 @@ void quickspi_handle_input_data(struct quickspi_device *qsdev, u32 buf_len)
 	struct input_report_body *input_body;
 	u8 *input_report;
 	u32 input_len;
+	u32 report_len;
 	int ret = 0;
 
 	input_body = (struct input_report_body *)qsdev->input_buf;
@@ -210,7 +211,14 @@ void quickspi_handle_input_data(struct quickspi_device *qsdev, u32 buf_len)
 
 	case GET_FEATURE_RESPONSE:
 	case GET_INPUT_REPORT_RESPONSE:
-		qsdev->report_len = sizeof(body_hdr->content_id) + input_len;
+		report_len = sizeof(body_hdr->content_id) + input_len;
+		if (report_len > qsdev->report_buf_size) {
+			dev_err_once(qsdev->dev, "Get report response too big: %u\n",
+				     report_len);
+			return;
+		}
+
+		qsdev->report_len = report_len;
 		input_report = input_body->content - sizeof(body_hdr->content_id);
 
 		memcpy(qsdev->report_buf, input_report, qsdev->report_len);
-- 
2.43.0

Re: [PATCH v2] HID: intel-thc-hid: intel-quickspi: validate GET REPORT response size

[HyeongJun An]

Hello Even,

Thank you for picking this up, and thanks to Rui for testing it.

On Tue, Oct 27, 2026 at 10:58:20AM +0800, Even Xu wrote:
> -		qsdev->report_len = sizeof(body_hdr->content_id) + input_len;
> +		report_len = sizeof(body_hdr->content_id) + input_len;
> +		if (report_len > qsdev->report_buf_size) {

One question about this bound. The DATA case accepts a content_len up
to max_input_len, and the GET response adds the content ID on top of
that. A GET_FEATURE or GET_INPUT_REPORT response of the same size then
gives report_len = max_input_len + 1, which this version rejects when
max_input_len is the larger of the two lengths. v1 compared against the
allocation, which has the output report header to spare.

Can a response that large happen? If it can, comparing input_len would
keep the same limit as the DATA case:

	if (input_len > qsdev->report_buf_size) {

Best regards,
HyeongJun

RE: [PATCH v2] HID: intel-thc-hid: intel-quickspi: validate GET REPORT response size

[Xu, Even]

> -----Original Message-----
> From: HyeongJun An <sammiee5311@gmail.com>
> Sent: Tuesday, October 27, 2026 5:41 PM
> To: Xu, Even <even.xu@intel.com>
> Cc: jikos@kernel.org; bentiss@kernel.org; srinivas.pandruvada@linux.intel.com;
> Zhang, Rui1 <rui1.zhang@intel.com>; linux-input@vger.kernel.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH v2] HID: intel-thc-hid: intel-quickspi: validate GET REPORT
> response size
> 
> Hello Even,
> 
> Thank you for picking this up, and thanks to Rui for testing it.
> 
> On Tue, Oct 27, 2026 at 10:58:20AM +0800, Even Xu wrote:
> > -		qsdev->report_len = sizeof(body_hdr->content_id) + input_len;
> > +		report_len = sizeof(body_hdr->content_id) + input_len;
> > +		if (report_len > qsdev->report_buf_size) {
> 
> One question about this bound. The DATA case accepts a content_len up to
> max_input_len, and the GET response adds the content ID on top of that. A
> GET_FEATURE or GET_INPUT_REPORT response of the same size then gives
> report_len = max_input_len + 1, which this version rejects when max_input_len is
> the larger of the two lengths. v1 compared against the allocation, which has the
> output report header to spare.

It's a valid question.
Based on my experience with enabling, touch vendors typically leave some headroom when setting the max input size and max output size, and in most cases, the packet size doesn't reach either limit.

> 
> Can a response that large happen? If it can, comparing input_len would keep the
> same limit as the DATA case:
> 
> 	if (input_len > qsdev->report_buf_size) {

Given that we've left enough headroom, this is a good suggestion. Let's go ahead with it.

Best Regards,
Even Xu

> 
> Best regards,
> HyeongJun