Devicetree
 help / color / mirror / Atom feed
From: Max Hsu <max.hsu@sifive.com>
To: Paul Walmsley <pjw@kernel.org>,
	 Samuel Holland <samuel.holland@sifive.com>,
	Vinod Koul <vkoul@kernel.org>,  Frank Li <Frank.Li@kernel.org>,
	Green Wan <green.wan@sifive.com>,  Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	 Conor Dooley <conor+dt@kernel.org>,
	Palmer Debbelt <palmer@sifive.com>,
	 Conor Dooley <conor@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	 Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>
Cc: linux-riscv@lists.infradead.org, dmaengine@vger.kernel.org,
	 linux-kernel@vger.kernel.org,
	Paul Walmsley <paul.walmsley@sifive.com>,
	 devicetree@vger.kernel.org, Max Hsu <max.hsu@sifive.com>,
	 stable@vger.kernel.org
Subject: [PATCH 3/5] dmaengine: sf-pdma: fix NULL pointer dereference in error and done handlers
Date: Sat, 21 Feb 2026 03:43:55 +0800	[thread overview]
Message-ID: <20260221-pdma-v1-3-838d929c2326@sifive.com> (raw)
In-Reply-To: <20260221-pdma-v1-0-838d929c2326@sifive.com>

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


  parent reply	other threads:[~2026-02-20 19:44 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-20 19:43 [PATCH 0/5] dmaengine: sf-pdma: critical fixes and FU740 support Max Hsu
2026-02-20 19:43 ` [PATCH 1/5] dmaengine: sf-pdma: add missing PDMA base offset to register calculations Max Hsu
2026-02-20 20:10   ` Frank Li
2026-02-20 22:26     ` Conor Dooley
2026-02-20 19:43 ` [PATCH 2/5] dmaengine: sf-pdma: fix race between done and error interrupts Max Hsu
2026-02-20 20:26   ` Frank Li
2026-02-20 19:43 ` Max Hsu [this message]
2026-02-20 20:28   ` [PATCH 3/5] dmaengine: sf-pdma: fix NULL pointer dereference in error and done handlers Frank Li
2026-02-20 19:43 ` [PATCH 4/5] dt-bindings: dma: sifive,fu540-c000-pdma: add fu740 support Max Hsu
2026-02-20 22:28   ` Conor Dooley
2026-02-20 19:43 ` [PATCH 5/5] riscv: dts: sifive: fu740: add PDMA device node Max Hsu
2026-02-20 20:30   ` Frank Li
2026-02-20 22:27     ` Conor Dooley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260221-pdma-v1-3-838d929c2326@sifive.com \
    --to=max.hsu@sifive.com \
    --cc=Frank.Li@kernel.org \
    --cc=alex@ghiti.fr \
    --cc=aou@eecs.berkeley.edu \
    --cc=conor+dt@kernel.org \
    --cc=conor@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=green.wan@sifive.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=palmer@sifive.com \
    --cc=paul.walmsley@sifive.com \
    --cc=pjw@kernel.org \
    --cc=robh@kernel.org \
    --cc=samuel.holland@sifive.com \
    --cc=stable@vger.kernel.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox