[PATCH] mtd: rawnand: r852: serialize DMA timeout cleanup with interrupts

[James Hilliard]

r852_do_dma() calls r852_dma_done() without irqlock when its completion
wait times out. A completion interrupt can finish the same transfer and
unmap its buffer before, or concurrently with, the timeout cleanup. The
second cleanup then unmaps the buffer again and overwrites the result.

Take irqlock after the wait and abort only if the DMA stage is still
active. If the interrupt completed first, retain its result and copy
successful bounced reads back to the caller even when the wait expired.

Publish the active transfer state under the same lock after preparing
its buffer, and clear pending DMA status before starting it. Ignore
interrupts for an idle transfer, including error interrupts, so they
cannot release an already completed mapping or signal a new completion.

Use the active stage to determine mapping ownership and clear the saved
address after teardown. A streaming mapping at DMA address zero must
also be unmapped; only the persistent coherent bounce buffer is exempt.

Fixes: 67e054e91924 ("mtd: nand: Add driver for Ricoh xD/SmartMedia reader")
Cc: stable at vger.kernel.org
Closes: https://lore.kernel.org/r/sashiko-outbox-204135@kernel.org/
Signed-off-by: James Hilliard <james.hilliard1 at gmail.com>
---
 drivers/mtd/nand/raw/r852.c | 43 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/nand/raw/r852.c b/drivers/mtd/nand/raw/r852.c
index 92c47d351c27..12877dd14683 100644
--- a/drivers/mtd/nand/raw/r852.c
+++ b/drivers/mtd/nand/raw/r852.c
@@ -12,6 +12,7 @@
 #include <linux/jiffies.h>
 #include <linux/workqueue.h>
 #include <linux/interrupt.h>
+#include <linux/lockdep.h>
 #include <linux/pci.h>
 #include <linux/pci_ids.h>
 #include <linux/delay.h>
@@ -124,11 +125,11 @@ static void r852_dma_enable(struct r852_device *dev)
 }
 
 /*
- * Disable dma, called from the interrupt handler, which specifies
- * success of the operation via 'error' argument
+ * Disable DMA under irqlock and record the result of the active transfer.
  */
 static void r852_dma_done(struct r852_device *dev, int error)
 {
+	lockdep_assert_held(&dev->irqlock);
 	WARN_ON(dev->dma_stage == 0);
 
 	r852_write_reg_dword(dev, R852_DMA_IRQ_STA,
@@ -145,10 +146,11 @@ static void r852_dma_done(struct r852_device *dev, int error)
 	dev->dma_error = error;
 	dev->dma_stage = 0;
 
-	if (dev->phys_dma_addr && dev->phys_dma_addr != dev->phys_bounce_buffer)
+	if (dev->phys_dma_addr != dev->phys_bounce_buffer)
 		dma_unmap_single(&dev->pci_dev->dev, dev->phys_dma_addr,
 			R852_DMA_LEN,
 			dev->dma_dir ? DMA_FROM_DEVICE : DMA_TO_DEVICE);
+	dev->phys_dma_addr = 0;
 }
 
 /*
@@ -175,19 +177,8 @@ static void r852_do_dma(struct r852_device *dev, uint8_t *buf, int do_read)
 	unsigned long flags;
 	int error;
 
-	dev->dma_error = 0;
-
-	/* Set dma direction */
-	dev->dma_dir = do_read;
-	dev->dma_stage = 1;
-	reinit_completion(&dev->dma_done);
-
 	dbg_verbose("doing dma %s ", do_read ? "read" : "write");
 
-	/* Set initial dma state: for reading first fill on board buffer,
-	  from device, for writes first fill the buffer  from memory*/
-	dev->dma_state = do_read ? DMA_INTERNAL : DMA_MEMORY;
-
 	/* if incoming buffer is not page aligned, we should do bounce */
 	if ((unsigned long)buf & (R852_DMA_LEN-1))
 		bounce = 1;
@@ -209,16 +200,26 @@ static void r852_do_dma(struct r852_device *dev, uint8_t *buf, int do_read)
 
 	/* Enable DMA */
 	spin_lock_irqsave(&dev->irqlock, flags);
+	r852_write_reg_dword(dev, R852_DMA_IRQ_STA, R852_DMA_IRQ_MASK);
+	dev->dma_error = 0;
+	dev->dma_dir = do_read;
+	dev->dma_stage = 1;
+	dev->dma_state = do_read ? DMA_INTERNAL : DMA_MEMORY;
+	reinit_completion(&dev->dma_done);
 	r852_dma_enable(dev);
 	spin_unlock_irqrestore(&dev->irqlock, flags);
 
 	/* Wait till complete */
 	error = r852_dma_wait(dev);
 
-	if (error) {
+	spin_lock_irqsave(&dev->irqlock, flags);
+	if (error && dev->dma_stage)
 		r852_dma_done(dev, error);
+	error = dev->dma_error;
+	spin_unlock_irqrestore(&dev->irqlock, flags);
+
+	if (error)
 		return;
-	}
 
 	if (do_read && bounce)
 		memcpy((void *)buf, dev->bounce_buffer, R852_DMA_LEN);
@@ -766,6 +767,10 @@ static irqreturn_t r852_irq(int irq, void *data)
 
 		ret = IRQ_HANDLED;
 
+		/* Ignore late interrupts after a timeout. */
+		if (dev->dma_stage == 0)
+			goto out;
+
 		if (dma_status & R852_DMA_IRQ_ERROR) {
 			dbg("received dma error IRQ");
 			r852_dma_done(dev, -EIO);
@@ -773,12 +778,6 @@ static irqreturn_t r852_irq(int irq, void *data)
 			goto out;
 		}
 
-		/* received DMA interrupt out of nowhere? */
-		WARN_ON_ONCE(dev->dma_stage == 0);
-
-		if (dev->dma_stage == 0)
-			goto out;
-
 		/* done device access */
 		if (dev->dma_state == DMA_INTERNAL &&
 				(dma_status & R852_DMA_IRQ_INTERNAL)) {

---
base-commit: 30daaf3e7897b4a8d9b72e5b0c86dd6e2a4d7d85
change-id: 20261108-submit-r852-dma-timeout-race-v1-9b256d202dce

Best regards,
--  
James Hilliard <james.hilliard1 at gmail.com>