RE: [PATCH] HID: intel-thc-hid: intel-quicki2c: size the input buffer for the DMA

[Xu, Even]

> -----Original Message-----
> From: HyeongJun An <sammiee5311@xxxxxxxxx>
> Sent: Saturday, August 22, 2026 8:46 PM
> To: Xu, Even <even.xu@xxxxxxxxx>; Sun, Xinpeng <xinpeng.sun@xxxxxxxxx>; Jiri
> Kosina <jikos@xxxxxxxxxx>; Benjamin Tissoires <bentiss@xxxxxxxxxx>
> Cc: linux-input@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> stable@xxxxxxxxxxxxxxx; HyeongJun An <sammiee5311@xxxxxxxxx>
> Subject: [PATCH] HID: intel-thc-hid: intel-quicki2c: size the input buffer for the
> DMA
> 
> quicki2c_alloc_report_buf() sizes input_buf as max(max_input_len, SZ_4K), but
> two channels deliver into it and dma_set_max_packet_size() rounds both
> up: RxDMA2 to ALIGN(max_input_len, SZ_4K) and SWDMA to
> ALIGN(max(max_input_len, report_desc_len), SZ_4K).  read_dma_buffer() bounds
> the copy against those, not against the allocation.

Hi,  HyeongJun,

It's not a problem.
There are two layers buffer in THC driver:
one is in thc-hw driver for HW DMA, which initialized in quicki2c_dma_init() by thc_dma_set_max_packet_sizes();
one is in quicki2c driver for HID, which allocated in quicki2c_alloc_report_buf().

SWDMA will use the buffer and size initialized by thc_dma_set_max_packet_sizes(), it's already the max one, so don't worry.

> 
> quicki2c_get_report() reads into input_buf with prd_tbl_len NULL, so nothing
> programs a length, and a device declaring report_desc_len 5000 with
> max_input_len 64 overruns the 4K buffer by 4096 bytes.  The RxDMA2 leg
> additionally needs the I2C max input size clamp to be off.

Quicki2c driver has different buffers for different hid report separately, so input report and report descriptor have different buffer with different size.

> 
> Size input_buf from the SWDMA packet size and keep the 4K floor.
> 
> Fixes: 66b59bfce6d9 ("HID: intel-thc-hid: intel-quicki2c: Complete THC QuickI2C
> driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: HyeongJun An <sammiee5311@xxxxxxxxx>
> ---
> Not reproduced on hardware.  Triggering it needs a controller that sends more
> than the lengths it declared, which is the case the existing comment was already
> written for.
> 
> max_report_len is reused for the report_buf allocation below, so that buffer and
> qcdev->report_len grow with input_buf whenever the SWDMA ceiling is the
> larger term.  Both stay bounded by that ceiling.

SWDMA can be used for report descriptor retrieve and manual input report read,
quicki2c driver already set SWDMA buffer to max(le16_to_cpu(qcdev->dev_desc.max_input_len),  le16_to_cpu(qcdev->dev_desc.report_desc_len));
so it's not a problem, that's why you never reproduce the issue.

> 
>  drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> index 0d2ad7bc3648..f6f9f95296d3 100644
> --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> @@ -602,9 +602,12 @@ static int quicki2c_alloc_report_buf(struct
> quicki2c_device *qcdev)
> 
>  	/*
>  	 * Some HIDI2C devices don't declare input/output max length correctly,
> -	 * give default 4K buffer to avoid DMA buffer overrun.
> +	 * give default 4K buffer to avoid DMA buffer overrun.  Both RxDMA2 and
> +	 * SWDMA land here, so cover the larger SWDMA packet size.
>  	 */
> -	max_report_len = max(le16_to_cpu(qcdev->dev_desc.max_input_len),
> SZ_4K);
> +	max_report_len = max(le16_to_cpu(qcdev->dev_desc.max_input_len),
> +			     le16_to_cpu(qcdev->dev_desc.report_desc_len));
> +	max_report_len = max_t(size_t, ALIGN(max_report_len, SZ_4K), SZ_4K);
> 
>  	qcdev->input_buf = devm_kzalloc(qcdev->dev, max_report_len,
> GFP_KERNEL);
>  	if (!qcdev->input_buf)
> --
> 2.43.0