linux-kernel - [PATCH] dmaengine: mediatek: Fix a possible deadlock error in mtk_cqdma_tx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20250507053736.47220-1-chenqiuji666@gmail.com>
Date: Wed,  7 May 2025 13:37:36 +0800
From: Qiu-ji Chen <chenqiuji666@...il.com>
To: sean.wang@...iatek.com,
	vkoul@...nel.org,
	matthias.bgg@...il.com,
	angelogioacchino.delregno@...labora.com
Cc: dmaengine@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	baijiaju1990@...il.com,
	Qiu-ji Chen <chenqiuji666@...il.com>,
	stable@...r.kernel.org
Subject: [PATCH] dmaengine: mediatek: Fix a possible deadlock error in mtk_cqdma_tx_status()

This patch fixes a potential deadlock bug. We observed that in the
mtk-cqdma.c file, most functions like mtk_cqdma_issue_pending() and
mtk_cqdma_free_active_desc() follow the correct locking sequence by
acquiring the pc lock first before taking the vc lock when handling the vc
and pc fields. However, in mtk_cqdma_tx_status(), the function incorrectly
acquires the vc lock first before calling mtk_cqdma_find_active_desc(),
which subsequently acquires the pc lock. This reversed lock acquisition
order (vc → pc) violates the established sequence (pc → vc) and could
potentially trigger deadlock scenarios.

To resolve this issue, we have moved the vc lock acquisition code from
mtk_cqdma_tx_status() into the mtk_cqdma_find_active_desc() function.
This adjustment ensures proper lock ordering while maintaining
functionality. Since mtk_cqdma_find_active_desc() is a static function
with only one call site in mtk_cqdma_tx_status(), this fix effectively
addresses the deadlock risk without introducing unintended side effects
to other components.

This possible bug is found by an experimental static analysis tool
developed by our team. This tool analyzes the locking APIs to extract
function pairs that can be concurrently executed, and then analyzes the
instructions in the paired functions to identify possible concurrency bugs
including data races and atomicity violations.

Fixes: b1f01e48df5a ("dmaengine: mediatek: Add MediaTek Command-Queue DMA controller for MT6765 SoC")
Cc: stable@...r.kernel.org
Signed-off-by: Qiu-ji Chen <chenqiuji666@...il.com>
---
 drivers/dma/mediatek/mtk-cqdma.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
index d5ddb4e30e71..656354bccb44 100644
--- a/drivers/dma/mediatek/mtk-cqdma.c
+++ b/drivers/dma/mediatek/mtk-cqdma.c
@@ -423,11 +423,14 @@ static struct virt_dma_desc *mtk_cqdma_find_active_desc(struct dma_chan *c,
 	unsigned long flags;

 	spin_lock_irqsave(&cvc->pc->lock, flags);
+	spin_lock_irqsave(&cvc->vc.lock, flags);
 	list_for_each_entry(vd, &cvc->pc->queue, node)
 		if (vd->tx.cookie == cookie) {
+			spin_unlock_irqrestore(&cvc->vc.lock, flags);
 			spin_unlock_irqrestore(&cvc->pc->lock, flags);
 			return vd;
 		}
+	spin_unlock_irqrestore(&cvc->vc.lock, flags);
 	spin_unlock_irqrestore(&cvc->pc->lock, flags);

 	list_for_each_entry(vd, &cvc->vc.desc_issued, node)
@@ -452,9 +455,7 @@ static enum dma_status mtk_cqdma_tx_status(struct dma_chan *c,
 	if (ret == DMA_COMPLETE || !txstate)
 		return ret;

-	spin_lock_irqsave(&cvc->vc.lock, flags);
 	vd = mtk_cqdma_find_active_desc(c, cookie);
-	spin_unlock_irqrestore(&cvc->vc.lock, flags);

 	if (vd) {
 		cvd = to_cqdma_vdesc(vd);
-- 
2.34.1