The DMA core requires drivers to set dma_device.device_release so that
the container structure is only freed after all references to it have
been dropped (see the comment above dma_async_device_register()).

This driver violated that contract: fdev was devm_kzalloc()'d with no
device_release callback.  If a client still held a channel reference
when the driver was unbound, dma_device_release() would eventually
run on freed memory, causing a use-after-free.

Fix by allocating fdev with kzalloc_obj(), adding
fsldma_device_release() to free it, and setting device_release.
fsldma_of_remove() now saves channel pointers and frees IRQs before
calling dma_async_device_unregister(), since fdev may be freed by
the release callback inside that call.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <[email protected]>
---
 drivers/dma/fsldma.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 1ba10d065278..43d817f6ded1 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1219,6 +1219,8 @@ static void fsl_dma_chan_remove(struct fsldma_chan *chan)
        kfree(chan);
 }
 
+static void fsldma_device_release(struct dma_device *dma_dev);
+
 static int fsldma_of_probe(struct platform_device *op)
 {
        struct fsldma_device *fdev;
@@ -1257,6 +1259,7 @@ static int fsldma_of_probe(struct platform_device *op)
        fdev->common.device_issue_pending = fsl_dma_memcpy_issue_pending;
        fdev->common.device_config = fsl_dma_device_config;
        fdev->common.device_terminate_all = fsl_dma_device_terminate_all;
+       fdev->common.device_release = fsldma_device_release;
        fdev->common.dev = &op->dev;
 
        fdev->common.src_addr_widths = FSL_DMA_BUSWIDTHS;
@@ -1316,19 +1319,33 @@ static int fsldma_of_probe(struct platform_device *op)
        return err;
 }
 
+static void fsldma_device_release(struct dma_device *dma_dev)
+{
+       struct fsldma_device *fdev = container_of(dma_dev, struct fsldma_device,
+                                                 common);
+       kfree(fdev);
+}
+
 static void fsldma_of_remove(struct platform_device *op)
 {
-       struct fsldma_device *fdev;
+       struct fsldma_device *fdev = platform_get_drvdata(op);
+       struct fsldma_chan *chans[FSL_DMA_MAX_CHANS_PER_DEVICE];
        unsigned int i;
 
-       fdev = platform_get_drvdata(op);
-       dma_async_device_unregister(&fdev->common);
+       for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++)
+               chans[i] = fdev->chan[i];
 
        fsldma_free_irqs(fdev);
 
+       /*
+        * fdev may be freed by fsldma_device_release inside this call;
+        * use saved copies of the channel pointers afterwards.
+        */
+       dma_async_device_unregister(&fdev->common);
+
        for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
-               if (fdev->chan[i])
-                       fsl_dma_chan_remove(fdev->chan[i]);
+               if (chans[i])
+                       fsl_dma_chan_remove(chans[i]);
        }
        irq_dispose_mapping(fdev->irq);
 
-- 
2.54.0


Reply via email to