Re: [PATCHv3] dmaengine: ti: omap-dma: turn lch_map into a flexible array

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Freeing the shared IRQ handler before properly disabling hardware interrupts can cause an interrupt storm.
- [High] The late error path in probe frees channels while the shared IRQ handler is active, leading to UAF.
- [High] Destroying the descriptor pool before killing virt-dma tasklets causes a UAF when tasklets free memory.
--

commit b8aab98ec296892e7aaf7bec47814ddd0493ced7
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: ti: omap-dma: turn lch_map into a flexible array

Convert the separately-allocated lch_map pointer array to a C99
flexible array member at the end of struct omap_dmadev and annotate it
with __counted_by(lch_count). This patch also adds fixes for a missing
return, a needs_busy_check, and a free_irq ordering issue.

> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> index 55ece7fd0d99a..7c46be2755be7 100644
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c
[ ... ]
> @@ -1828,6 +1828,7 @@ static int omap_dma_probe(struct platform_device *pdev)
>  			if (od->ll123_supported)
>  				dma_pool_destroy(od->desc_pool);
>  			omap_dma_free(od);
> +			return rc;
>  		}
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does destroying the descriptor
pool before freeing the channels lead to a use-after-free?

omap_dma_free() kills the virt-dma tasklets:

drivers/dma/ti/omap-dma.c:omap_dma_free() {
	while (!list_empty(&od->ddev.channels)) {
        ...
		tasklet_kill(&c->vc.task);
		kfree(c);
	}
}

Because tasklets can run concurrently until they are explicitly killed, could a
pending tasklet invoke omap_dma_desc_free() and attempt to return memory to the
already destroyed od->desc_pool?

[Severity: High]
This is also a pre-existing issue, but is it safe to free the channels while
the shared IRQ handler is still active?

The IRQ handler was registered with devm_request_irq() earlier in probe(), so
it remains active during this error path since devres hasn't cleaned it up yet.

If a shared interrupt fires, could omap_dma_irq() dereference the freed channel
pointers in od->lch_map?

[ ... ]
> @@ -1852,17 +1853,18 @@ static void omap_dma_remove(struct platform_device *pdev)
>  	struct omap_dmadev *od = platform_get_drvdata(pdev);
>  	int irq;
>  
> -	if (od->cfg->may_lose_context)
> +	if (od->cfg->needs_busy_check || od->cfg->may_lose_context)
>  		cpu_pm_unregister_notifier(&od->nb);
>  
>  	if (pdev->dev.of_node)
>  		of_dma_controller_free(pdev->dev.of_node);
>  
> -	irq = platform_get_irq(pdev, 1);
> -	devm_free_irq(&pdev->dev, irq, od);
> -
>  	dma_async_device_unregister(&od->ddev);
>  
> +	irq = platform_get_irq(pdev, 1);
> +	if (irq > 0)
> +		devm_free_irq(&pdev->dev, irq, od);
> +
>  	if (!omap_dma_legacy(od)) {
>  		/* Disable all interrupts */
>  		omap_dma_glbl_write(od, IRQENABLE_L0, 0);

[Severity: High]
This isn't a bug introduced by this patch, but does freeing the shared IRQ
handler before disabling hardware interrupts create a window for an interrupt
storm?

Since the IRQ is requested with IRQF_SHARED, if the handler is freed before
the hardware stops asserting the interrupt, the unhandled interrupt might cause
the kernel to permanently disable the shared IRQ line.

Additionally, probe() configures IRQENABLE_L1 to enable interrupts, but this
code attempts to disable them by writing to IRQENABLE_L0. Does this mean the
hardware interrupts may not actually be disabled here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528233507.305178-1-rosenp@gmail.com?part=1