Re: [PATCH v5 08/17] i3c: renesas: Fix out-of-bounds access for newdevs mask

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver completely ignores transfer completion status and timeouts in critical I/O functions (`renesas_i3c_daa`, `renesas_i3c_i3c_xfers`, `renesas_i3c_i2c_xfers`), leading to silent failures, uninitialized kernel memory leaks, and race conditions.
- [High] The Dynamic Address Assignment (DAA) logic incorrectly calculates the `newdevs` bitmask using `GENMASK(..., 0)`, failing to account for non-contiguous (fragmented) index allocations.
- [High] Missing hardware reset on transfer timeout causes command/response FIFOs to desynchronize, corrupting subsequent transfers.
--

--- Patch [8]: [PATCH v5 08/17] i3c: renesas: Fix out-of-bounds access for newdevs mask ---
commit 141d263a8a62dfaec861ec3e856a94b76c38fa01
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

i3c: renesas: Fix out-of-bounds access for newdevs mask

When software initiates DAA (Dynamic Address Assignment), the controller
reports the result via the NRSPQP (Normal Response Queue Port Register).
This patch adjusts the condition that computes the newly discovered devices
bitmask to prevent an out-of-bounds access when the I3C bus is empty.

> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index b9784d238f61c..c459e40fd5ffc 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -703,7 +703,11 @@ static int renesas_i3c_daa(struct i3c_master_controller *m)
>  
>  	renesas_i3c_wait_xfer(i3c, xfer);

[Severity: High]
This is a pre-existing issue, but does the driver need to check the
completion status of renesas_i3c_wait_xfer()?

If this transfer times out, cmd->rx_count remains 0, which could cause the
subsequent code to process phantom devices.

Similar unhandled timeouts appear in renesas_i3c_i3c_xfers(), where returning
0 unconditionally could leak uninitialized read buffers, and in
renesas_i3c_i2c_xfers(), where ignoring the timeout allows the loop to modify
cmd fields concurrently with pending hardware operations.

Additionally, what happens to the hardware state if a timeout occurs?

drivers/i3c/master/renesas-i3c.c:renesas_i3c_wait_xfer() {
    ...
    time_left = wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000));
    if (!time_left)
        renesas_i3c_dequeue_xfer(i3c, xfer);
    ...
}

If the transfer is dequeued from software on timeout without triggering a
hardware abort or reset, could the controller remain active? The next
enqueued software transfer might then be corrupted by the hardware completion
interrupt of the previous timed-out transfer.

> -	newdevs = GENMASK(i3c->maxdevs - cmd->rx_count - 1, 0);
> +	if (cmd->rx_count >= i3c->maxdevs)
> +		newdevs = 0;
> +	else
> +		newdevs = GENMASK(i3c->maxdevs - cmd->rx_count - 1, 0);
> +
>  	newdevs &= ~olddevs;

[Severity: High]
This isn't a bug introduced by this patch, but does the calculation of
newdevs properly account for non-contiguous index allocations?

The Dynamic Address Assignment logic calculates the newdevs bitmask using
GENMASK(..., 0), which computes a bitmask strictly starting from index 0.

If a device is detached leaving a gap in free_pos (e.g., at index 0) while
higher indices are occupied, DAA sets DEV_INDEX to the first gap. Since
GENMASK(..., 0) ignores the DEV_INDEX offset, applying newdevs &= ~olddevs
could clear the incorrectly shifted bits corresponding to occupied slots.
Could this cause newly attached devices to be permanently dropped?

>  
>  	for (pos = 0; pos < i3c->maxdevs; pos++) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713130545.568657-1-claudiu.beznea+renesas@tuxon.dev?part=8

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