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

[sashiko-bot@kernel.org]

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