Message-ID: <sashiko-outbox-207637@kernel.org>
From: sashiko-bot@kernel.org
Subject: Re: [PATCH v4 28/33] dmaengine: sf-pdma: route error callbacks
 through channel BH
Reply-To: sashiko-reviews@lists.linux.dev
To: "Allen Pais" <allen.lkml@gmail.com>
Cc: dmaengine@vger.kernel.org, vkoul@kernel.org, Frank.Li@kernel.org
In-Reply-To: <25d7826e0e6738356657f523e6f6cf3450e4b26c.1794315350.git.allen.lkml@gmail.com>
References: <cover.1794315350.git.allen.lkml@gmail.com>
 <25d7826e0e6738356657f523e6f6cf3450e4b26c.1794315350.git.allen.lkml@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Tue, 10 Nov 2026 19:08:47 +0000
Precedence: list

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] dmaengine: sf-pdma: unlocked chan->desc read in
  sf_pdma_errbh_tasklet()
- [Medium] dmaengine: sf-pdma: stale error tx_result on reused
  descriptors
--

commit cbd0063b16af2f825d62b2888a259c514966e4ac
Author: Allen Pais <allen.lkml@gmail.com>

dmaengine: sf-pdma: route error callbacks through channel BH

This commit routes failed transfer completions in sf-pdma through
vchan_cookie_complete() with DMA_TRANS_ABORTED instead of invoking the
client callback directly from the error tasklet, and kills the driver
tasklets before killing the channel BH on removal.

> diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
> index ca60fb46ade00..992f9f1037028 100644
> --- a/drivers/dma/sf-pdma/sf-pdma.c
> +++ b/drivers/dma/sf-pdma/sf-pdma.c
> @@ -326,9 +326,21 @@ static void sf_pdma_errbh_tasklet(struct tasklet_str=
uct *t)
> =20
>  	spin_lock_irqsave(&chan->lock, flags);
>  	if (chan->retries <=3D 0) {
> -		/* fail to recover */
> +		/* fail to recover: abort this descriptor and move on */
> +		chan->retries =3D MAX_RETRY;
> +		chan->xfer_err =3D false;
>  		spin_unlock_irqrestore(&chan->lock, flags);
> -		dmaengine_desc_get_callback_invoke(desc->async_tx, NULL);
> +
> +		spin_lock_irqsave(&chan->vchan.lock, flags);
> +		list_del(&desc->vdesc.node);

[Severity: High]
Can this cause a use-after-free or NULL pointer dereference on desc?

sf_pdma_errbh_tasklet() reads desc =3D chan->desc at entry before acquiring
chan->vchan.lock, and neither sf_pdma_terminate_all() nor
sf_pdma_free_chan_resources() synchronizes with err_tasklet when freeing
chan->desc:

CPU0                                    CPU1
sf_pdma_errbh_tasklet()
  desc =3D chan->desc;
                                        sf_pdma_terminate_all()
                                          spin_lock_irqsave(&chan->vchan.lo=
ck, ...)
                                          kfree(chan->desc);
                                          chan->desc =3D NULL;
                                          spin_unlock_irqrestore(&chan->vch=
an.lock, ...)
  ...
  spin_lock_irqsave(&chan->vchan.lock, ...)
  list_del(&desc->vdesc.node);          <-- use-after-free
  vchan_cookie_complete(&desc->vdesc);

If sf_pdma_terminate_all() or sf_pdma_donebh_tasklet() clears or replaces
chan->desc before sf_pdma_errbh_tasklet() runs, desc can also be NULL or
already completed, corrupting the virt-dma descriptor lists.  Re-reading
chan->desc under chan->vchan.lock and checking that it is non-NULL before
unlinking and completing it would avoid this.

> +		desc->vdesc.tx_result.result =3D DMA_TRANS_ABORTED;
> +		desc->vdesc.tx_result.residue =3D desc->xfer_size;
> +		vchan_cookie_complete(&desc->vdesc);

[Severity: Medium]
Does a reusable descriptor (DMA_CTRL_REUSE) keep reporting
DMA_TRANS_ABORTED on later successful transfers after an abort?

sf_pdma_probe() sets pdma->dma_dev.descriptor_reuse =3D true, and
vchan_tx_prep() only initializes tx_result to DMA_TRANS_NOERROR and
residue 0 when the descriptor is first prepared.  Once
sf_pdma_errbh_tasklet() overwrites desc->vdesc.tx_result here, neither
sf_pdma_xfer_desc() nor sf_pdma_donebh_tasklet() resets tx_result back to
DMA_TRANS_NOERROR with zero residue, so subsequent successful completions
of the reused descriptor will pass the stale error result and residue to
the client callback.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/cover.1794315350.gi=
t.allen.lkml@gmail.com?part=3D28