RE: [PATCH 2/2] HID: intel-thc-hid: intel-quickspi: bound the response to report_buf

-----Original Message-----
From: Xu, Even
Sent: Monday, September 14, 2026 10:00 AM
To: 'HyeongJun An' <redacted>; Sun, Xinpeng
[off-list ref]; Jiri Kosina [off-list ref]; Benjamin Tissoires
[off-list ref]
Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org;
stable@vger.kernel.org
Subject: RE: [PATCH 2/2] HID: intel-thc-hid: intel-quickspi: bound the response to
report_buf

quoted

-----Original Message-----
From: HyeongJun An <redacted>
Sent: Thursday, August 20, 2026 7:16 PM
To: Xu, Even <even.xu@intel.com>; Sun, Xinpeng
[off-list ref]; Jiri Kosina [off-list ref]; Benjamin
Tissoires [off-list ref]
Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org;
stable@vger.kernel.org; HyeongJun An [off-list ref]
Subject: [PATCH 2/2] HID: intel-thc-hid: intel-quickspi: bound the
response to report_buf

quickspi_handle_input_data() copies a GET_FEATURE or GET_INPUT_REPORT
response into qsdev->report_buf using a length the controller supplied.
The only check it passes is against buf_len, the number of bytes the
DMA delivered, which says nothing about the destination.  report_buf
holds

This is a special capability of the THC hardware. At the hardware level, THC
supports the HID-over-SPI protocol, which means it understands every HIDSPI
message it receives. When THC initiates a DMA transfer to fetch data, it verifies
whether the received data size, including both the packet size declared in the
header and the actual size of the transmitted packet, complies with the MAX
Packet Size requirement. The packet is accepted only if it does. If the size
exceeds the limit, THC reports a DMA buffer overflow, and the packet is
discarded.

This capability of handling software logic in hardware can greatly simplify the
driver's processing logic and maximize data-transfer performance. This is also
why Intel created the THC IP.

quoted

HIDSPI_OUTPUT_REPORT_SIZE(max(max_output_len, max_input_len)), 68
bytes for a controller reporting 64 for both, while the copy is
bounded only by the 4K DMA packet.

The REPORT_DESCRIPTOR_RESPONSE case a few lines up validates against
the size of its own destination.  Do the same here and let the waiter
in
quickspi_get_report() time out, as the other malformed-frame checks do.

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 <redacted>
---
This needs commit a59cf84441f9 ("HID: intel-thc-hid: intel-quickspi:
validate report size before copy") for qsdev->report_buf_size, which
is in hid.git for-next but not mainline yet.  Same Fixes: tag, so a
stable backport wants the two in that order.

 .../intel-thc-hid/intel-quickspi/quickspi-protocol.c | 12
++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

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 847c5ec55569..7d34cc22a11a 100644

--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c

@@ -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,10
+211,17 @@ 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-

quoted

content_id);

-		memcpy(qsdev->report_buf, input_report, qsdev->report_len);
+		memcpy(qsdev->report_buf, input_report, report_len);

According to above explanation, this logic checking isn't a problem, because THC
hardware will check it and guarantee the safe size.

Thanks for your patch!

Best Regards,
Even Xu

Recently, just realized a case:
THC hardware depends on touch controller's report length to determine the correct package length and buffer overflow.
If there is something wrong in the touch controller side, such as: touch controller's behavior is wrong, or data lost in bus transferring such as bus noise, THC hardware will receive wrong data.
We need to avoid this in software driver.
From that viewpoint, this patch is valuable.

Reviewed-by: Even Xu <even.xu@intel.com>

Hi, Jiri and Benjamin,

Could you help merge this patch?
Thank you very much!

Best Regards,
Even Xu

quoted

 		qsdev->get_report_cmpl = true;
 		wake_up_interruptible(&qsdev->get_report_cmpl_wq);
--
2.43.0