[PATCH 12/17] i3c: mipi-i3c-hci: Fix Response Descriptor DATA_LENGTH mask

[sashiko-bot at kernel.org]

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

Pre-existing issues:
- [Critical] The driver blindly trusts the `RESP_DATA_LENGTH` reported by the hardware response descriptor without validating it against the originally requested transfer length (`xfer->data_len`). This allows a buggy or malicious I3C hardware controller to cause an out-of-bounds heap memory write. Specifically, in `hci_pio_process_resp()`, if the hardware reports an `expected` length greater than `received` (which is bounded by `xfer->data_len`), it calls `hci_pio_do_trailing_rx()` with the difference. The `hci_pio_do_trailing_rx()` function then reads this many words from the FIFO and writes them into `xfer->data` without any bounds checking against the allocated buffer size. Furthermore, in `i3c_hci_send_ccc_cmd()` and `i3c_hci_i3c_xfers()`, the unvalidated `RESP_DATA_LENGTH` is copied into the payload or transfer length fields, causing upper layers to potentially trust this mutated length, leading to information leaks or further bounds violations.
--

commit 1331b71b6fc3650effd8b493f413537d7aac5b39
Author: Adrian Hunter <adrian.hunter at intel.com>

i3c: mipi-i3c-hci: Fix Response Descriptor DATA_LENGTH mask

The I3C HCI Response Descriptor defines DATA_LENGTH in Bits[15:0],
with Bits[23:16] reserved in all published HCI specification versions.
This updates the RESP_DATA_LENGTH mask to extract only the 16 defined
bits, correcting the previous inclusion of six reserved bits.

> diff --git a/drivers/i3c/master/mipi-i3c-hci/cmd.h b/drivers/i3c/master/mipi-i3c-hci/cmd.h
> index 7bada7b4b2de5..e0d4a6e0e319f 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/cmd.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/cmd.h
> @@ -24,7 +24,7 @@
>   */
>  #define RESP_STATUS(resp)		FIELD_GET(GENMASK(31, 28), resp)
>  #define RESP_TID(resp)			FIELD_GET(GENMASK(27, 24), resp)
> -#define RESP_DATA_LENGTH(resp)		FIELD_GET(GENMASK(21,  0), resp)
> +#define RESP_DATA_LENGTH(resp)		FIELD_GET(GENMASK(15,  0), resp)

[Severity: Critical]
This is a pre-existing issue, but does the driver blindly trust the
RESP_DATA_LENGTH reported by the hardware response descriptor without
validating it against the originally requested transfer length?

If a buggy or malicious I3C hardware controller reports an expected length
greater than the requested transfer length (xfer->data_len), could this
lead to an out-of-bounds heap memory write?

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);

If expected is greater than received, this passes the difference to
hci_pio_do_trailing_rx().

This function appears to read that many words from the FIFO without
bounds checking against the allocated buffer size:

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

		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);

Additionally, does this unvalidated hardware response length overwrite the
transfer or payload lengths returned to upper layers?

For instance, in drivers/i3c/master/mipi-i3c-hci/core.c:i3c_hci_i3c_xfers():

	for (i = 0; i < nxfers; i++) {
		if (xfer[i].rnw)
			i3c_xfers[i].len = RESP_DATA_LENGTH(xfer[i].response);

And in drivers/i3c/master/mipi-i3c-hci/core.c:i3c_hci_send_ccc_cmd():

	for (i = prefixed; i < nxfers; i++) {
		if (ccc->rnw)
			ccc->dests[i - prefixed].payload.actual_len =
				RESP_DATA_LENGTH(xfer[i].response);

>  
>  #define RESP_ERR_FIELD			GENMASK(31, 28)
>

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