Re: [PATCH 3/8] i3c: mipi-i3c-hci: Add a quirk for 64-bit DMA addressing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] A malicious or non-compliant hardware controller can trigger a division-by-zero kernel panic during probe by reporting a non-zero DAT_ENTRY_SIZE or DCT_ENTRY_SIZE.
- [High] If an I3C transfer is queued but never started by the hardware, the waiting thread will enter an infinite loop in the kernel, leading to a permanent hang.
- [High] A malicious I3C hardware controller can trigger a heap out-of-bounds read by reporting an arbitrarily large number of chunks in IBI status descriptors.
--

commit a8b22e15760d201630758778160cab05d823bd74
Author: Billy Tsai <billy_tsai@aspeedtech.com>

i3c: mipi-i3c-hci: Add a quirk for 64-bit DMA addressing

This commit adds a HCI_QUIRK_DMA_64BIT quirk to declare a 64-bit
streaming and coherent DMA mask for I3C controllers that support it.

> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index a624e3c40484e..c03c3a9cbe4f9 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c

[ ... ]

> @@ -1167,6 +1168,12 @@ static int i3c_hci_probe(struct platform_device *pdev)
>  	hci->quirks = (unsigned long)device_get_match_data(&pdev->dev);
>  	if (!hci->quirks && platform_get_device_id(pdev))
>  		hci->quirks = platform_get_device_id(pdev)->driver_data;
> +	if (hci->quirks & HCI_QUIRK_DMA_64BIT) {
> +		ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +		if (ret)
> +			return dev_err_probe(&pdev->dev, ret,
> +					     "cannot set DMA mask\n");
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but could a malicious or buggy
hardware controller trigger a kernel panic due to division by zero?

In i3c_hci_init(), if the hardware is version 1.0 (where size_in_dwords is
true) and reports a non-zero DAT_ENTRY_SIZE, the driver sets DAT_entry_size
to 0 and then immediately divides by it:

drivers/i3c/master/mipi-i3c-hci/core.c:i3c_hci_init() {
    ...
	hci->DAT_entry_size = FIELD_GET(DAT_ENTRY_SIZE, regval) ? 0 : 8;
	if (size_in_dwords)
		hci->DAT_entries = 4 * hci->DAT_entries / hci->DAT_entry_size;
    ...
}

[Severity: High]
This is a pre-existing issue, but could an unresponsive hardware controller
cause a permanent kernel thread hang?

In i3c_hci_process_xfer(), if the hardware stalls or the DMA ring is blocked,
the transfer might never be marked as started. The loop checks for a timeout,
but remaining_timeout is not decremented, causing an infinite loop:

drivers/i3c/master/mipi-i3c-hci/core.c:i3c_hci_process_xfer() {
    ...
		/* Keep waiting if xfer has not started */
		if (!started)
			continue;
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but can an out-of-bounds heap
read happen if a hardware controller reports an arbitrarily large number of
chunks?

In hci_dma_process_ibi(), ibi_size is calculated from chunk counts reported
in IBI status descriptors. If the calculated ibi_size exceeds the physical
ring buffer size (rh->ibi_chunks_total * rh->ibi_chunk_sz), this could
overflow the rh->ibi_data buffer during memcpy:

drivers/i3c/master/mipi-i3c-hci/dma.c:hci_dma_process_ibi() {
    ...
		memcpy(slot->data + first_part, ring_ibi_data,
		       ibi_size - first_part);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-b4-i3c-hci-ast2700-v1-0-19909e7cbd7e@aspeedtech.com?part=3

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c