Re: [PATCH] dmaengine: fl1-edma: Add error handling for devm_kasprintf

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver masks `-EPROBE_DEFER` from both `platform_get_irq()` and `platform_get_irq_optional()`.
- [High] Missing explicit cleanup of `txirq_16_31` in `fsl_edma_irq_exit()` creates a race condition during module removal.
- [High] Use of `BIT()` instead of `BIT_ULL()` with a `u64` bitmask causes shift out of bounds on 32-bit platforms.
--

commit dcca75c04a71e337392f5a5707b96105b925f750
Author: Griffin Kroah-Hartman <griffin@kroah.com>

dmaengine: fl1-edma: Add error handling for devm_kasprintf
    
Add error handling statement to fls_edma3_irq_init() for the
devm_kasprintf call.

> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> index 36155ab1602a9..d9fb717b5b53c 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
> @@ -414,6 +414,8 @@ static int fsl_edma3_irq_init(struct platform_device *pdev, struct fsl_edma_engi
>  
>  		errirq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-err",
>  					     dev_name(&pdev->dev));
> +		if (!errirq_name)
> +			return -ENOMEM;
>  
>  		ret = devm_request_irq(&pdev->dev, fsl_edma->errirq, fsl_edma3_err_handler_shared,
>  				       0, errirq_name, fsl_edma);

[Severity: High]
This isn't a bug introduced by this patch, but while modifying 
fsl_edma3_irq_init(), does the driver mask -EPROBE_DEFER from 
platform_get_irq() and platform_get_irq_optional()?

drivers/dma/fsl-edma-main.c:fsl_edma3_irq_init() {
    ...
		fsl_chan->txirq = platform_get_irq(pdev, i);
		if (fsl_chan->txirq < 0)
			return  -EINVAL;
    ...
		fsl_edma->errirq = platform_get_irq_optional(pdev, fsl_edma->n_chans);
		if (fsl_edma->errirq < 0)
			return 0; /* dts miss err irq, treat as no err irq case */
    ...
}

If either of these functions return -EPROBE_DEFER because the IRQ controller
is not yet ready, discarding the error code converts a recoverable probe
deferral into a permanent initialization failure or a silent missing interrupt.

[Severity: High]
This is a pre-existing issue, but can the chan_masked check result in an 
out-of-bounds shift on 32-bit platforms?

drivers/dma/fsl-edma-main.c:fsl_edma3_irq_init() {
    ...
	for (i = 0; i < fsl_edma->n_chans; i++) {
        ...
		if (fsl_edma->chan_masked & BIT(i))
			continue;
    ...
}

Since chan_masked is a u64, using BIT(i) instead of BIT_ULL(i) produces an
unsigned long.

On 32-bit architectures, this evaluates incorrectly for channels 32-63,
proceeding with initialization for channels that should have been masked off.

[Severity: High]
This is a pre-existing issue, but is there a race condition during module 
removal due to missing cleanup of txirq_16_31?

drivers/dma/fsl-edma-main.c:fsl_edma_remove() {
    ...
	fsl_edma_irq_exit(pdev, fsl_edma);
	fsl_edma_cleanup_vchan(&fsl_edma->dma_dev);
    ...
}

During fsl_edma_remove(), the driver attempts to silence interrupts
by calling devm_free_irq() in fsl_edma_irq_exit():

drivers/dma/fsl-edma-main.c:fsl_edma_irq_exit() {
    ...
    (missing explicit free of txirq_16_31)
    ...
}

If fsl_edma_irq_exit() fails to free fsl_edma->txirq_16_31, an interrupt
can fire for channels 16-31 while fsl_edma_cleanup_vchan() is tearing down
the virtual channels.

Could this cause the handler to execute on partially destroyed state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/2026070605-frying-fling-b9c5@gregkh?part=1