Message-ID: <sashiko-outbox-207449@kernel.org>
From: sashiko-bot@kernel.org
Subject: Re: [PATCH v4 17/33] dmaengine: ste_dma40: convert per-channel
 tasklet to dmaengine BH
Reply-To: sashiko-reviews@lists.linux.dev
To: "Allen Pais" <allen.lkml@gmail.com>
Cc: dmaengine@vger.kernel.org, vkoul@kernel.org, Frank.Li@kernel.org
In-Reply-To: <33ed46c14f6be61dd5c839b3b307512749e0d067.1794315350.git.allen.lkml@gmail.com>
References: <cover.1794315350.git.allen.lkml@gmail.com>
 <33ed46c14f6be61dd5c839b3b307512749e0d067.1794315350.git.allen.lkml@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Tue, 10 Nov 2026 17:08:14 +0000
Precedence: list

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)
>  	}
> =20
>  	d40c->pending_tx +=3D callbacks;
> -	tasklet_schedule(&d40c->tasklet);
> +	dmaengine_schedule_bh(&d40c->chan);
> =20
>  }
> =20
> -static void dma_tasklet(struct tasklet_struct *t)
> +static void dma_tasklet(struct dma_chan *chan)
>  {
> -	struct d40_chan *d40c =3D from_tasklet(d40c, t, tasklet);
> +	struct d40_chan *d40c =3D 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--;
> =20
>  	if (d40c->pending_tx)
> -		tasklet_schedule(&d40c->tasklet);
> +		dmaengine_schedule_bh(&d40c->chan);
> =20
>  	spin_unlock_irqrestore(&d40c->lock, flags);
> =20
> @@ -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);
> =20
> -		tasklet_setup(&d40c->tasklet, dma_tasklet);
> +		dmaengine_init_bh(&d40c->chan, dma_tasklet);
> =20
>  		list_add_tail(&d40c->chan.device_node,
>  			      &dma->channels);
>  	}
>  }
> =20
> -static void d40_kill_tasklets(void *data)
> +static void d40_kill_bh(void *data)
>  {
>  	struct dma_device *dma =3D data;
>  	struct d40_chan *d40c;
> @@ -3091,7 +3088,7 @@ static void d40_kill_tasklets(void *data)
> =20
>  	list_for_each_entry(chan, &dma->channels, device_node) {
>  		d40c =3D 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.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/cover.1794315350.gi=
t.allen.lkml@gmail.com?part=3D17