[PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling

[PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling

[Binbin Zhou]

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

Re: [PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling

[sashiko-bot]

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

Re: [PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling

[Huacai Chen]

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
>

Re: [PATCH] dmaengine: loongson: loongson2-apb-cmc: Fix signedness bug in irq handling

[Frank Li]

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
>