diff mbox series

[v2,33/64] dmaengine: sun6i: kill vchan BH on teardown

Message ID 176cf55cdf7e686ba064befa2b257f3ae76696ab.1785183549.git.allen.lkml@gmail.com (mailing list archive)
State New
Headers
Series None |

Commit Message

Allen Pais July 27, 2026, 8:36 p.m. UTC 
Use dma_chan_kill_bh() for virt-dma channel cleanup.

Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
 drivers/dma/sun6i-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

sashiko-bot@kernel.org July 27, 2026, 9:08 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Client callbacks flushed during cleanup can reschedule the driver tasklet after it was killed and clocks were disabled, leading to a hardware fault and list corruption.
--

--- Patch [33]: [PATCH v2 33/64] dmaengine: sun6i: kill vchan BH on teardown ---
commit 0c78bc25a98ea85ad52645998923d5aa57383201
Author: Allen Pais <allen.lkml@gmail.com>

dmaengine: sun6i: kill vchan BH on teardown
    
Use dma_chan_kill_bh() for virt-dma channel cleanup.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index a9a254dbf8cb9..9289c24dfcfaf 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -1097,7 +1097,7 @@ static inline void sun6i_dma_free(struct sun6i_dma_dev *sdev)
>  		struct sun6i_vchan *vchan = &sdev->vchans[i];
>  
>  		list_del(&vchan->vc.chan.device_node);
> -		tasklet_kill(&vchan->vc.task);
> +		dma_chan_kill_bh(&vchan->vc.chan);

[Severity: High]
This is a pre-existing issue, but could this cleanup sequence trigger a
hardware fault or list corruption if a client callback submits a new
transfer?

During device removal or sysfs unbind, sun6i_dma_remove() kills the
hardware tasklet and disables controller clocks before calling
sun6i_dma_free():

drivers/dma/sun6i-dma.c:sun6i_dma_remove() {
    ...
    sun6i_kill_tasklet(sdc);

    clk_disable_unprepare(sdc->clk_mbus);
    clk_disable_unprepare(sdc->clk);
    reset_control_assert(sdc->rstc);

    sun6i_dma_free(sdc);
}

sun6i_dma_free() then calls dma_chan_kill_bh() which flushes pending work.
If a client callback runs during this flush and submits a new transfer,
sun6i_dma_issue_pending() is invoked:

drivers/dma/sun6i-dma.c:sun6i_dma_issue_pending() {
    ...
    list_add_tail(&vchan->node, &sdev->pending);
    tasklet_schedule(&sdev->task);
}

Because tasklet_schedule() is called unconditionally without checking if
the device is tearing down, it resurrects the killed tasklet.

When the scheduled tasklet runs, it accesses hardware registers without
clocks enabled, leading to an external abort. Additionally, it concurrently
accesses the channel lists while sun6i_dma_free() mutates them via
list_del() without locks.

Should the teardown order in sun6i_dma_remove() be adjusted to flush
callbacks before disabling clocks, or should sun6i_dma_issue_pending()
check a shutdown flag before scheduling the tasklet?

>  	}
>  }
diff mbox series

Patch

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index a9a254dbf8cb..9289c24dfcfa 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -1097,7 +1097,7 @@  static inline void sun6i_dma_free(struct sun6i_dma_dev *sdev)
 		struct sun6i_vchan *vchan = &sdev->vchans[i];
 
 		list_del(&vchan->vc.chan.device_node);
-		tasklet_kill(&vchan->vc.task);
+		dma_chan_kill_bh(&vchan->vc.chan);
 	}
 }