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

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

[Christian Taedcke via B4 Relay]

From: Christian Taedcke <christian.taedcke@weidmueller.com>

When one irq is used for errors and each channel gets a dedicated irq,
the total number of irqs is num_channels + 1. If the error irq is not
the last entry in irqbuf[] but an earlier one, the loop assigning
per-channel irqs terminates one iteration too early and the last
channel is left without an irq.

Iterate over all collected irqs instead of num_channels so the
error-irq skip does not shorten the effective channel count.

Fixes: 188c6ba1dd92 ("dmaengine: nbpfaxi: Fix memory corruption in probe()")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
---
Changes in v3:
- Guard against out-of-bound writes to chan in case of an invalid eirq.
- Link to v2: https://patch.msgid.link/20260702-upstreaming-nbpfaxi-v1-v2-1-e6d6b178a278@weidmueller.com

Changes in v2:
- Advance chan only when assigning a real irq to fix out-of-bounds
  memory access.
- Remove now redundant ARRAY_SIZE(irqbuf) check.
- Link to v1: https://patch.msgid.link/20260702-upstreaming-nbpfaxi-v1-v1-1-fd8ea8830cea@weidmueller.com

To: christian.taedcke-oss@weidmueller.com
To: Vinod Koul <vkoul@kernel.org>
To: Frank Li <Frank.Li@kernel.org>
To: Dan Carpenter <error27@gmail.com>
Cc: dmaengine@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/dma/nbpfaxi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/nbpfaxi.c b/drivers/dma/nbpfaxi.c
index 05d7321629cc..b1f06f0bd0d5 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 */

---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260702-upstreaming-nbpfaxi-v1-0364e355af85

Best regards,
--  
Christian Taedcke <christian.taedcke@weidmueller.com>

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

[sashiko-bot]

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