Re: [PATCH v3] 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:

Pre-existing issues:
- [High] The IRQ collection loop in nbpf_probe() will infinite loop and permanently hang the kernel during probe if the device tree defines fewer than 9 interrupts.
- [High] Shared IRQs are requested before enabling the device clock, which can cause a bus fault if a shared interrupt fires early.
- [High] nbpf_desc_page_alloc() leaks a DMA-capable page and passes an incorrect size to dma_unmap_single() on mapping failures.
--

commit 4b0854f80b4f9ee26435b82925b2ce35ade384d6
Author: Christian Taedcke <christian.taedcke@weidmueller.com>

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

This commit fixes an issue in the IRQ assignment loop for nbpfaxi channels.
When the error IRQ is not the last entry in the buffer, the previous loop
terminated early, leaving the last channel without an IRQ. The update iterates
over all collected IRQs instead of the channel count to fix this.

> diff --git a/drivers/dma/nbpfaxi.c b/drivers/dma/nbpfaxi.c
> index 05d7321629cc8..b1f06f0bd0d50 100644
> --- a/drivers/dma/nbpfaxi.c
> +++ b/drivers/dma/nbpfaxi.c
> @@ -1374,14 +1374,14 @@ 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))
> +					continue;
> +				if (chan >= nbpf->chan + num_channels)
>  					return -EINVAL;
>  				chan->irq = irqbuf[i];
> +				chan++;
>  			}
>  		} else {
>  			/* 2 IRQs and more than one channel */

While reviewing this patch, I noticed a few pre-existing issues in this driver.

[Severity: High]
This is a pre-existing issue, but does the IRQ collection loop earlier in
nbpf_probe() have a potential infinite loop?

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 the device tree defines fewer than 9 interrupts, platform_get_irq_optional()
will return -ENXIO, which bypasses the loop break condition. Because the irqs
counter never increments in this case, the i variable increments infinitely,
hanging the kernel during probe.


[Severity: High]
This is a pre-existing issue, but could this shared IRQ request cause an early
interrupt leading to a bus fault?

drivers/dma/nbpfaxi.c:nbpf_probe() {
    ...
	ret = devm_request_irq(dev, eirq, nbpf_err_irq,
			       IRQF_SHARED, "dma error", nbpf);
    ...
	ret = clk_prepare_enable(nbpf->clk);
    ...
}

The error IRQ is requested here as shared, allowing it to fire immediately
if another device raises the line. Since the device clock is not enabled
until much later, is there a race window where register reads by the early
ISR could crash?


[Severity: High]
This is a pre-existing issue, but is there a memory leak and incorrect sizing
in the DMA mapping error path of nbpf_desc_page_alloc()?

drivers/dma/nbpfaxi.c:nbpf_desc_page_alloc() {
    ...
unmap_error:
	while (i--) {
		ldesc--; hwdesc--;

		dma_unmap_single(dchan->device->dev, ldesc->hwdesc_dma_addr,
				 sizeof(hwdesc), DMA_TO_DEVICE);
	}

	return -ENOMEM;
}

In this error path, sizeof(hwdesc) evaluates to the pointer size instead of
the actual struct size. Additionally, the page allocated for dpage doesn't
appear to be freed before returning, leaking a DMA-capable page.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-upstreaming-nbpfaxi-v1-v3-1-24f7f9aa102f@weidmueller.com?part=1