* Re: [PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling
2026-08-18 9:34 [PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling Binbin Zhou
@ 2026-08-18 9:49 ` sashiko-bot
2026-08-18 12:13 ` Huacai Chen
2026-08-18 16:39 ` Frank Li
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-18 9:49 UTC (permalink / raw)
To: Binbin Zhou; +Cc: dmaengine, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The ACPI DMA filter function unconditionally casts and modifies unknown dma_chan pointers, causing memory corruption.
- [High] Activating the IRQ error path exposes a race condition where the DMA engine is unregistered without proper synchronization, and virt-dma tasklets are leaked, causing Use-After-Free bugs.
- [High] The shared interrupt handler unconditionally returns IRQ_HANDLED, breaking spurious interrupt detection.
- [High] A NULL pointer dereference will occur in loongson2_cmc_dma_desc_residue() when querying the status of an idle channel or pending descriptor.
- [High] loongson2_cmc_dma_prep_slave_sg() leaks the allocated descriptor if loongson2_cmc_dma_set_xfer_param() fails.
--
--- Patch [1]: [PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling ---
commit a6570585f5f63a15a2b81f25847b66a84b8e9a42
Author: Binbin Zhou <zhoubinbin@loongson.cn>
dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling
This patch changes the type of irq to int in the loongson2-apb-cmc DMA
driver, allowing proper signed comparison and error handling when
platform_get_irq() returns a negative value.
Closes: https://lore.kernel.org/r/202608152230.p20WPlS0-lkp@intel.com/
> diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> index 1c9a542edc859..969dbc5dabe75 100644
> --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> @@ -90,7 +90,7 @@ struct loongson2_cmc_dma_chan {
> struct dma_slave_config dma_sconfig;
> struct loongson2_cmc_dma_desc *desc;
> u32 id;
> - u32 irq;
> + int irq;
[Severity: High]
This isn't a bug introduced by this patch, but since this patch fixes the
error check for lchan->irq < 0, it enables this error path which exposes
a race condition and a tasklet leak.
In loongson2_cmc_dma_probe(), dmaenginem_async_device_register() is called
before the IRQ setup loop. If platform_get_irq() or devm_request_irq()
fails now, the probe aborts and triggers devres cleanup. If a client
requested a channel in this narrow window, it takes a kref, but devres
will unwind and free the lddev memory anyway, leading to a Use-After-Free.
Also, vchan_init() initializes tasklets, but tasklet_kill() is never
called in the error path or in loongson2_cmc_dma_remove():
loongson2-apb-cmc-dma.c:loongson2_cmc_dma_remove() {
of_dma_controller_free(pdev->dev.of_node);
}
If a tasklet was scheduled, it will execute after the device memory is
freed. Could the IRQ initialization be moved before
dmaenginem_async_device_register(), and tasklet_kill() be added to the
cleanup paths?
[Severity: High]
This is a pre-existing issue, but the shared interrupt handler
unconditionally returns IRQ_HANDLED, breaking spurious interrupt
detection.
loongson2-apb-cmc-dma.c:loongson2_cmc_dma_chan_irq() {
...
scoped_guard(spinlock, &lchan->vchan.lock) {
...
}
return IRQ_HANDLED;
}
Since IRQF_SHARED is used, if the status register reads 0 (no interrupt
generated by this channel), the handler simply drops the lock and returns
IRQ_HANDLED.
Does this falsely claim to handle a spurious interrupt, masking an
interrupt storm from a misbehaving shared device?
[Severity: Critical]
This is a pre-existing issue, but the ACPI DMA filter function
unconditionally casts and modifies unknown dma_chan pointers, causing
memory corruption.
loongson2-apb-cmc-dma.c:loongson2_cmc_dma_acpi_filter() {
struct loongson2_cmc_dma_chan *lchan = to_lmdma_chan(chan);
struct acpi_dma_spec *dma_spec = param;
memset(&lchan->chan_reg, 0,
sizeof(struct loongson2_cmc_dma_chan_reg));
lchan->chan_reg.ccr = dma_spec->chan_id &
LOONGSON2_CMCDMA_STREAM_MASK;
return true;
}
The DMA engine core iterates over all available DMA channels in the system
and passes them to this filter. Blindly casting the generic dma_chan to
loongson2_cmc_dma_chan without verifying ownership corrupts other drivers'
memory via memset(). Could ownership be verified first?
[Severity: High]
This is a pre-existing issue, but a NULL pointer dereference will occur in
loongson2_cmc_dma_desc_residue() when querying the status of an idle
channel.
loongson2-apb-cmc-dma.c:loongson2_cmc_dma_desc_residue() {
...
if (lchan->desc->cyclic && next_sg == 0)
return residue;
...
}
When a caller requests the status of a pending descriptor via
loongson2_cmc_dma_tx_status(), it passes next_sg = 0. If the channel is
idle or has no active descriptor, lchan->desc is NULL. Does this
unconditionally dereference a NULL pointer?
[Severity: High]
This is a pre-existing issue, but loongson2_cmc_dma_prep_slave_sg() leaks
the allocated descriptor if loongson2_cmc_dma_set_xfer_param() fails.
loongson2-apb-cmc-dma.c:loongson2_cmc_dma_prep_slave_sg() {
...
desc = kzalloc_flex(*desc, sg_req, sg_len, GFP_NOWAIT);
if (!desc)
return ERR_PTR(-ENOMEM);
for_each_sg(sgl, sg, sg_len, i) {
ret = loongson2_cmc_dma_set_xfer_param(lchan, direction,
&buswidth,
sg_dma_len(sg));
if (ret)
return ERR_PTR(ret);
...
}
If the transfer direction is invalid, the function returns ERR_PTR(ret)
directly without freeing the dynamically allocated desc. Can this allow an
application to deplete kernel memory by repeatedly submitting bad SG
lists?
> u32 next_sg;
> struct loongson2_cmc_dma_chan_reg chan_reg;
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260818093448.3597016-1-zhoubinbin@loongson.cn?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling
2026-08-18 9:34 [PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling Binbin Zhou
2026-08-18 9:49 ` sashiko-bot
@ 2026-08-18 12:13 ` Huacai Chen
2026-08-18 16:39 ` Frank Li
2 siblings, 0 replies; 4+ messages in thread
From: Huacai Chen @ 2026-08-18 12:13 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Huacai Chen, Vinod Koul, Frank Li, dmaengine,
Julia Lawall, kernel test robot
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
On Tue, Aug 18, 2026 at 5:35 PM Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>
> Storing that negative value into an unsigned variable makes the check
> `if (lchan->irq < 0)` always false, so probe errors are not correctly
> detected and propagated.
>
> Fix this by changing the type of `irq` to `int`, which allows proper
> signed comparison and error handling.
>
> Fixes: 1c0028e725f1 ("dmaengine: loongson: New driver for the Loongson Multi-Channel DMA controller")
> Reported-by: Julia Lawall <julia.lawall@inria.fr>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/r/202608152230.p20WPlS0-lkp@intel.com/
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
> drivers/dma/loongson/loongson2-apb-cmc-dma.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> index 1c9a542edc85..969dbc5dabe7 100644
> --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> @@ -90,7 +90,7 @@ struct loongson2_cmc_dma_chan {
> struct dma_slave_config dma_sconfig;
> struct loongson2_cmc_dma_desc *desc;
> u32 id;
> - u32 irq;
> + int irq;
> u32 next_sg;
> struct loongson2_cmc_dma_chan_reg chan_reg;
> };
>
> base-commit: 075b74841bd0065a3bda3440873c747938e69b68
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling
2026-08-18 9:34 [PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling Binbin Zhou
2026-08-18 9:49 ` sashiko-bot
2026-08-18 12:13 ` Huacai Chen
@ 2026-08-18 16:39 ` Frank Li
2 siblings, 0 replies; 4+ messages in thread
From: Frank Li @ 2026-08-18 16:39 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Huacai Chen, Vinod Koul, Frank Li, dmaengine,
Huacai Chen, Julia Lawall, kernel test robot
On Tue, Aug 18, 2026 at 05:34:48PM +0800, Binbin Zhou wrote:
> Storing that negative value into an unsigned variable makes the check
> `if (lchan->irq < 0)` always false, so probe errors are not correctly
> detected and propagated.
>
> Fix this by changing the type of `irq` to `int`, which allows proper
> signed comparison and error handling.
>
> Fixes: 1c0028e725f1 ("dmaengine: loongson: New driver for the Loongson Multi-Channel DMA controller")
> Reported-by: Julia Lawall <julia.lawall@inria.fr>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/r/202608152230.p20WPlS0-lkp@intel.com/
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/dma/loongson/loongson2-apb-cmc-dma.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> index 1c9a542edc85..969dbc5dabe7 100644
> --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> @@ -90,7 +90,7 @@ struct loongson2_cmc_dma_chan {
> struct dma_slave_config dma_sconfig;
> struct loongson2_cmc_dma_desc *desc;
> u32 id;
> - u32 irq;
> + int irq;
> u32 next_sg;
> struct loongson2_cmc_dma_chan_reg chan_reg;
> };
>
> base-commit: 075b74841bd0065a3bda3440873c747938e69b68
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread