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 <vincent.whitchurch@xxxxxxxx> --- 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