Re: [PATCH v2 3/4] dmaengine: dma-axi-dmac: fix use-after-free on unbind

[Frank Li <Frank.li@nxp.com>]

On Fri, Mar 27, 2026 at 04:58:40PM +0000, Nuno Sá wrote:
> The DMA device lifetime can extend beyond the platform driver unbind if
> DMA channels are still referenced by client drivers. This leads to
> use-after-free when the devm-managed memory is freed on unbind but the
> DMA device callbacks still access it.
>
> Fix this by:
>  - Allocating axi_dmac with kzalloc_obj() instead of devm_kzalloc() so
> its lifetime is not tied to the platform device.
>  - Implementing the device_release callback that so that we can free
> the object when reference count gets to 0 (no users).
>  - Adding an 'unbound' flag protected by the vchan lock that is set
> during driver removal, preventing MMIO accesses after the device has been
> unbound.
>
> While at it, explicitly include spinlock.h given it was missing.
>
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---

Not sure if it similar with
https://lore.kernel.org/dmaengine/20250903-v6-16-topic-sdma-v1-9-ac7bab629e8b@pengutronix.de/

It looks like miss device link between comsumer and provider.

Frank

>  drivers/dma/dma-axi-dmac.c | 70 +++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 60 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
> index 127c3cf80a0e..70d3ad7e7d37 100644
> --- a/drivers/dma/dma-axi-dmac.c
> +++ b/drivers/dma/dma-axi-dmac.c
> @@ -24,6 +24,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
> +#include <linux/spinlock.h>
>
>  #include <dt-bindings/dma/axi-dmac.h>
>
> @@ -174,6 +175,8 @@ struct axi_dmac {
>
>  	struct dma_device dma_dev;
>  	struct axi_dmac_chan chan;
> +
> +	bool unbound;
>  };
>
>  static struct axi_dmac *chan_to_axi_dmac(struct axi_dmac_chan *chan)
> @@ -182,6 +185,11 @@ static struct axi_dmac *chan_to_axi_dmac(struct axi_dmac_chan *chan)
>  		dma_dev);
>  }
>
> +static struct axi_dmac *dev_to_axi_dmac(struct dma_device *dev)
> +{
> +	return container_of(dev, struct axi_dmac, dma_dev);
> +}
> +
>  static struct axi_dmac_chan *to_axi_dmac_chan(struct dma_chan *c)
>  {
>  	return container_of(c, struct axi_dmac_chan, vchan.chan);
> @@ -614,7 +622,12 @@ static int axi_dmac_terminate_all(struct dma_chan *c)
>  	LIST_HEAD(head);
>
>  	spin_lock_irqsave(&chan->vchan.lock, flags);
> -	axi_dmac_write(dmac, AXI_DMAC_REG_CTRL, 0);
> +	/*
> +	 * Only allow the MMIO access if the device is live. Otherwise still
> +	 * go for freeing the descriptors.
> +	 */
> +	if (!dmac->unbound)
> +		axi_dmac_write(dmac, AXI_DMAC_REG_CTRL, 0);
>  	chan->next_desc = NULL;
>  	vchan_get_all_descriptors(&chan->vchan, &head);
>  	list_splice_tail_init(&chan->active_descs, &head);
> @@ -642,9 +655,12 @@ static void axi_dmac_issue_pending(struct dma_chan *c)
>  	if (chan->hw_sg)
>  		ctrl |= AXI_DMAC_CTRL_ENABLE_SG;
>
> -	axi_dmac_write(dmac, AXI_DMAC_REG_CTRL, ctrl);
> -
>  	spin_lock_irqsave(&chan->vchan.lock, flags);
> +	if (dmac->unbound) {
> +		spin_unlock_irqrestore(&chan->vchan.lock, flags);
> +		return;
> +	}
> +	axi_dmac_write(dmac, AXI_DMAC_REG_CTRL, ctrl);
>  	if (vchan_issue_pending(&chan->vchan))
>  		axi_dmac_start_transfer(chan);
>  	spin_unlock_irqrestore(&chan->vchan.lock, flags);
> @@ -1184,6 +1200,14 @@ static int axi_dmac_detect_caps(struct axi_dmac *dmac, unsigned int version)
>  	return 0;
>  }
>
> +static void axi_dmac_release(struct dma_device *dma_dev)
> +{
> +	struct axi_dmac *dmac = dev_to_axi_dmac(dma_dev);
> +
> +	put_device(dma_dev->dev);
> +	kfree(dmac);
> +}
> +
>  static void axi_dmac_tasklet_kill(void *task)
>  {
>  	tasklet_kill(task);
> @@ -1194,16 +1218,27 @@ static void axi_dmac_free_dma_controller(void *of_node)
>  	of_dma_controller_free(of_node);
>  }
>
> +static void axi_dmac_disable(void *__dmac)
> +{
> +	struct axi_dmac *dmac = __dmac;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&dmac->chan.vchan.lock, flags);
> +	dmac->unbound = true;
> +	spin_unlock_irqrestore(&dmac->chan.vchan.lock, flags);
> +	axi_dmac_write(dmac, AXI_DMAC_REG_CTRL, 0);
> +}
> +
>  static int axi_dmac_probe(struct platform_device *pdev)
>  {
>  	struct dma_device *dma_dev;
> -	struct axi_dmac *dmac;
> +	struct axi_dmac *__dmac;
>  	struct regmap *regmap;
>  	unsigned int version;
>  	u32 irq_mask = 0;
>  	int ret;
>
> -	dmac = devm_kzalloc(&pdev->dev, sizeof(*dmac), GFP_KERNEL);
> +	struct axi_dmac *dmac __free(kfree) = kzalloc_obj(struct axi_dmac);
>  	if (!dmac)
>  		return -ENOMEM;
>
> @@ -1251,6 +1286,7 @@ static int axi_dmac_probe(struct platform_device *pdev)
>  	dma_dev->dev = &pdev->dev;
>  	dma_dev->src_addr_widths = BIT(dmac->chan.src_width);
>  	dma_dev->dst_addr_widths = BIT(dmac->chan.dest_width);
> +	dma_dev->device_release = axi_dmac_release;
>  	dma_dev->directions = BIT(dmac->chan.direction);
>  	dma_dev->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
>  	dma_dev->max_sg_burst = 31; /* 31 SGs maximum in one burst */
> @@ -1285,12 +1321,21 @@ static int axi_dmac_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>
> +	/*
> +	 * From this point on, our dmac object has it's lifetime bounded with
> +	 * dma_dev. Will be freed when dma_dev refcount goes to 0. That means,
> +	 * no more automatic kfree(). Also note that dmac is now NULL so we
> +	 * need __dmac.
> +	 */
> +	__dmac = no_free_ptr(dmac);
> +	get_device(&pdev->dev);
> +
>  	/*
>  	 * Put the action in here so it get's done before unregistering the DMA
>  	 * device.
>  	 */
>  	ret = devm_add_action_or_reset(&pdev->dev, axi_dmac_tasklet_kill,
> -				       &dmac->chan.vchan.task);
> +				       &__dmac->chan.vchan.task);
>  	if (ret)
>  		return ret;
>
> @@ -1304,13 +1349,18 @@ static int axi_dmac_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>
> -	ret = devm_request_irq(&pdev->dev, dmac->irq, axi_dmac_interrupt_handler,
> -			       IRQF_SHARED, dev_name(&pdev->dev), dmac);
> +	/* So that we can mark the device as unbound and disable it */
> +	ret = devm_add_action_or_reset(&pdev->dev, axi_dmac_disable, __dmac);
>  	if (ret)
>  		return ret;
>
> -	regmap = devm_regmap_init_mmio(&pdev->dev, dmac->base,
> -		 &axi_dmac_regmap_config);
> +	ret = devm_request_irq(&pdev->dev, __dmac->irq, axi_dmac_interrupt_handler,
> +			       IRQF_SHARED, dev_name(&pdev->dev), __dmac);
> +	if (ret)
> +		return ret;
> +
> +	regmap = devm_regmap_init_mmio(&pdev->dev, __dmac->base,
> +				       &axi_dmac_regmap_config);
>
>  	return PTR_ERR_OR_ZERO(regmap);
>  }
>
> --
> 2.53.0
>