Re: [PATCH 13/17] i3c: mipi-i3c-hci: Remove invalid transfer size limit

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Out-of-bounds write in `hci_pio_do_trailing_rx` due to trusting hardware-provided response data length.
--

commit 3ab8715cd2bb671d95cd856e91ac9ea00583f567
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: mipi-i3c-hci: Remove invalid transfer size limit

The driver derives a maximum transfer size from HC_CAP_MAX_DATA_LENGTH in
HC_CAPABILITIES, but no published version of the I3C HCI specification
defines such a field. This commit removes the unused capability definition
and the dead size check.

> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index b9b20797d0454..4629a16debc98 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
[ ... ]
> @@ -482,13 +480,8 @@ static int i3c_hci_i3c_xfers(struct i3c_dev_desc *dev,
>  	if (!xfer)
>  		return -ENOMEM;
>  
> -	size_limit = 1U << (16 + FIELD_GET(HC_CAP_MAX_DATA_LENGTH, hci->caps));
> -
>  	for (i = 0; i < nxfers; i++) {
>  		xfer[i].data_len = i3c_xfers[i].len;
> -		ret = -EFBIG;
> -		if (xfer[i].data_len >= size_limit)
> -			goto out;

[Severity: High]
This is a pre-existing issue, but could this lead to an out-of-bounds write
during response processing?

If the I3C HCI controller hardware returns a response descriptor with a data
length (RESP_DATA_LENGTH) greater than the originally requested buffer size
stored here in xfer->data_len, it appears the hardware-provided value is
trusted without being bounded.

In drivers/i3c/master/mipi-i3c-hci/pio.c:hci_pio_process_resp():

    received = xfer->data_len - xfer->data_left;
    expected = RESP_DATA_LENGTH(xfer->response);
    if (expected > received) {
        hci_pio_do_trailing_rx(hci, pio, expected - received);
    }

This leads to an oversized count being passed to hci_pio_do_trailing_rx():

drivers/i3c/master/mipi-i3c-hci/pio.c:hci_pio_do_trailing_rx() {
    ...
    if (count >= 4) {
        unsigned int nr_words = count / 4;
        /* extract data from FIFO */
        xfer->data_left -= nr_words * 4;
        dev_dbg(&hci->master.dev, "now %d left %d",
            nr_words * 4, xfer->data_left);
        while (nr_words--)
            *p++ = pio_reg_read(XFER_DATA_PORT);
    }
    ...
}

Will this unconditionally read the extra bytes from the hardware FIFO into
the memory pointed to by p, overflowing the xfer->data buffer if a faulty
or malicious target device induces the master to read more data than
requested?

>  		xfer[i].rnw = i3c_xfers[i].rnw;
>  		if (i3c_xfers[i].rnw) {
>  			xfer[i].data = i3c_xfers[i].data.in;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914113003.183150-1-adrian.hunter@intel.com?part=13

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