Re: [PATCH v5 01/17] i3c: renesas: Check that the transfer is valid before accessing it

On Mon, Jul 13, 2026 at 04:05:29PM +0300, Claudiu Beznea wrote:
> From: Claudiu Beznea <[email protected]>
>
> The Renesas I3C driver uses an asynchronous model to transfer data. It
> prepares a struct renesas_i3c_xfer, enqueues it, and waits for completion.
> The interrupt handler dequeues the transfer, updates/uses it, and signals
> the waiting thread.
>
> If the completion times out, the waiting thread dequeues the transfer and
> free it. If an interrupt fires after that, the handler may access freed
> memory, leading to crashes.
>
> Check that the transfer is still valid before accessing it in the
> interrupt handler. With it clear any status flags and disable all
> the interrupts to avoid triggering the same interrupts again.
>
> Fixes: d028219a9f14 ("i3c: master: Add basic driver for the Renesas I3C controller")
> Cc: [email protected]
> Signed-off-by: Claudiu Beznea <[email protected]>
> ---

Reviewed-by: Frank Li <[email protected]>

>
> Changes in v5:
> - introduced renesas_i3c_irqs_mask_and_clear_locked() that keeps
>   unified the IRQ mask and clean path
> - updated the patch description
>
> Changes in v4:
> - disable also the interrupts
> - dropped the Rb tag
>
> Changes in v3:
> - none
>
> Changes in v2:
> - clean the IRQ status bits before returning IRQ_HANDLED and adjusted the
>   patch description to reflect this change
> - collected Frank's tag. Frank, please let me know if you consider
>   I should drop your tag. Thanks!
>
>  drivers/i3c/master/renesas-i3c.c | 52 +++++++++++++++++++++++++++-----
>  1 file changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index f39c449922ca..38b8428f464c 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -433,6 +433,21 @@ static void renesas_i3c_enqueue_xfer(struct renesas_i3c *i3c, struct renesas_i3c
>  	}
>  }
>
> +static void renesas_i3c_irqs_mask_and_clear_locked(struct renesas_i3c *i3c)
> +{
> +	/* Disable all the interrupts. */
> +	renesas_writel(i3c->regs, BIE, 0);
> +	renesas_writel(i3c->regs, NTIE, 0);
> +
> +	/* Clear normal transfer status flags. */
> +	renesas_writel(i3c->regs, NTST, 0);
> +
> +	/* Clear bus status flags. */
> +	renesas_writel(i3c->regs, BST, 0);
> +	/* Read back registers to confirm writes have fully propagated. */
> +	renesas_readl(i3c->regs, BST);
> +}
> +
>  static void renesas_i3c_wait_xfer(struct renesas_i3c *i3c, struct renesas_i3c_xfer *xfer)
>  {
>  	unsigned long time_left;
> @@ -1014,6 +1029,11 @@ static irqreturn_t renesas_i3c_tx_isr(int irq, void *data)
>
>  	scoped_guard(spinlock, &i3c->xferqueue.lock) {
>  		xfer = i3c->xferqueue.cur;
> +		if (!xfer) {
> +			renesas_i3c_irqs_mask_and_clear_locked(i3c);
> +			return IRQ_HANDLED;
> +		}
> +
>  		cmd = xfer->cmds;
>
>  		if (xfer->is_i2c_xfer) {
> @@ -1054,6 +1074,11 @@ static irqreturn_t renesas_i3c_resp_isr(int irq, void *data)
>
>  	scoped_guard(spinlock, &i3c->xferqueue.lock) {
>  		xfer = i3c->xferqueue.cur;
> +		if (!xfer) {
> +			renesas_i3c_irqs_mask_and_clear_locked(i3c);
> +			return IRQ_HANDLED;
> +		}
> +
>  		cmd = xfer->cmds;
>
>  		/* Clear the Respone Queue Full status flag*/
> @@ -1138,6 +1163,11 @@ static irqreturn_t renesas_i3c_tend_isr(int irq, void *data)
>
>  	scoped_guard(spinlock, &i3c->xferqueue.lock) {
>  		xfer = i3c->xferqueue.cur;
> +		if (!xfer) {
> +			renesas_i3c_irqs_mask_and_clear_locked(i3c);
> +			return IRQ_HANDLED;
> +		}
> +
>  		cmd = xfer->cmds;
>
>  		if (xfer->is_i2c_xfer) {
> @@ -1184,6 +1214,11 @@ static irqreturn_t renesas_i3c_rx_isr(int irq, void *data)
>
>  	scoped_guard(spinlock, &i3c->xferqueue.lock) {
>  		xfer = i3c->xferqueue.cur;
> +		if (!xfer) {
> +			renesas_i3c_irqs_mask_and_clear_locked(i3c);
> +			return IRQ_HANDLED;
> +		}
> +
>  		cmd = xfer->cmds;
>
>  		if (xfer->is_i2c_xfer) {
> @@ -1234,15 +1269,13 @@ static irqreturn_t renesas_i3c_stop_isr(int irq, void *data)
>  	struct renesas_i3c_xfer *xfer;
>
>  	scoped_guard(spinlock, &i3c->xferqueue.lock) {
> -		xfer = i3c->xferqueue.cur;
> -
> -		/* read back registers to confirm writes have fully propagated */
> -		renesas_writel(i3c->regs, BST, 0);
> -		renesas_readl(i3c->regs, BST);
> -		renesas_writel(i3c->regs, BIE, 0);
> -		renesas_clear_bit(i3c->regs, NTST, NTST_TDBEF0 | NTST_RDBFF0);
> +		renesas_i3c_irqs_mask_and_clear_locked(i3c);
>  		renesas_clear_bit(i3c->regs, SCSTRCTL, SCSTRCTL_RWE);
>
> +		xfer = i3c->xferqueue.cur;
> +		if (!xfer)
> +			return IRQ_HANDLED;
> +
>  		xfer->ret = 0;
>  		complete(&xfer->comp);
>  	}
> @@ -1259,6 +1292,11 @@ static irqreturn_t renesas_i3c_start_isr(int irq, void *data)
>
>  	scoped_guard(spinlock, &i3c->xferqueue.lock) {
>  		xfer = i3c->xferqueue.cur;
> +		if (!xfer) {
> +			renesas_i3c_irqs_mask_and_clear_locked(i3c);
> +			return IRQ_HANDLED;
> +		}
> +
>  		cmd = xfer->cmds;
>
>  		if (xfer->is_i2c_xfer) {
> --
> 2.43.0
>