* [PATCH v3] dmaengine: nbpfaxi: Fix setting channel irqs in probe()
@ 2026-07-03 7:56 ` Christian Taedcke
0 siblings, 0 replies; 8+ messages in thread
From: Christian Taedcke via B4 Relay @ 2026-07-03 7:56 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Dan Carpenter, christian.taedcke-oss
Cc: dmaengine, linux-kernel, stable, Christian Taedcke
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>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3] dmaengine: nbpfaxi: Fix setting channel irqs in probe()
@ 2026-07-03 7:56 ` Christian Taedcke
0 siblings, 0 replies; 8+ messages in thread
From: Christian Taedcke @ 2026-07-03 7:56 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Dan Carpenter, christian.taedcke-oss
Cc: dmaengine, linux-kernel, stable, Christian Taedcke
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>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] dmaengine: nbpfaxi: Fix setting channel irqs in probe()
2026-07-03 7:56 ` Christian Taedcke
(?)
@ 2026-07-03 8:10 ` sashiko-bot
-1 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-07-03 8:10 UTC (permalink / raw)
To: Christian Taedcke; +Cc: Frank.Li, dmaengine, vkoul
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] dmaengine: nbpfaxi: Fix setting channel irqs in probe()
2026-07-03 7:56 ` Christian Taedcke
(?)
(?)
@ 2026-07-09 10:38 ` Dan Carpenter
2026-07-14 7:44 ` Taedcke, Christian
2026-07-14 11:58 ` Taedcke, Christian
-1 siblings, 2 replies; 8+ messages in thread
From: Dan Carpenter @ 2026-07-09 10:38 UTC (permalink / raw)
To: christian.taedcke
Cc: Vinod Koul, Frank Li, christian.taedcke-oss, dmaengine,
linux-kernel, stable
On Fri, Jul 03, 2026 at 09:56:12AM +0200, Christian Taedcke via B4 Relay wrote:
> 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)
Prefer my check, but sure...
It's pretty annoying that sashiko bot doesn't CC the CC list.
regards,
dan carpenter
> return -EINVAL;
> chan->irq = irqbuf[i];
> + chan++;
> }
> } else {
> /* 2 IRQs and more than one channel */
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] dmaengine: nbpfaxi: Fix setting channel irqs in probe()
2026-07-09 10:38 ` Dan Carpenter
@ 2026-07-14 7:44 ` Taedcke, Christian
2026-07-14 7:48 ` Dan Carpenter
2026-07-14 11:58 ` Taedcke, Christian
1 sibling, 1 reply; 8+ messages in thread
From: Taedcke, Christian @ 2026-07-14 7:44 UTC (permalink / raw)
To: Dan Carpenter, christian.taedcke
Cc: Vinod Koul, Frank Li, dmaengine, linux-kernel, stable
On 7/9/2026 12:38 PM, Dan Carpenter wrote:
> On Fri, Jul 03, 2026 at 09:56:12AM +0200, Christian Taedcke via B4 Relay wrote:
>> 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)
>
> Prefer my check, but sure...
Thank you for your review, i send a new version where i try to use your preferred check.
>
> It's pretty annoying that sashiko bot doesn't CC the CC list.
Is there anything i can do to improve this? Should i reply to the sashiko bot comments and also send this to the CC list?
>
> regards,
> dan carpenter
>
>> return -EINVAL;
>> chan->irq = irqbuf[i];
>> + chan++;
>> }
>> } else {
>> /* 2 IRQs and more than one channel */
>
Regards,
Christian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] dmaengine: nbpfaxi: Fix setting channel irqs in probe()
2026-07-14 7:44 ` Taedcke, Christian
@ 2026-07-14 7:48 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2026-07-14 7:48 UTC (permalink / raw)
To: Taedcke, Christian
Cc: christian.taedcke, Vinod Koul, Frank Li, dmaengine, linux-kernel,
stable
On Tue, Jul 14, 2026 at 09:44:22AM +0200, Taedcke, Christian wrote:
>
>
> On 7/9/2026 12:38 PM, Dan Carpenter wrote:
> > On Fri, Jul 03, 2026 at 09:56:12AM +0200, Christian Taedcke via B4 Relay wrote:
> >> 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)
> >
> > Prefer my check, but sure...
>
> Thank you for your review, i send a new version where i try to use your preferred check.
> >
> > It's pretty annoying that sashiko bot doesn't CC the CC list.
>
> Is there anything i can do to improve this? Should i reply to the sashiko bot comments and also send this to the CC list?
>
Not at all. Just random grumbling.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] dmaengine: nbpfaxi: Fix setting channel irqs in probe()
2026-07-09 10:38 ` Dan Carpenter
2026-07-14 7:44 ` Taedcke, Christian
@ 2026-07-14 11:58 ` Taedcke, Christian
2026-07-14 12:07 ` Dan Carpenter
1 sibling, 1 reply; 8+ messages in thread
From: Taedcke, Christian @ 2026-07-14 11:58 UTC (permalink / raw)
To: Dan Carpenter, christian.taedcke
Cc: Vinod Koul, Frank Li, dmaengine, linux-kernel, stable
On 7/9/2026 12:38 PM, Dan Carpenter wrote:
> On Fri, Jul 03, 2026 at 09:56:12AM +0200, Christian Taedcke via B4 Relay wrote:
>> 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)
>
> Prefer my check, but sure...
I tested changing the condition back to check for i. But after a few different approaches, i think the check in v3 (chan >= nbpf->chan + num_channels) is more robust.
It handles the following cases well:
1. eirq is the last entry in irqbuf[]
2. eirq is not in irqbuf[] (which is not expected)
This check also makes it clear that the write destination is verified.
-> i would prefer to keep the v3 patch as is.
>
> It's pretty annoying that sashiko bot doesn't CC the CC list.
>
> regards,
> dan carpenter
>
>> return -EINVAL;
>> chan->irq = irqbuf[i];
>> + chan++;
>> }
>> } else {
>> /* 2 IRQs and more than one channel */
>
Regards,
Christian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] dmaengine: nbpfaxi: Fix setting channel irqs in probe()
2026-07-14 11:58 ` Taedcke, Christian
@ 2026-07-14 12:07 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2026-07-14 12:07 UTC (permalink / raw)
To: Taedcke, Christian
Cc: christian.taedcke, Vinod Koul, Frank Li, dmaengine, linux-kernel,
stable
On Tue, Jul 14, 2026 at 01:58:53PM +0200, Taedcke, Christian wrote:
>
>
> On 7/9/2026 12:38 PM, Dan Carpenter wrote:
> > On Fri, Jul 03, 2026 at 09:56:12AM +0200, Christian Taedcke via B4 Relay wrote:
> >> 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)
> >
> > Prefer my check, but sure...
>
> I tested changing the condition back to check for i. But after a few different approaches, i think the check in v3 (chan >= nbpf->chan + num_channels) is more robust.
>
> It handles the following cases well:
> 1. eirq is the last entry in irqbuf[]
> 2. eirq is not in irqbuf[] (which is not expected)
>
> This check also makes it clear that the write destination is verified.
>
> -> i would prefer to keep the v3 patch as is.
>
Ah, yeah. You're right. v3 is good.
Reviewed-by: Dan Carpenter <error27@gmail.com>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-07-14 12:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-03 7:56 [PATCH v3] dmaengine: nbpfaxi: Fix setting channel irqs in probe() Christian Taedcke via B4 Relay
2026-07-03 7:56 ` Christian Taedcke
2026-07-03 8:10 ` sashiko-bot
2026-07-09 10:38 ` Dan Carpenter
2026-07-14 7:44 ` Taedcke, Christian
2026-07-14 7:48 ` Dan Carpenter
2026-07-14 11:58 ` Taedcke, Christian
2026-07-14 12:07 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.