DMA Engine development
 help / color / mirror / Atom feed
* [PATCH] dmaengine: mediatek: initialize tasklets before requesting IRQs
@ 2026-08-30  6:04 Runyu Xiao
  2026-08-30  6:20 ` sashiko-bot
  2026-08-30 12:09 ` [PATCH v2] " Runyu Xiao
  0 siblings, 2 replies; 4+ messages in thread
From: Runyu Xiao @ 2026-08-30  6:04 UTC (permalink / raw)
  To: vkoul; +Cc: dmaengine, linux-kernel, stable, runyu.xiao, jianhao.xu

The CQDMA probe registers each physical-channel IRQ before initializing
the tasklet scheduled by the interrupt handler. A pending interrupt can
schedule an uninitialized tasklet while probe is still running.

Initialize each tasklet when its physical channel is allocated, before
registering the corresponding IRQ.

Fixes: b1f01e48df5a ("dmaengine: Add MediaTek Command-Queue DMA controller for MT6765 SoC")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/dma/mediatek/mtk-cqdma.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
index 80791e30a..8038f14d1 100644
--- a/drivers/dma/mediatek/mtk-cqdma.c
+++ b/drivers/dma/mediatek/mtk-cqdma.c
@@ -815,6 +815,7 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
 		INIT_LIST_HEAD(&cqdma->pc[i]->queue);
 		spin_lock_init(&cqdma->pc[i]->lock);
 		refcount_set(&cqdma->pc[i]->refcnt, 0);
+		tasklet_setup(&cqdma->pc[i]->tasklet, mtk_cqdma_tasklet_cb);
 		cqdma->pc[i]->base = devm_platform_ioremap_resource(pdev, i);
 		if (IS_ERR(cqdma->pc[i]->base))
 			return PTR_ERR(cqdma->pc[i]->base);
@@ -869,10 +870,6 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, cqdma);
 
-	/* initialize tasklet for each PC */
-	for (i = 0; i < cqdma->dma_channels; ++i)
-		tasklet_setup(&cqdma->pc[i]->tasklet, mtk_cqdma_tasklet_cb);
-
 	dev_info(&pdev->dev, "MediaTek CQDMA driver registered\n");
 
 	return 0;
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] dmaengine: mediatek: initialize tasklets before requesting IRQs
  2026-08-30  6:04 [PATCH] dmaengine: mediatek: initialize tasklets before requesting IRQs Runyu Xiao
@ 2026-08-30  6:20 ` sashiko-bot
  2026-08-30 12:09 ` [PATCH v2] " Runyu Xiao
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-30  6:20 UTC (permalink / raw)
  To: Runyu Xiao; +Cc: Frank.Li, vkoul, dmaengine

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:

New issues:
- [High] Missing `tasklet_kill()` on the `mtk_cqdma_probe()` error path results in a Use-After-Free if a tasklet is scheduled before the failure occurs.

Pre-existing issues:
- [High] The interrupt handler `mtk_cqdma_irq()` can crash with a NULL pointer dereference if an interrupt triggers before all physical channels are allocated during probe.
- [High] Requesting IRQs and registering the DMA device before enabling power domains and clocks causes asynchronous or synchronous external aborts.
- [High] Missing `of_dma_controller_free()` on the probe error path leaves a dangling pointer in the OF DMA list.
- [High] Incorrect teardown order in `mtk_cqdma_remove()` allows tasklets to be re-scheduled after being killed.
- [High] Error path in `mtk_cqdma_prep_dma_memcpy()` frees descriptors that are already linked into the virtual channel's list, causing list corruption and use-after-free.
- [High] A zero-length DMA transfer request causes an out-of-bounds access and crash in `mtk_cqdma_prep_dma_memcpy()`.
- [High] When issuing multiple pending descriptors, the CPU is forced to synchronously wait for hardware completion for all descriptors, stalling the system.
--

commit 0bc1ea9cc66d3b94f9b7b69d7f95a506a7e4bd12
Author: Runyu Xiao <runyu.xiao@seu.edu.cn>

dmaengine: mediatek: initialize tasklets before requesting IRQs

The CQDMA probe registers each physical-channel IRQ before initializing
the tasklet scheduled by the interrupt handler. A pending interrupt can
schedule an uninitialized tasklet while probe is still running.

> diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
> index 80791e30aec24..8038f14d1afb4 100644
> --- a/drivers/dma/mediatek/mtk-cqdma.c
> +++ b/drivers/dma/mediatek/mtk-cqdma.c
> @@ -815,6 +815,7 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
>  		INIT_LIST_HEAD(&cqdma->pc[i]->queue);
>  		spin_lock_init(&cqdma->pc[i]->lock);
>  		refcount_set(&cqdma->pc[i]->refcnt, 0);
> +		tasklet_setup(&cqdma->pc[i]->tasklet, mtk_cqdma_tasklet_cb);

[Severity: High]
Does this patch introduce a use-after-free risk on the probe error path?

By moving tasklet_setup() before devm_request_irq(), the tasklet becomes
active early. If a subsequent probe step (like mtk_cqdma_hw_init) fails,
the error path returns without killing the scheduled tasklets:

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_probe() {
    ...
err_unregister:
    dma_async_device_unregister(dd);
    
    return err;
}

If an early interrupt has already scheduled the tasklet, and devres frees
the channel memory upon probe failure, will the tasklet subsequently execute
on freed memory?


[Severity: High]
This is a pre-existing issue, but can the interrupt handler crash with a
NULL pointer dereference if an interrupt triggers before all physical channels
are allocated during probe?

During mtk_cqdma_probe(), channels are allocated and their IRQs requested
sequentially in a loop. If an IRQ fires for channel 0 before channel 1 is
allocated, mtk_cqdma_irq() iterates over all channels:

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_irq() {
    ...
    for (i = 0; i < cqdma->dma_channels; ++i, schedule_tasklet = false) {
        spin_lock(&cqdma->pc[i]->lock);
    ...
}

Would this dereference the unallocated cqdma->pc[1] and cause a crash via
spin_lock()?


[Severity: High]
This isn't a bug introduced by this patch, but does requesting IRQs and
registering the DMA device before enabling power domains and clocks risk an
external abort?

In mtk_cqdma_probe(), devm_request_irq() and dma_async_device_register()
are called before mtk_cqdma_hw_init() enables power domains and clocks. 

If a spurious interrupt or an immediate client transaction occurs, it will
attempt to read or write unclocked hardware registers:

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_irq() {
    ...
    if (mtk_dma_read(cqdma->pc[i], MTK_CQDMA_INT_FLAG) ...
    ...
}

Could this lead to a synchronous external abort and a kernel panic?


[Severity: High]
This is a pre-existing issue, but does the probe error path leave a dangling
pointer in the OF DMA list?

In mtk_cqdma_probe(), the device structure pointer is added to the global
OF DMA list:

    err = of_dma_controller_register(pdev->dev.of_node,
                                     of_dma_xlate_by_chan_id, cqdma);

If mtk_cqdma_hw_init() fails, the error path jumps to err_unregister, which
fails to call of_dma_controller_free():

err_unregister:
    dma_async_device_unregister(dd);

    return err;

When devres frees the cqdma device structure, will this leave a dangling
pointer that can be accessed by subsequent DMA requests?

>  		cqdma->pc[i]->base = devm_platform_ioremap_resource(pdev, i);
>  		if (IS_ERR(cqdma->pc[i]->base))
>  			return PTR_ERR(cqdma->pc[i]->base);
> @@ -869,10 +870,6 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, cqdma);
>  
> -	/* initialize tasklet for each PC */
> -	for (i = 0; i < cqdma->dma_channels; ++i)
> -		tasklet_setup(&cqdma->pc[i]->tasklet, mtk_cqdma_tasklet_cb);
> -
>  	dev_info(&pdev->dev, "MediaTek CQDMA driver registered\n");
>  
>  	return 0;

[Severity: High]
This isn't a bug introduced by this patch, but does the teardown order in
mtk_cqdma_remove() allow tasklets to be re-scheduled after being killed?

VC tasklets are killed before PC hardware interrupts are disabled and
PC tasklets are killed:

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_remove() {
    ...
    for (i = 0; i < cqdma->dma_requests; i++) {
        vc = &cqdma->vc[i];
        ...
        tasklet_kill(&vc->vc.task);
    }
    ...
    for (i = 0; i < cqdma->dma_channels; i++)
        tasklet_kill(&cqdma->pc[i]->tasklet);
    ...
}

If an interrupt fires in this window and schedules a PC tasklet, couldn't
the PC tasklet call vchan_cookie_complete() and reschedule the VC tasklet
after it was already killed, leading to a use-after-free when devres frees
the tasklet memory?


[Severity: High]
This is a pre-existing issue, but does the error path in
mtk_cqdma_prep_dma_memcpy() free descriptors that are already linked into
the virtual channel's list?

Descriptors are added to the active vc->desc_allocated list:

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_prep_dma_memcpy() {
    ...
    tx = vchan_tx_prep(to_virt_chan(c), &cvd[i]->vd, flags);
    ...
}

If a subsequent memory allocation for cvd[i] fails, the error path frees
cvd[i - 1] without unlinking it:

    ...
    for (; i > 0; --i)
        kfree(cvd[i - 1]);
    return NULL;
}

Will the DMA framework later access this freed memory and cause a
use-after-free?


[Severity: High]
This isn't a bug introduced by this patch, but does a zero-length DMA
transfer request cause an out-of-bounds access?

If mtk_cqdma_prep_dma_memcpy() is called with len == 0, the allocation
count becomes 0:

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_prep_dma_memcpy() {
    ...
    nr_vd = DIV_ROUND_UP(len, MTK_CQDMA_MAX_LEN);
    cvd = kzalloc_objs(*cvd, nr_vd, GFP_NOWAIT);
    ...
}

kzalloc_objs() will return the ZERO_SIZE_PTR error pointer. The
initialization loop is skipped, and the code attempts to dereference
it out-of-bounds:

    ...
    return &cvd[0]->vd.tx;
}

Could this trigger a kernel panic when a driver submits a 0-length DMA
request?


[Severity: High]
This is a pre-existing issue, but does issuing multiple pending
descriptors force the CPU to synchronously wait for hardware completion?

In mtk_cqdma_issue_vchan_pending(), the trigger_engine variable is scoped
outside the loop and never reset back to false:

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_issue_vchan_pending() {
    ...
    bool trigger_engine = false;
    ...
    if (trigger_engine)
        mtk_cqdma_start(pc, cvd);
    ...
}

For all subsequent queued descriptors, will this repeatedly execute
mtk_cqdma_start(), which busy-waits with spinlocks held and interrupts
disabled for the previous transaction to finish, potentially stalling
the system?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830060422.2204459-1-runyu.xiao@seu.edu.cn?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] dmaengine: mediatek: initialize tasklets before requesting IRQs
  2026-08-30  6:04 [PATCH] dmaengine: mediatek: initialize tasklets before requesting IRQs Runyu Xiao
  2026-08-30  6:20 ` sashiko-bot
@ 2026-08-30 12:09 ` Runyu Xiao
  2026-08-30 12:24   ` sashiko-bot
  1 sibling, 1 reply; 4+ messages in thread
From: Runyu Xiao @ 2026-08-30 12:09 UTC (permalink / raw)
  To: sean.wang
  Cc: vkoul, Frank.Li, matthias.bgg, angelogioacchino.delregno,
	dmaengine, linux-arm-kernel, linux-mediatek, linux-kernel,
	runyu.xiao, jianhao.xu, stable

The CQDMA probe registers each physical-channel IRQ before initializing
the tasklet scheduled by the interrupt handler. A pending interrupt can
access an uninitialized physical channel or schedule an uninitialized
tasklet while probe is still running.

Initialize all physical channels, including their tasklets, before
registering any IRQ.

On probe failure, unwind every requested IRQ and initialized tasklet
before devres releases the physical-channel and virtual-channel objects.
Also release the OF DMA controller when hardware initialization fails
after registration.

Fixes: b1f01e48df5a ("dmaengine: Add MediaTek Command-Queue DMA controller for MT6765 SoC")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/20260830060422.2204459-1-runyu.xiao@seu.edu.cn/
Assisted-by: Codex:GPT-5
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>

---
v2:
- Initialize all physical channels before registering any IRQ, since the
  interrupt handler scans all channels.
- Disable requested IRQs before killing physical- and virtual-channel
  tasklets, then free the IRQs on every probe error path before devres
  releases channel state.
- Release the OF DMA controller when hardware initialization fails.
---
 drivers/dma/mediatek/mtk-cqdma.c | 59 +++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
index 80791e30a..741b6325c 100644
--- a/drivers/dma/mediatek/mtk-cqdma.c
+++ b/drivers/dma/mediatek/mtk-cqdma.c
@@ -749,7 +749,7 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
 	struct mtk_cqdma_vchan *vc;
 	struct dma_device *dd;
 	int err;
-	u32 i;
+	u32 i, nr_tasklets = 0, nr_irqs = 0;
 
 	cqdma = devm_kzalloc(&pdev->dev, sizeof(*cqdma), GFP_KERNEL);
 	if (!cqdma)
@@ -805,41 +805,54 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
 	if (!cqdma->pc)
 		return -ENOMEM;
 
-	/* initialization for PCs */
+	/* Initialize all PCs before registering any IRQ. */
 	for (i = 0; i < cqdma->dma_channels; ++i) {
 		cqdma->pc[i] = devm_kcalloc(&pdev->dev, 1,
 					    sizeof(**cqdma->pc), GFP_KERNEL);
-		if (!cqdma->pc[i])
-			return -ENOMEM;
+		if (!cqdma->pc[i]) {
+			err = -ENOMEM;
+			goto err_tasklets;
+		}
 
 		INIT_LIST_HEAD(&cqdma->pc[i]->queue);
 		spin_lock_init(&cqdma->pc[i]->lock);
 		refcount_set(&cqdma->pc[i]->refcnt, 0);
+		tasklet_setup(&cqdma->pc[i]->tasklet, mtk_cqdma_tasklet_cb);
+		nr_tasklets++;
 		cqdma->pc[i]->base = devm_platform_ioremap_resource(pdev, i);
-		if (IS_ERR(cqdma->pc[i]->base))
-			return PTR_ERR(cqdma->pc[i]->base);
+		if (IS_ERR(cqdma->pc[i]->base)) {
+			err = PTR_ERR(cqdma->pc[i]->base);
+			goto err_tasklets;
+		}
 
 		/* allocate IRQ resource */
 		err = platform_get_irq(pdev, i);
 		if (err < 0)
-			return err;
+			goto err_tasklets;
 		cqdma->pc[i]->irq = err;
+	}
 
+	/* Register IRQs only after all PCs are initialized. */
+	for (i = 0; i < cqdma->dma_channels; ++i) {
 		err = devm_request_irq(&pdev->dev, cqdma->pc[i]->irq,
 				       mtk_cqdma_irq, 0, dev_name(&pdev->dev),
 				       cqdma);
 		if (err) {
 			dev_err(&pdev->dev,
 				"request_irq failed with err %d\n", err);
-			return -EINVAL;
+			err = -EINVAL;
+			goto err_tasklets;
 		}
+		nr_irqs++;
 	}
 
 	/* allocate resource for VCs */
 	cqdma->vc = devm_kcalloc(&pdev->dev, cqdma->dma_requests,
 				 sizeof(*cqdma->vc), GFP_KERNEL);
-	if (!cqdma->vc)
-		return -ENOMEM;
+	if (!cqdma->vc) {
+		err = -ENOMEM;
+		goto err_tasklets;
+	}
 
 	for (i = 0; i < cqdma->dma_requests; i++) {
 		vc = &cqdma->vc[i];
@@ -850,7 +863,7 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
 
 	err = dma_async_device_register(dd);
 	if (err)
-		return err;
+		goto err_tasklets;
 
 	err = of_dma_controller_register(pdev->dev.of_node,
 					 of_dma_xlate_by_chan_id, cqdma);
@@ -864,22 +877,36 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
 	if (err) {
 		dev_err(&pdev->dev,
 			"MediaTek CQDMA HW initialization failed %d\n", err);
-		goto err_unregister;
+		goto err_of_unregister;
 	}
 
 	platform_set_drvdata(pdev, cqdma);
 
-	/* initialize tasklet for each PC */
-	for (i = 0; i < cqdma->dma_channels; ++i)
-		tasklet_setup(&cqdma->pc[i]->tasklet, mtk_cqdma_tasklet_cb);
-
 	dev_info(&pdev->dev, "MediaTek CQDMA driver registered\n");
 
 	return 0;
 
+err_of_unregister:
+	of_dma_controller_free(pdev->dev.of_node);
+
 err_unregister:
 	dma_async_device_unregister(dd);
 
+err_tasklets:
+	/* Prevent IRQ handlers from accessing channel state during cleanup. */
+	for (i = 0; i < nr_irqs; i++)
+		disable_irq(cqdma->pc[i]->irq);
+
+	for (i = 0; i < nr_tasklets; i++)
+		tasklet_kill(&cqdma->pc[i]->tasklet);
+
+	if (cqdma->vc)
+		for (i = 0; i < cqdma->dma_requests; i++)
+			tasklet_kill(&cqdma->vc[i].vc.task);
+
+	for (i = 0; i < nr_irqs; i++)
+		devm_free_irq(&pdev->dev, cqdma->pc[i]->irq, cqdma);
+
 	return err;
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] dmaengine: mediatek: initialize tasklets before requesting IRQs
  2026-08-30 12:09 ` [PATCH v2] " Runyu Xiao
@ 2026-08-30 12:24   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-30 12:24 UTC (permalink / raw)
  To: Runyu Xiao; +Cc: Frank.Li, dmaengine, vkoul

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [High] The DMA device, OF controller, and IRQ handlers are registered and exposed to the system before the hardware clocks and power domains are enabled, risking a fatal system crash.
- [High] Incorrect teardown ordering in `mtk_cqdma_remove` allows virtual channel tasklets to be rescheduled after they are killed, leading to use-after-free, and permits hardware access during de-initialization.
- [High] `mtk_cqdma_remove` manually removes channels from the device list, completely bypassing channel unregistration and leaking memory and ID allocations.
- [High] Lockless access of descriptor state in `mtk_cqdma_tx_status` leads to a TOCTOU use-after-free.
- [High] If `mtk_cqdma_prep_dma_memcpy` is called with a transfer length of 0, memory allocation returns `ZERO_SIZE_PTR`, bypassing the NULL check and causing a kernel Oops.
--

commit 8a7b8d66b024c2ee132ed95053a3dfaf346172c6
Author: Runyu Xiao <runyu.xiao@seu.edu.cn>

dmaengine: mediatek: initialize tasklets before requesting IRQs

The CQDMA probe registers each physical-channel IRQ before initializing
the tasklet scheduled by the interrupt handler. A pending interrupt can
access an uninitialized physical channel or schedule an uninitialized
tasklet while probe is still running.

Link: https://lore.kernel.org/all/20260830060422.2204459-1-runyu.xiao@seu.edu.cn/

> diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
> index 80791e30aec24..741b6325cdee4 100644
> --- a/drivers/dma/mediatek/mtk-cqdma.c
> +++ b/drivers/dma/mediatek/mtk-cqdma.c

[ ... ]

> @@ -805,41 +805,54 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
>  		err = platform_get_irq(pdev, i);
>  		if (err < 0)
> -			return err;
> +			goto err_tasklets;
>  		cqdma->pc[i]->irq = err;
> +	}
>  
> +	/* Register IRQs only after all PCs are initialized. */
> +	for (i = 0; i < cqdma->dma_channels; ++i) {
>  		err = devm_request_irq(&pdev->dev, cqdma->pc[i]->irq,
>  				       mtk_cqdma_irq, 0, dev_name(&pdev->dev),
>  				       cqdma);

[Severity: High]
This is a pre-existing issue, but does requesting the IRQ here unmask interrupts
at the controller, allowing the ISR to execute unclocked register reads before
the hardware clocks are enabled?

>  		if (err) {
>  			dev_err(&pdev->dev,
>  				"request_irq failed with err %d\n", err);

[ ... ]

> @@ -850,7 +863,7 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
>  
>  	err = dma_async_device_register(dd);
>  	if (err)
> -		return err;
> +		goto err_tasklets;

[Severity: High]
This is a pre-existing issue, but is the DMA device exposed to clients before
its hardware is initialized?

>  
>  	err = of_dma_controller_register(pdev->dev.of_node,
>  					 of_dma_xlate_by_chan_id, cqdma);
> @@ -864,22 +877,36 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
>  	err = mtk_cqdma_hw_init(cqdma);
>  	if (err) {

[Severity: High]
This is a pre-existing issue, but hardware clocks and PM runtime are only
initialized here, which appears to be too late in the probe sequence. Could an
immediately pending interrupt or an immediate DMA request access the unclocked
hardware, leading to a fatal system crash?

[ ... ]

This is a pre-existing issue, but could the incorrect teardown ordering in
mtk_cqdma_remove() lead to use-after-free and hardware access during
de-initialization?

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_remove() {
...
	/* kill VC task */
	for (i = 0; i < cqdma->dma_requests; i++) {
		vc = &cqdma->vc[i];

		list_del(&vc->vc.chan.device_node);
		tasklet_kill(&vc->vc.task);
	}
...
	for (i = 0; i < cqdma->dma_channels; i++) {
		spin_lock_irqsave(&cqdma->pc[i]->lock, flags);
		mtk_dma_clr(cqdma->pc[i], MTK_CQDMA_INT_EN,
			    MTK_CQDMA_INT_EN_BIT);
		spin_unlock_irqrestore(&cqdma->pc[i]->lock, flags);

		/* Waits for any pending IRQ handlers to complete */
		synchronize_irq(cqdma->pc[i]->irq);

		tasklet_kill(&cqdma->pc[i]->tasklet);
	}

	/* disable hardware */
	mtk_cqdma_hw_deinit(cqdma);

	dma_async_device_unregister(&cqdma->ddev);
...
}

[Severity: High]
VC tasklets are killed prematurely before PC producers and IRQs are fully
stopped. Furthermore, the teardown loop fails to call disable_irq(), leaving the
global IRQ handler vulnerable to cross-channel polling execution. Lastly, is it
safe that the DMA device is unregistered after hardware deinitialization, which
might allow late client requests to interact with unclocked hardware?


This is a pre-existing issue, but does mtk_cqdma_remove() manually remove
channels from the device list, completely bypassing channel unregistration
and leaking memory and ID allocations?

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_remove() {
...
	for (i = 0; i < cqdma->dma_requests; i++) {
		vc = &cqdma->vc[i];

		list_del(&vc->vc.chan.device_node);
		tasklet_kill(&vc->vc.task);
	}
...
}

[Severity: High]
Does calling list_del(&vc->vc.chan.device_node) hide the channels from the DMA
core unregistration, leading to a leak of memory and ID allocations?


This is a pre-existing issue, but does lockless access of descriptor state in
mtk_cqdma_tx_status() lead to a TOCTOU use-after-free?

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_tx_status() {
...
	spin_lock_irqsave(&cvc->pc->lock, flags);
	spin_lock(&cvc->vc.lock);
	vd = mtk_cqdma_find_active_desc(c, cookie);
	spin_unlock(&cvc->vc.lock);
	spin_unlock_irqrestore(&cvc->pc->lock, flags);

	if (vd) {
		cvd = to_cqdma_vdesc(vd);
		bytes = cvd->residue;
	}
...
}

[Severity: High]
Since the locks protecting the descriptor's lifecycle are dropped right before
its fields are accessed, could concurrent tasklets complete the transaction and
free the descriptor, resulting in a use-after-free?


This is a pre-existing issue, but if mtk_cqdma_prep_dma_memcpy() is called
with a transfer length of 0, could memory allocation return ZERO_SIZE_PTR and
bypass the NULL check?

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_prep_dma_memcpy() {
...
	nr_vd = DIV_ROUND_UP(len, MTK_CQDMA_MAX_LEN);
	cvd = kzalloc_objs(*cvd, nr_vd, GFP_NOWAIT);
	if (!cvd)
		return NULL;

	for (i = 0; i < nr_vd; ++i) {
...
	return &cvd[0]->vd.tx;
}

[Severity: High]
When len is 0, nr_vd becomes 0 and kzalloc_objs() returns ZERO_SIZE_PTR.
Does the NULL check pass in this scenario, skipping the loop and causing a
kernel oops when &cvd[0]->vd.tx is returned and dereferenced?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830120955.2638359-1-runyu.xiao@seu.edu.cn?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-30 12:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-30  6:04 [PATCH] dmaengine: mediatek: initialize tasklets before requesting IRQs Runyu Xiao
2026-08-30  6:20 ` sashiko-bot
2026-08-30 12:09 ` [PATCH v2] " Runyu Xiao
2026-08-30 12:24   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox