linux-kernel - [PATCH v16 12/12] dmaengine: imx-sdma: add terminated list for freed descriptor in worker

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1626258052-22198-13-git-send-email-yibin.gong@nxp.com>
Date:   Wed, 14 Jul 2021 18:20:52 +0800
From:   Robin Gong <yibin.gong@....com>
To:     vkoul@...nel.org, mark.rutland@....com, broonie@...nel.org,
        robh+dt@...nel.org, catalin.marinas@....com, will.deacon@....com,
        shawnguo@...nel.org, festevam@...il.com, s.hauer@...gutronix.de,
        martin.fuzzey@...wbird.group, u.kleine-koenig@...gutronix.de,
        dan.j.williams@...el.com, matthias.schiffer@...tq-group.com,
        frieder.schrempf@...tron.de, m.felsch@...gutronix.de,
        l.stach@...gutronix.de, xiaoning.wang@....com
Cc:     devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-spi@...r.kernel.org, linux-imx@....com,
        kernel@...gutronix.de, dmaengine@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: [PATCH v16 12/12] dmaengine: imx-sdma: add terminated list for freed descriptor in worker

Add terminated list for keeping descriptor so that it could be freed in
worker without any potential involving next descriptor raised up before
this descriptor freed, because vchan_get_all_descriptors get all
descriptors including the last terminated descriptor and the next
descriptor, hence, the next descriptor maybe freed unexpectly when it's
done in worker without this patch.
https://www.spinics.net/lists/dmaengine/msg23367.html

Signed-off-by: Robin Gong <yibin.gong@....com>
Reported-by: Richard Leitner <richard.leitner@...data.com>
Reviewed-by: Lucas Stach <l.stach@...gutronix.de>
---
 drivers/dma/imx-sdma.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 9ec1edf..cacc725 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -439,6 +439,7 @@ struct sdma_channel {
 	enum dma_status			status;
 	struct imx_dma_data		data;
 	struct work_struct		terminate_worker;
+	struct list_head                terminated;
 	bool				is_ram_script;
 };
 
@@ -1107,9 +1108,6 @@ static void sdma_channel_terminate_work(struct work_struct *work)
 {
 	struct sdma_channel *sdmac = container_of(work, struct sdma_channel,
 						  terminate_worker);
-	unsigned long flags;
-	LIST_HEAD(head);
-
 	/*
 	 * According to NXP R&D team a delay of one BD SDMA cost time
 	 * (maximum is 1ms) should be added after disable of the channel
@@ -1118,10 +1116,7 @@ static void sdma_channel_terminate_work(struct work_struct *work)
 	 */
 	usleep_range(1000, 2000);
 
-	spin_lock_irqsave(&sdmac->vc.lock, flags);
-	vchan_get_all_descriptors(&sdmac->vc, &head);
-	spin_unlock_irqrestore(&sdmac->vc.lock, flags);
-	vchan_dma_desc_free_list(&sdmac->vc, &head);
+	vchan_dma_desc_free_list(&sdmac->vc, &sdmac->terminated);
 }
 
 static int sdma_terminate_all(struct dma_chan *chan)
@@ -1135,6 +1130,13 @@ static int sdma_terminate_all(struct dma_chan *chan)
 
 	if (sdmac->desc) {
 		vchan_terminate_vdesc(&sdmac->desc->vd);
+		/*
+		 * move out current descriptor into terminated list so that
+		 * it could be free in sdma_channel_terminate_work alone
+		 * later without potential involving next descriptor raised
+		 * up before the last descriptor terminated.
+		 */
+		vchan_get_all_descriptors(&sdmac->vc, &sdmac->terminated);
 		sdmac->desc = NULL;
 		schedule_work(&sdmac->terminate_worker);
 	}
@@ -2130,6 +2132,7 @@ static int sdma_probe(struct platform_device *pdev)
 
 		sdmac->channel = i;
 		sdmac->vc.desc_free = sdma_desc_free;
+		INIT_LIST_HEAD(&sdmac->terminated);
 		INIT_WORK(&sdmac->terminate_worker,
 				sdma_channel_terminate_work);
 		/*
-- 
2.7.4