2023-07-17 13:15:06

by Vincent Whitchurch

Subject: [PATCH 0/2] dmaengine: Use-after-free fix and dummy DMAC

This series has a fix for a use-after-free in the DMA engine framework
and adds a dummy CPU-based "DMA" controller driver which can be used for
testing the DMA engine framework and clients on systems without a real
DMA engine, such as under KVM.

---
Vincent Whitchurch (2):
dmaengine: Fix use-after-free on release
dmaengine: Add dummy DMA controller driver

drivers/dma/Kconfig | 14 +++
drivers/dma/Makefile | 1 +
drivers/dma/dmaengine.c | 30 ++++--
drivers/dma/dummy-dmac.c | 258 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 294 insertions(+), 9 deletions(-)
---
base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
change-id: 20230717-dummy-dmac-9a2e7e3ddb29

Best regards,
--
Vincent Whitchurch <[email protected]>

2023-07-17 13:17:42

by Vincent Whitchurch

Subject: [PATCH 1/2] dmaengine: Fix use-after-free on release

Many dmaengine drivers cannot be safely unbound while being used since
they free their device structures in their platform driver ->remove()
callbacks or with devm. In order to avoid this, the framework allows
drivers to implement ->device_release() and free these structures there
instead. However, there are use-after-frees in the framework even when
->device_release() is implemented.

For example, the following sequence of commands with the upcoming
dummy-dmac driver triggers a KASAN splat without this patch:

# insmod dummy-dmac.ko
# insmod dmatest.ko iterations=1 wait=1 run=1
# echo dummy-dmac > /sys/bus/platform/drivers/dummy-dmac/unbind
# rmmod dmatest

==================================================================
BUG: KASAN: slab-use-after-free in dma_chan_put (drivers/dma/dmaengine.c:517)
Read of size 8 at addr ffff888008b00c78 by task rmmod/1063

Call Trace:
dma_chan_put (drivers/dma/dmaengine.c:517)
dma_release_channel (drivers/dma/dmaengine.c:910)
cleanup_module (drivers/dma/dmatest.c:1184) dmatest
...

Allocated by task 859:
kmalloc_trace (mm/slab_common.c:1082)
dummy_dmac_probe (drivers/dma/dummy-dmac.c:171) dummy_dmac
platform_probe (drivers/base/platform.c:1405)
...

Freed by task 1063:
kfree (mm/slab_common.c:1035)
dummy_dmac_release (drivers/dma/dummy-dmac.c:160) dummy_dmac
dma_device_put (include/linux/kref.h:65 drivers/dma/dmaengine.c:437)
dma_chan_put (drivers/dma/dmaengine.c:517)
dma_release_channel (drivers/dma/dmaengine.c:910)
cleanup_module (drivers/dma/dmatest.c:1184) dmatest
...
==================================================================

Fix this by making the framework not touch the dev and client structures
after the last call to dma_device_put().

Signed-off-by: Vincent Whitchurch <[email protected]>
---
drivers/dma/dmaengine.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 826b98284fa1..86b892df8ea1 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -485,13 +485,7 @@ static int dma_chan_get(struct dma_chan *chan)
return ret;
}

-/**
- * dma_chan_put - drop a reference to a DMA channel's parent driver module
- * @chan: channel to release
- *
- * Must be called under dma_list_mutex.
- */
-static void dma_chan_put(struct dma_chan *chan)
+static void __dma_chan_put(struct dma_chan *chan)
{
/* This channel is not in use, bail out */
if (!chan->client_count)
@@ -512,9 +506,22 @@ static void dma_chan_put(struct dma_chan *chan)
chan->router = NULL;
chan->route_data = NULL;
}
+}
+
+/**
+ * dma_chan_put - drop a reference to a DMA channel's parent driver module
+ * @chan: channel to release
+ *
+ * Must be called under dma_list_mutex.
+ */
+static void dma_chan_put(struct dma_chan *chan)
+{
+ struct module *owner = dma_chan_to_owner(chan);
+
+ __dma_chan_put(chan);

dma_device_put(chan->device);
- module_put(dma_chan_to_owner(chan));
+ module_put(owner);
}

enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)
@@ -902,10 +909,12 @@ EXPORT_SYMBOL_GPL(dma_request_chan_by_mask);

void dma_release_channel(struct dma_chan *chan)
{
+ struct module *owner = dma_chan_to_owner(chan);
+
mutex_lock(&dma_list_mutex);
WARN_ONCE(chan->client_count != 1,
"chan reference count %d != 1\n", chan->client_count);
- dma_chan_put(chan);
+ __dma_chan_put(chan);
/* drop PRIVATE cap enabled by __dma_request_channel() */
if (--chan->device->privatecnt == 0)
dma_cap_clear(DMA_PRIVATE, chan->device->cap_mask);
@@ -922,6 +931,9 @@ void dma_release_channel(struct dma_chan *chan)
kfree(chan->dbg_client_name);
chan->dbg_client_name = NULL;
#endif
+
+ dma_device_put(chan->device);
+ module_put(owner);
mutex_unlock(&dma_list_mutex);
}
EXPORT_SYMBOL_GPL(dma_release_channel);

--
2.34.1