* Re: [PATCH v4] dmaengine: sun6i-dma: Fix memory leak in sun6i_dma_terminate_all
2026-06-18 2:06 [PATCH v4] dmaengine: sun6i-dma: Fix memory leak in sun6i_dma_terminate_all Hongling Zeng
@ 2026-06-18 2:16 ` sashiko-bot
2026-06-18 3:22 ` Frank Li
2026-06-30 11:34 ` Vinod Koul
2 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-06-18 2:16 UTC (permalink / raw)
To: Hongling Zeng; +Cc: linux-sunxi, vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Lockless read of `vchan->phy` causes a Time-of-Check to Time-of-Use (TOCTOU) race, leading to cross-channel list corruption and improper hardware state modifications.
- [High] Lack of synchronization between `sun6i_dma_terminate_all` and the hardirq handler `sun6i_dma_interrupt` leads to an unmitigated NULL pointer dereference.
--
commit 0f302155373f5ca26acb89959d317a9f6c68ecc0
Author: Hongling Zeng <zenghongling@kylinos.cn>
dmaengine: sun6i-dma: Fix memory leak in sun6i_dma_terminate_all
This commit fixes a memory leak when terminating DMA transfers by using
vchan_terminate_vdesc() to handle both cyclic and non-cyclic descriptors.
It also prevents completed descriptors from being double-added to the list.
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index a9a254dbf8cb9..f47a326dd7ffa 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -945,16 +945,13 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
>
> spin_lock_irqsave(&vchan->vc.lock, flags);
>
> - if (vchan->cyclic) {
> - vchan->cyclic = false;
> - if (pchan && pchan->desc) {
> - struct virt_dma_desc *vd = &pchan->desc->vd;
> - struct virt_dma_chan *vc = &vchan->vc;
> + if (pchan && pchan->desc && pchan->desc != pchan->done) {
> + struct virt_dma_desc *vd = &pchan->desc->vd;
>
> - list_add_tail(&vd->node, &vc->desc_completed);
> - }
> + vchan_terminate_vdesc(vd);
[Severity: High]
This is a pre-existing issue, but by calling vchan_terminate_vdesc() here,
does this code now trigger list corruption or a lockdep splat?
Earlier in sun6i_dma_terminate_all(), the physical channel is read locklessly:
struct sun6i_pchan *pchan = vchan->phy;
If a concurrent DMA tasklet frees and reassigns this physical channel to a
completely different virtual channel before spin_lock_irqsave() is acquired,
we evaluate a stale pchan. Passing this foreign descriptor into
vchan_terminate_vdesc() will attempt to add it to desc_terminated while
holding the wrong lock, corrupting the list.
> }
>
> + vchan->cyclic = false;
> vchan_get_all_descriptors(&vchan->vc, &head);
>
> if (pchan) {
[Severity: High]
This is a pre-existing issue, but does clearing the physical channel state
here lead to a NULL pointer dereference in the interrupt handler?
Just below this hunk in sun6i_dma_terminate_all(), the code sets
pchan->desc = NULL without synchronizing with the IRQ handler:
if (pchan) {
writel(DMA_CHAN_ENABLE_STOP, pchan->base + DMA_CHAN_ENABLE);
writel(DMA_CHAN_PAUSE_RESUME, pchan->base + DMA_CHAN_PAUSE);
vchan->phy = NULL;
pchan->vchan = NULL;
pchan->desc = NULL;
pchan->done = NULL;
}
If sun6i_dma_interrupt() is running concurrently on another CPU, it
could be executing vchan_cookie_complete(&pchan->desc->vd) or
vchan_cyclic_callback(&pchan->desc->vd). Without synchronize_irq()
or similar to wait for pending handlers, this will pass a NULL offset
to those helpers, which then dereference vd->tx.chan.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618020609.1155962-1-zenghongling@kylinos.cn?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v4] dmaengine: sun6i-dma: Fix memory leak in sun6i_dma_terminate_all
2026-06-18 2:06 [PATCH v4] dmaengine: sun6i-dma: Fix memory leak in sun6i_dma_terminate_all Hongling Zeng
2026-06-18 2:16 ` sashiko-bot
@ 2026-06-18 3:22 ` Frank Li
2026-06-30 11:34 ` Vinod Koul
2 siblings, 0 replies; 5+ messages in thread
From: Frank Li @ 2026-06-18 3:22 UTC (permalink / raw)
To: Hongling Zeng
Cc: vkoul, Frank.Li, wens, jernej.skrabec, samuel, mripard, arnd,
dmaengine, linux-arm-kernel, linux-sunxi, linux-kernel,
zhongling0719
On Thu, Jun 18, 2026 at 10:06:09AM +0800, Hongling Zeng wrote:
> When terminating DMA transfers, active descriptors are not properly
> reclaimed. Only cyclic descriptors were handled, leaving non-cyclic
> descriptors and their LLI chains to be permanently leaked.
>
> Fix by using vchan_terminate_vdesc() which handles both cyclic and
> non-cyclic descriptors by adding them to desc_terminated queue for
> proper cleanup.
>
> Add pchan->desc != pchan->done check to prevent double-adding completed
> descriptors, which would corrupt the list.
>
> Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> Suggested-by: Frank Li <Frank.li@oss.nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> ---
> Change in v2;
> -Add pchan->desc != pchan->done check to prevent race condition
> where completed descriptors could be double-added to desc_completed
> list, causing list corruption
> ---
> Change in v3:
> -Fix by using vchan_terminate_vdesc() as suggested by Frank Li
> ---
> Change in v4:
> -Correct the commit message
> ---
> drivers/dma/sun6i-dma.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 7a79f346250a..134ae840f176 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -946,16 +946,13 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
>
> spin_lock_irqsave(&vchan->vc.lock, flags);
>
> - if (vchan->cyclic) {
> - vchan->cyclic = false;
> - if (pchan && pchan->desc) {
> - struct virt_dma_desc *vd = &pchan->desc->vd;
> - struct virt_dma_chan *vc = &vchan->vc;
> -
> - list_add_tail(&vd->node, &vc->desc_completed);
> - }
> + if (pchan && pchan->desc && pchan->desc != pchan->done) {
> + struct virt_dma_desc *vd = &pchan->desc->vd;
> +
> + vchan_terminate_vdesc(vd);
> }
>
> + vchan->cyclic = false;
> vchan_get_all_descriptors(&vchan->vc, &head);
>
> if (pchan) {
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v4] dmaengine: sun6i-dma: Fix memory leak in sun6i_dma_terminate_all
2026-06-18 2:06 [PATCH v4] dmaengine: sun6i-dma: Fix memory leak in sun6i_dma_terminate_all Hongling Zeng
2026-06-18 2:16 ` sashiko-bot
2026-06-18 3:22 ` Frank Li
@ 2026-06-30 11:34 ` Vinod Koul
2026-07-01 5:01 ` Hongling Zeng
2 siblings, 1 reply; 5+ messages in thread
From: Vinod Koul @ 2026-06-30 11:34 UTC (permalink / raw)
To: Hongling Zeng
Cc: Frank.Li, wens, jernej.skrabec, samuel, mripard, arnd, dmaengine,
linux-arm-kernel, linux-sunxi, linux-kernel, zhongling0719,
Frank Li
On 18-06-26, 10:06, Hongling Zeng wrote:
> When terminating DMA transfers, active descriptors are not properly
> reclaimed. Only cyclic descriptors were handled, leaving non-cyclic
> descriptors and their LLI chains to be permanently leaked.
>
> Fix by using vchan_terminate_vdesc() which handles both cyclic and
> non-cyclic descriptors by adding them to desc_terminated queue for
> proper cleanup.
>
> Add pchan->desc != pchan->done check to prevent double-adding completed
> descriptors, which would corrupt the list.
Thanks for the patch. Please consider revising the subject which should
describe the changes in the patch and not the fix/issue.
A better one would be "fix reclaim descriptors while terminating"
>
> Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> Suggested-by: Frank Li <Frank.li@oss.nxp.com>
>
> ---
> Change in v2;
> -Add pchan->desc != pchan->done check to prevent race condition
> where completed descriptors could be double-added to desc_completed
> list, causing list corruption
> ---
> Change in v3:
> -Fix by using vchan_terminate_vdesc() as suggested by Frank Li
> ---
> Change in v4:
> -Correct the commit message
> ---
> drivers/dma/sun6i-dma.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 7a79f346250a..134ae840f176 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -946,16 +946,13 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
>
> spin_lock_irqsave(&vchan->vc.lock, flags);
>
> - if (vchan->cyclic) {
> - vchan->cyclic = false;
> - if (pchan && pchan->desc) {
> - struct virt_dma_desc *vd = &pchan->desc->vd;
> - struct virt_dma_chan *vc = &vchan->vc;
> -
> - list_add_tail(&vd->node, &vc->desc_completed);
> - }
> + if (pchan && pchan->desc && pchan->desc != pchan->done) {
> + struct virt_dma_desc *vd = &pchan->desc->vd;
> +
> + vchan_terminate_vdesc(vd);
> }
>
> + vchan->cyclic = false;
> vchan_get_all_descriptors(&vchan->vc, &head);
>
> if (pchan) {
> --
> 2.25.1
--
~Vinod
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v4] dmaengine: sun6i-dma: Fix memory leak in sun6i_dma_terminate_all
2026-06-30 11:34 ` Vinod Koul
@ 2026-07-01 5:01 ` Hongling Zeng
0 siblings, 0 replies; 5+ messages in thread
From: Hongling Zeng @ 2026-07-01 5:01 UTC (permalink / raw)
To: Vinod Koul, Hongling Zeng
Cc: Frank.Li, wens, jernej.skrabec, samuel, mripard, arnd, dmaengine,
linux-arm-kernel, linux-sunxi, linux-kernel, Frank Li
在 2026年06月30日 19:34, Vinod Koul 写道:
> On 18-06-26, 10:06, Hongling Zeng wrote:
>> When terminating DMA transfers, active descriptors are not properly
>> reclaimed. Only cyclic descriptors were handled, leaving non-cyclic
>> descriptors and their LLI chains to be permanently leaked.
>>
>> Fix by using vchan_terminate_vdesc() which handles both cyclic and
>> non-cyclic descriptors by adding them to desc_terminated queue for
>> proper cleanup.
>>
>> Add pchan->desc != pchan->done check to prevent double-adding completed
>> descriptors, which would corrupt the list.
> Thanks for the patch. Please consider revising the subject which should
> describe the changes in the patch and not the fix/issue.
>
> A better one would be "fix reclaim descriptors while terminating"
Thank you for the suggestion. I'll update the subject in v5 to describe
the changes rather than the issue.
>> Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
>> Suggested-by: Frank Li <Frank.li@oss.nxp.com>
>>
>> ---
>> Change in v2;
>> -Add pchan->desc != pchan->done check to prevent race condition
>> where completed descriptors could be double-added to desc_completed
>> list, causing list corruption
>> ---
>> Change in v3:
>> -Fix by using vchan_terminate_vdesc() as suggested by Frank Li
>> ---
>> Change in v4:
>> -Correct the commit message
>> ---
>> drivers/dma/sun6i-dma.c | 13 +++++--------
>> 1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
>> index 7a79f346250a..134ae840f176 100644
>> --- a/drivers/dma/sun6i-dma.c
>> +++ b/drivers/dma/sun6i-dma.c
>> @@ -946,16 +946,13 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
>>
>> spin_lock_irqsave(&vchan->vc.lock, flags);
>>
>> - if (vchan->cyclic) {
>> - vchan->cyclic = false;
>> - if (pchan && pchan->desc) {
>> - struct virt_dma_desc *vd = &pchan->desc->vd;
>> - struct virt_dma_chan *vc = &vchan->vc;
>> -
>> - list_add_tail(&vd->node, &vc->desc_completed);
>> - }
>> + if (pchan && pchan->desc && pchan->desc != pchan->done) {
>> + struct virt_dma_desc *vd = &pchan->desc->vd;
>> +
>> + vchan_terminate_vdesc(vd);
>> }
>>
>> + vchan->cyclic = false;
>> vchan_get_all_descriptors(&vchan->vc, &head);
>>
>> if (pchan) {
>> --
>> 2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread