Re: [PATCH v2 2/2] dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Use-After-Free in dmaengine_put() during channel iteration.
- [Critical] Use-After-Free in dma_release_channel() after forceful DMA provider unbind.
--

commit 5c65161b4e60f0575ac687342210c12a79a7958e
Author: Shivank Garg <shivankg@amd.com>

dmaengine: fix use-after-free in dma_chan_put() and dma_release_channel()

This commit resolves use-after-free bugs in the DMA engine channel
management by caching the module owner prior to releasing channel
references and reordering the cleanup in dma_release_channel().

Link: https://sashiko.dev/#/patchset/20260518-dmaengine-kref-fix-v1-1-4d6125048fb7@amd.com

> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 605bfa477a004..9c4e206f24686 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
[ ... ]
> @@ -518,7 +521,7 @@ static void dma_chan_put(struct dma_chan *chan)
>  	/* This channel is not in use anymore, drop the device ref */
>  	if (!chan->client_count)
>  		dma_device_put(chan->device);
> -	module_put(dma_chan_to_owner(chan));
> +	module_put(owner);
>  }

[Severity: Critical]
This is a pre-existing issue, but can this lead to a use-after-free
regression during channel iteration?

If list_for_each_entry() iterates over a device's channels in
dmaengine_put():

dmaengine_put() {
    list_for_each_entry(chan, &device->channels, device_node)
        dma_chan_put(chan);
}

When dma_chan_put() drops the final client_count reference, it triggers
dma_device_put(), which can free the device and its embedded channels via
dma_device_release().

When execution returns to the loop macro, evaluating chan = list_next_entry()
would dereference the freed chan->device_node.next pointer. Even
list_for_each_entry_safe() appears insufficient here since the list head is
also inside the freed device.

[ ... ]
> @@ -924,6 +926,7 @@ void dma_release_channel(struct dma_chan *chan)
>  	kfree(chan->dbg_client_name);
>  	chan->dbg_client_name = NULL;
>  #endif
> +	dma_chan_put(chan);

[Severity: Critical]
This is a pre-existing issue, but can this code trigger a use-after-free
regression if a DMA provider is forcefully unbound?

If a provider is unbound via sysfs while clients hold channel references,
dma_async_device_unregister() iterates over channels and unregisters them:

__dma_async_device_channel_unregister() {
    ...
    device_unregister(&chan->dev->device);
}

This drops the reference to chan->dev and frees it without setting the
pointer to NULL.

Later, when the client driver closes the channel via dma_release_channel(),
it calls sysfs_remove_link(&chan->dev->device.kobj, DMA_SLAVE_NAME) before
reaching the dma_chan_put() call.

Will this dereference the dangling chan->dev pointer?

>  	mutex_unlock(&dma_list_mutex);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-dmaengine-kref-fix-v2-0-3df60afac01d@amd.com?part=2