Re: [PATCH] dma: at_hdmac: add device_synchronize callback

On Fri, Jul 24, 2026 at 11:35:56PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Adding `device_synchronize` is insufficient to fix descriptor synchronization races because `atc_terminate_all()` still prematurely frees active/terminated descriptors, causing a Use-After-Free.
> --
>
> --- Patch [1]: [PATCH] dma: at_hdmac: add device_synchronize callback ---
> commit cf29784e04ea139aef404987d19236faf066af03
> Author: Rosen Penev <[email protected]>
>
> dma: at_hdmac: add device_synchronize callback
>
> Implement the device_synchronize callback for the atmel DMA driver by adding
> atc_synchronize() which calls vchan_synchronize() on the associated virt
> channel. This ensures proper synchronization of pending descriptors when
> requested by the DMA framework.
>
> > diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> > index e5b30a57c477a..96233eb7a3c6e 100644
> > --- a/drivers/dma/at_hdmac.c
> > +++ b/drivers/dma/at_hdmac.c
> > @@ -1616,6 +1616,13 @@ static int atc_resume(struct dma_chan *chan)
> >  	return 0;
> >  }
> >
> > +static void atc_synchronize(struct dma_chan *chan)
> > +{
> > +	struct at_dma_chan *atchan = to_at_dma_chan(chan);
> > +
> > +	vchan_synchronize(&atchan->vc);
> > +}
>
> [Severity: High]
> This is a pre-existing issue, but does adding device_synchronize fully resolve
> the descriptor synchronization races?
>
> When dmaengine_terminate_sync() is called, it first invokes
> device_terminate_all() (which maps to atc_terminate_all()) and then calls
> device_synchronize().
>
> Looking at atc_terminate_all(), it currently stops the hardware and moves the
> active cyclic descriptor to the desc_terminated list, but then immediately
> extracts and frees all lists:
>
> atc_terminate_all() {
>         ...
>         if (atchan->desc) {
>                 vchan_terminate_vdesc(&atchan->desc->vd);
>                 ...
>         }
>
>         vchan_get_all_descriptors(&atchan->vc, &list);
>         ...
>         vchan_dma_desc_free_list(&atchan->vc, &list);
> }
>
> If the period completion tasklet (vchan_complete()) is concurrently executing
> on another CPU, the cyclic descriptor will be freed out from under it. By the
> time atc_synchronize() is called to wait for the tasklet via tasklet_kill(),
> the memory is already freed.
>
> Can this result in a use-after-free when the tasklet calls
> dmaengine_desc_callback_invoke() and computes an address based on the freed
> pointer?
>
> Should atc_terminate_all() avoid extracting and freeing the desc_terminated
> list so vchan_synchronize() can clean it up safely after the tasklet finishes?
>

Please check this issue

Frank

> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1