Re: [PATCH v4 17/33] dmaengine: ste_dma40: convert per-channel tasklet to dmaengine BH

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] dmaengine: ste_dma40: incomplete BH teardown in d40_kill_bh()
  on self-requeue
--

commit 04d3037c70d29e8460f8499e686daa8b7686fd6c
Author: Allen Pais <allen.lkml@gmail.com>

dmaengine: ste_dma40: convert per-channel tasklet to dmaengine BH

This commit replaces the per-channel tasklet in ste_dma40 with the shared
dmaengine bottom-half helper.

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 8b89d44f9b0d3..a06008d8dd275 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c

[ ... ]

> @@ -1730,13 +1727,13 @@ static void dma_tc_handle(struct d40_chan *d40c)
>  	}
>  
>  	d40c->pending_tx += callbacks;
> -	tasklet_schedule(&d40c->tasklet);
> +	dmaengine_schedule_bh(&d40c->chan);
>  
>  }
>  
> -static void dma_tasklet(struct tasklet_struct *t)
> +static void dma_tasklet(struct dma_chan *chan)
>  {
> -	struct d40_chan *d40c = from_tasklet(d40c, t, tasklet);
> +	struct d40_chan *d40c = container_of(chan, struct d40_chan, chan);
>  	struct d40_desc *d40d;
>  	unsigned long flags;
>  	bool callback_active;
> @@ -1786,7 +1783,7 @@ static void dma_tasklet(struct tasklet_struct *t)
>  	d40c->pending_tx--;
>  
>  	if (d40c->pending_tx)
> -		tasklet_schedule(&d40c->tasklet);
> +		dmaengine_schedule_bh(&d40c->chan);
>  
>  	spin_unlock_irqrestore(&d40c->lock, flags);
>  
> @@ -3076,14 +3073,14 @@ static void __init d40_chan_init(struct d40_base *base, struct dma_device *dma,
>  		INIT_LIST_HEAD(&d40c->client);
>  		INIT_LIST_HEAD(&d40c->prepare_queue);
>  
> -		tasklet_setup(&d40c->tasklet, dma_tasklet);
> +		dmaengine_init_bh(&d40c->chan, dma_tasklet);
>  
>  		list_add_tail(&d40c->chan.device_node,
>  			      &dma->channels);
>  	}
>  }
>  
> -static void d40_kill_tasklets(void *data)
> +static void d40_kill_bh(void *data)
>  {
>  	struct dma_device *dma = data;
>  	struct d40_chan *d40c;
> @@ -3091,7 +3088,7 @@ static void d40_kill_tasklets(void *data)
>  
>  	list_for_each_entry(chan, &dma->channels, device_node) {
>  		d40c = container_of(chan, struct d40_chan, chan);
> -		tasklet_kill(&d40c->tasklet);
> +		dmaengine_kill_bh(&d40c->chan);
>  	}
>  }

[Severity: Medium]
Can d40_kill_bh() return while d40c->chan.bh_work is still queued if
dma_tasklet() re-schedules itself?

dma_tasklet() processes a single transfer per invocation and calls
dmaengine_schedule_bh(&d40c->chan) when d40c->pending_tx remains non-zero
(for instance, after multiple elapsed cyclic periods in dma_tc_handle()).
Previously, tasklet_kill() held TASKLET_STATE_SCHED while waiting for
TASKLET_STATE_RUN to clear, preventing dma_tasklet() from re-scheduling
itself during teardown.

dmaengine_kill_bh() calls dmaengine_flush_bh_work(), which only runs
flush_work(&chan->bh_work).  Because process_one_work() clears
WORK_STRUCT_PENDING_BIT before invoking dma_tasklet(), a self-requeue in
dma_tasklet() queues chan->bh_work behind the flush barrier:

CPU0                                  CPU1 (dmaengine_bh_wq)
d40_kill_bh()
  dmaengine_kill_bh(&d40c->chan)
    flush_work(&chan->bh_work)
                                      process_one_work()
                                        dma_tasklet()
                                          d40c->pending_tx--;
                                          if (d40c->pending_tx)
                                            dmaengine_schedule_bh(...)
      /* flush barrier completes */
destroy_cache() / devres release
                                      process_one_work()
                                        dma_tasklet()  <-- use-after-free

When d40_kill_bh() runs during probe unwind, d40c->chan.bh_work can
execute after base->desc_slab, base->lcla_pool, and base have been freed,
causing a use-after-free.  Using disable_work_sync() or cancel_work_sync()
in dmaengine_kill_bh(), or draining all pending transfers inside
dma_tasklet() instead of re-scheduling, would avoid leaving bh_work queued
after teardown.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794315350.git.allen.lkml@gmail.com?part=17