Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chengfeng Ye <dg573847474@gmail.com>
To:     shuge@allwinnertech.com, maxime.ripard@free-electrons.com,
        vkoul@kernel.org, wens@csie.org, jernej.skrabec@gmail.com,
        samuel@sholland.org, p.zabel@pengutronix.de
Cc:     dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org,
        Chengfeng Ye <dg573847474@gmail.com>
Subject: [PATCH] dmaengine: sun6i: Fix potential deadlock on &sdev->lock
Date:   Wed, 26 Jul 2023 04:58:26 +0000
Message-Id: <20230726045826.49554-1-dg573847474@gmail.com>
Precedence: bulk

As &sdev->lock is acquired by tasklet sun6i_dma_tasklet() executed under
softirq context, other acquisition of the same lock under process context
should disable irq, otherwise deadlock could happen if the soft irq preempt
the execution while the lock is held in process context on the same CPU.

sun6i_dma_terminate_all() and sun6i_dma_pause() callbacks acquire the same
lock without disabling irq inside the function.

Possible deadlock scenario:
sun6i_dma_pause()
    -> spin_lock(&sdev->lock);
        <tasklet softirq interruption>
        -> sun6i_dma_tasklet()
        -> spin_lock_irq(&sdev->lock) (deadlock here)

This flaw was found by an experimental static analysis tool I am developing
for irq-related deadlock.

The tentative patch fixes the potential deadlock by spin_lock_bh() to
disable softirq.

Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
---
 drivers/dma/sun6i-dma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index ebfd29888b2f..8bad6ce62ea8 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -873,9 +873,9 @@ static int sun6i_dma_pause(struct dma_chan *chan)
 		writel(DMA_CHAN_PAUSE_PAUSE,
 		       pchan->base + DMA_CHAN_PAUSE);
 	} else {
-		spin_lock(&sdev->lock);
+		spin_lock_bh(&sdev->lock);
 		list_del_init(&vchan->node);
-		spin_unlock(&sdev->lock);
+		spin_unlock_bh(&sdev->lock);
 	}
 
 	return 0;
@@ -914,9 +914,9 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
 	unsigned long flags;
 	LIST_HEAD(head);
 
-	spin_lock(&sdev->lock);
+	spin_lock_bh(&sdev->lock);
 	list_del_init(&vchan->node);
-	spin_unlock(&sdev->lock);
+	spin_unlock_bh(&sdev->lock);
 
 	spin_lock_irqsave(&vchan->vc.lock, flags);
 
-- 
2.17.1