Re: [PATCH v2] dmaengine: nbpfaxi: Fix setting channel irqs in probe()

[sashiko-bot@kernel.org]

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

New issues:
- [High] Heap buffer overflow in channel IRQ assignment loop if the 'error' IRQ is not in `irqbuf`.

Pre-existing issues:
- [High] Infinite loop in `nbpf_probe()` if the device tree provides fewer than 9 interrupts.
- [High] Use-after-free on probe error paths due to missing `tasklet_kill()`.
--

commit 20180c724c3634307b1f833b241c0ea7d9e4c009
Author: Christian Taedcke <christian.taedcke@weidmueller.com>

dmaengine: nbpfaxi: Fix setting channel irqs in probe()

This commit changes the per-channel IRQ assignment loop to iterate over the
total number of collected IRQs instead of the channel count. This ensures
that skipping the error IRQ does not prematurely terminate the loop and leave
the last channel without an assigned IRQ.

> diff --git a/drivers/dma/nbpfaxi.c b/drivers/dma/nbpfaxi.c
> index 05d7321629cc8..bcfab62a71d75 100644
> --- a/drivers/dma/nbpfaxi.c
> +++ b/drivers/dma/nbpfaxi.c
> @@ -1374,14 +1374,12 @@ static int nbpf_probe(struct platform_device *pdev)
>  		if (irqs == num_channels + 1) {
>  			struct nbpf_channel *chan;
>  
> -			for (i = 0, chan = nbpf->chan; i < num_channels;
> -			     i++, chan++) {
> +			for (i = 0, chan = nbpf->chan; i < irqs; i++) {
>  				/* Skip the error IRQ */
>  				if (irqbuf[i] == eirq)
> -					i++;
> -				if (i >= ARRAY_SIZE(irqbuf))
> -					return -EINVAL;
> +					continue;
>  				chan->irq = irqbuf[i];
> +				chan++;
>  			}

[Severity: High]
If a malformed device tree causes the 'error' interrupt to not match any
of the interrupts in irqbuf, the continue statement will never be reached.
Could this lead to chan being incremented num_channels + 1 times, causing an
out-of-bounds write to the nbpf->chan array on the final iteration?

[Severity: High]
This is a pre-existing issue, but looking earlier in nbpf_probe(),
there appears to be a problem if the device tree provides fewer than 9
interrupts:

drivers/dma/nbpfaxi.c:nbpf_probe() {
	...
	for (i = 0; irqs < ARRAY_SIZE(irqbuf); i++) {
		irq = platform_get_irq_optional(pdev, i);
		if (irq < 0 && irq != -ENXIO)
			return irq;
		if (irq > 0)
			irqbuf[irqs++] = irq;
	}
	...
}

If platform_get_irq_optional() returns -ENXIO, irqs is not incremented.
Would this cause an infinite loop where i increments endlessly because irqs
never reaches ARRAY_SIZE(irqbuf)?

[Severity: High]
This is a pre-existing issue, but I also noticed a potential use-after-free
later in nbpf_probe() during error handling:

drivers/dma/nbpfaxi.c:nbpf_probe() {
	...
	ret = clk_prepare_enable(nbpf->clk);
	if (ret < 0)
		return ret;
	...
}

If clk_prepare_enable() fails, the function returns without killing the
tasklets initialized in nbpf_chan_probe(). If a shared interrupt fires before
the devres framework frees the IRQ and memory, could this schedule a tasklet
that accesses the freed nbpf structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-upstreaming-nbpfaxi-v1-v2-1-e6d6b178a278@weidmueller.com?part=1