[PATCH 3/5] dmaengine: sf-pdma: fix NULL pointer dereference in error and done handlers

Max Hsu
2026-02-20T19:44:21Z

Fix NULL pointer dereferences in both the error and done tasklets that
can occur due to race conditions during channel termination or completion.

Both tasklets (sf_pdma_errbh_tasklet and sf_pdma_donebh_tasklet)
dereference chan->desc without checking if it's NULL. However,
chan->desc can be NULL in legitimate scenarios:

1. During sf_pdma_terminate_all(): The function sets chan->desc = NULL
while holding vchan.lock, but interrupts for previously submitted
transactions could fire after the lock is released, before the
hardware is fully quiesced. These interrupts can schedule tasklets
that will run with chan->desc = NULL.

2. During channel cleanup: Similar race condition during
sf_pdma_free_chan_resources().

The fix adds NULL checks at the beginning of both tasklets, protected
by vchan.lock, using the same lock that terminate_all and
free_chan_resources use when setting chan->desc = NULL. This ensures
that either:
- The descriptor is valid and we can safely process it, or
- The descriptor was already freed and we safely skip processing

Fixes: 6973886ad58e ("dmaengine: sf-pdma: add platform DMA support for HiFive Unleashed A00")
Cc: stable@vger.kernel.org
Signed-off-by: Max Hsu <max.hsu@sifive.com>
---
drivers/dma/sf-pdma/sf-pdma.c | 43 +++++++++++++++++++++++++++++++++----------
1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
index ac7d3b127a24..70e4afcda52a 100644
--- a/drivers/dma/sf-pdma/sf-pdma.c
+++ b/drivers/dma/sf-pdma/sf-pdma.c
@@ -298,33 +298,56 @@ static void sf_pdma_free_desc(struct virt_dma_desc *vdesc)
static void sf_pdma_donebh_tasklet(struct tasklet_struct *t)
{
struct sf_pdma_chan *chan = from_tasklet(chan, t, done_tasklet);
+ struct sf_pdma_desc *desc;
unsigned long flags;

- spin_lock_irqsave(&chan->lock, flags);
- if (chan->xfer_err) {
- chan->retries = MAX_RETRY;
- chan->status = DMA_COMPLETE;
- chan->xfer_err = false;
+ spin_lock_irqsave(&chan->vchan.lock, flags);
+ desc = chan->desc;
+ if (!desc) {
+ /*
+ * The descriptor was already freed (e.g., by terminate_all
+ * or completion on another CPU). Nothing to do.
+ */
+ spin_unlock_irqrestore(&chan->vchan.lock, flags);
+ return;
}
- spin_unlock_irqrestore(&chan->lock, flags);

- spin_lock_irqsave(&chan->vchan.lock, flags);
- list_del(&chan->desc->vdesc.node);
- vchan_cookie_complete(&chan->desc->vdesc);
+ list_del(&desc->vdesc.node);
+ vchan_cookie_complete(&desc->vdesc);

chan->desc = sf_pdma_get_first_pending_desc(chan);
if (chan->desc)
sf_pdma_xfer_desc(chan);

spin_unlock_irqrestore(&chan->vchan.lock, flags);
+
+ spin_lock_irqsave(&chan->lock, flags);
+ if (chan->xfer_err) {
+ chan->retries = MAX_RETRY;
+ chan->status = DMA_COMPLETE;
+ chan->xfer_err = false;
+ }
+ spin_unlock_irqrestore(&chan->lock, flags);
}

static void sf_pdma_errbh_tasklet(struct tasklet_struct *t)
{
struct sf_pdma_chan *chan = from_tasklet(chan, t, err_tasklet);
- struct sf_pdma_desc *desc = chan->desc;
+ struct sf_pdma_desc *desc;
unsigned long flags;

+ spin_lock_irqsave(&chan->vchan.lock, flags);
+ desc = chan->desc;
+ if (!desc) {
+ /*
+ * The descriptor was already freed (e.g., by terminate_all
+ * or completion on another CPU). Nothing to do.
+ */
+ spin_unlock_irqrestore(&chan->vchan.lock, flags);
+ return;
+ }
+ spin_unlock_irqrestore(&chan->vchan.lock, flags);
+
spin_lock_irqsave(&chan->lock, flags);
if (chan->retries <= 0) {
/* fail to recover */

--
2.43.0