* [PATCH v2 01/64] dmaengine: add tasklet-backed channel BH helpers
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:56 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 02/64] dmaengine: back channel BH helpers with WQ_BH Allen Pais
` (62 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
DMAengine drivers commonly use a per-channel tasklet to invoke client
callbacks. Add helpers that initialize, schedule, and kill a channel
bottom half, with an initial tasklet-backed implementation that preserves
the existing execution context.
Convert virt-dma to the new API and remove its private tasklet. While
touching the completion handler, avoid forming a result pointer from a
NULL cyclic descriptor.
This establishes a backend-independent API before changing how channel
bottom halves are dispatched.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/dmaengine.c | 42 +++++++++++++++++++++++++++++++++++++++
drivers/dma/virt-dma.c | 12 +++++------
drivers/dma/virt-dma.h | 7 +++----
include/linux/dmaengine.h | 28 ++++++++++++++++++++++++++
4 files changed, 79 insertions(+), 10 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 9049171df857..f43aeecb47f9 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -1428,6 +1428,48 @@ static void dmaengine_destroy_unmap_pool(void)
}
}
+static void dma_chan_bh_entry(struct tasklet_struct *tasklet)
+{
+ struct dma_chan *chan = from_tasklet(chan, tasklet, bh_tasklet);
+ dma_chan_bh_work_fn fn = READ_ONCE(chan->bh_work_fn);
+
+ if (fn)
+ fn(chan);
+}
+
+void dma_chan_init_bh(struct dma_chan *chan, dma_chan_bh_work_fn fn)
+{
+ if (WARN_ON(!fn))
+ return;
+
+ if (WARN_ON(chan->bh_work_initialized))
+ return;
+
+ chan->bh_work_fn = fn;
+ tasklet_setup(&chan->bh_tasklet, dma_chan_bh_entry);
+ chan->bh_work_initialized = true;
+}
+EXPORT_SYMBOL_GPL(dma_chan_init_bh);
+
+bool dma_chan_schedule_bh(struct dma_chan *chan)
+{
+ if (WARN_ON(!chan->bh_work_initialized))
+ return false;
+
+ tasklet_schedule(&chan->bh_tasklet);
+ return true;
+}
+EXPORT_SYMBOL_GPL(dma_chan_schedule_bh);
+
+void dma_chan_kill_bh(struct dma_chan *chan)
+{
+ if (!chan->bh_work_initialized)
+ return;
+
+ tasklet_kill(&chan->bh_tasklet);
+}
+EXPORT_SYMBOL_GPL(dma_chan_kill_bh);
+
static int __init dmaengine_init_unmap_pool(void)
{
int i;
diff --git a/drivers/dma/virt-dma.c b/drivers/dma/virt-dma.c
index 7961172a780d..89fcd51ddcab 100644
--- a/drivers/dma/virt-dma.c
+++ b/drivers/dma/virt-dma.c
@@ -77,12 +77,12 @@ struct virt_dma_desc *vchan_find_desc(struct virt_dma_chan *vc,
EXPORT_SYMBOL_GPL(vchan_find_desc);
/*
- * This tasklet handles the completion of a DMA descriptor by
- * calling its callback and freeing it.
+ * This bottom-half handler completes a DMA descriptor by invoking its
+ * callback and freeing it.
*/
-static void vchan_complete(struct tasklet_struct *t)
+static void vchan_complete(struct dma_chan *chan)
{
- struct virt_dma_chan *vc = from_tasklet(vc, t, task);
+ struct virt_dma_chan *vc = to_virt_chan(chan);
struct virt_dma_desc *vd, *_vd;
struct dmaengine_desc_callback cb;
LIST_HEAD(head);
@@ -98,7 +98,7 @@ static void vchan_complete(struct tasklet_struct *t)
}
spin_unlock_irq(&vc->lock);
- dmaengine_desc_callback_invoke(&cb, &vd->tx_result);
+ dmaengine_desc_callback_invoke(&cb, vd ? &vd->tx_result : NULL);
list_for_each_entry_safe(vd, _vd, &head, node) {
dmaengine_desc_get_callback(&vd->tx, &cb);
@@ -131,7 +131,7 @@ void vchan_init(struct virt_dma_chan *vc, struct dma_device *dmadev)
INIT_LIST_HEAD(&vc->desc_completed);
INIT_LIST_HEAD(&vc->desc_terminated);
- tasklet_setup(&vc->task, vchan_complete);
+ dma_chan_init_bh(&vc->chan, vchan_complete);
vc->chan.device = dmadev;
list_add_tail(&vc->chan.device_node, &dmadev->channels);
diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
index 59d9eabc8b67..abe2a4475744 100644
--- a/drivers/dma/virt-dma.h
+++ b/drivers/dma/virt-dma.h
@@ -21,7 +21,6 @@ struct virt_dma_desc {
struct virt_dma_chan {
struct dma_chan chan;
- struct tasklet_struct task;
void (*desc_free)(struct virt_dma_desc *);
spinlock_t lock;
@@ -106,7 +105,7 @@ static inline void vchan_cookie_complete(struct virt_dma_desc *vd)
vd, cookie);
list_add_tail(&vd->node, &vc->desc_completed);
- tasklet_schedule(&vc->task);
+ dma_chan_schedule_bh(&vc->chan);
}
/**
@@ -137,7 +136,7 @@ static inline void vchan_cyclic_callback(struct virt_dma_desc *vd)
struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);
vc->cyclic = vd;
- tasklet_schedule(&vc->task);
+ dma_chan_schedule_bh(&vc->chan);
}
/**
@@ -223,7 +222,7 @@ static inline void vchan_synchronize(struct virt_dma_chan *vc)
LIST_HEAD(head);
unsigned long flags;
- tasklet_kill(&vc->task);
+ dma_chan_kill_bh(&vc->chan);
spin_lock_irqsave(&vc->lock, flags);
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index b3d251c9734e..c670781b3150 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -12,6 +12,7 @@
#include <linux/scatterlist.h>
#include <linux/bitmap.h>
#include <linux/types.h>
+#include <linux/interrupt.h>
#include <asm/page.h>
/**
@@ -295,6 +296,10 @@ enum dma_desc_metadata_mode {
DESC_METADATA_ENGINE = BIT(1),
};
+struct dma_chan;
+
+typedef void (*dma_chan_bh_work_fn)(struct dma_chan *chan);
+
/**
* struct dma_chan_percpu - the per-CPU part of struct dma_chan
* @memcpy_count: transaction counter
@@ -334,6 +339,9 @@ struct dma_router {
* @router: pointer to the DMA router structure
* @route_data: channel specific data for the router
* @private: private data for certain client-channel associations
+ * @bh_tasklet: bottom-half tasklet stored per-channel
+ * @bh_work_fn: callback executed when @bh_tasklet runs
+ * @bh_work_initialized: indicates whether @bh_tasklet has been initialized
*/
struct dma_chan {
struct dma_device *device;
@@ -359,6 +367,9 @@ struct dma_chan {
void *route_data;
void *private;
+ struct tasklet_struct bh_tasklet;
+ dma_chan_bh_work_fn bh_work_fn;
+ bool bh_work_initialized;
};
/**
@@ -1529,6 +1540,9 @@ struct dma_chan *devm_dma_request_chan(struct device *dev, const char *name);
void dma_release_channel(struct dma_chan *chan);
int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps);
+void dma_chan_init_bh(struct dma_chan *chan, dma_chan_bh_work_fn fn);
+bool dma_chan_schedule_bh(struct dma_chan *chan);
+void dma_chan_kill_bh(struct dma_chan *chan);
#else
static inline struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type)
{
@@ -1576,6 +1590,20 @@ static inline int dma_get_slave_caps(struct dma_chan *chan,
{
return -ENXIO;
}
+
+static inline void dma_chan_init_bh(struct dma_chan *chan,
+ dma_chan_bh_work_fn fn)
+{
+}
+
+static inline bool dma_chan_schedule_bh(struct dma_chan *chan)
+{
+ return false;
+}
+
+static inline void dma_chan_kill_bh(struct dma_chan *chan)
+{
+}
#endif
static inline int dmaengine_desc_set_reuse(struct dma_async_tx_descriptor *tx)
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 01/64] dmaengine: add tasklet-backed channel BH helpers
2026-07-27 20:28 ` [PATCH v2 01/64] dmaengine: add tasklet-backed channel BH helpers Allen Pais
@ 2026-07-27 20:56 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:56 UTC (permalink / raw)
To: Allen Pais; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] API breakage leading to widespread build failures in downstream DMA drivers due to the removal of the `task` field from `struct virt_dma_chan` without updating its consumers.
Pre-existing issues:
- [Critical] Race condition causing a Use-After-Free of a cyclic descriptor's `tx_result` pointer when a channel is concurrently terminated.
--
--- Patch [1]: [PATCH v2 01/64] dmaengine: add tasklet-backed channel BH helpers ---
commit 9141d715bfba5e26881326cc343ca063eaf8458a
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: add tasklet-backed channel BH helpers
This patch introduces helpers to initialize, schedule, and kill a channel
bottom half. It converts virt-dma to use the new API, replacing its private
tasklet while preserving the execution context.
> diff --git a/drivers/dma/virt-dma.c b/drivers/dma/virt-dma.c
> index 7961172a780dd..89fcd51ddcabe 100644
> --- a/drivers/dma/virt-dma.c
> +++ b/drivers/dma/virt-dma.c
[ ... ]
> @@ -98,7 +98,7 @@ static void vchan_complete(struct tasklet_struct *t)
> }
> spin_unlock_irq(&vc->lock);
>
> - dmaengine_desc_callback_invoke(&cb, &vd->tx_result);
> + dmaengine_desc_callback_invoke(&cb, vd ? &vd->tx_result : NULL);
[Severity: Critical]
This is a pre-existing issue, but does dropping vc->lock just before
referencing vd->tx_result open a race window for a use-after-free?
If a concurrent dmaengine_terminate_sync() calls the driver's terminate_all
callback (like bcm2835_dma_terminate_all()), the driver could acquire the
lock and free vd via vchan_dma_desc_free_list():
CPU1
vchan_complete()
spin_unlock_irq(&vc->lock);
CPU2
dmaengine_terminate_sync()
bcm2835_dma_terminate_all()
spin_lock_irqsave(&vc->lock, flags);
vchan_dma_desc_free_list() // frees vd
spin_unlock_irqrestore(&vc->lock, flags);
CPU1
dmaengine_desc_callback_invoke(&cb, vd ? &vd->tx_result : NULL);
Could evaluating vd->tx_result here pass a dangling pointer to the client
callback if the descriptor was freed by the concurrent termination?
> list_for_each_entry_safe(vd, _vd, &head, node) {
> dmaengine_desc_get_callback(&vd->tx, &cb);
[ ... ]
> diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
> index 59d9eabc8b674..abe2a44757442 100644
> --- a/drivers/dma/virt-dma.h
> +++ b/drivers/dma/virt-dma.h
> @@ -21,7 +21,6 @@ struct virt_dma_desc {
>
> struct virt_dma_chan {
> struct dma_chan chan;
> - struct tasklet_struct task;
[Severity: Low]
Does removing the task field here cause build failures in downstream drivers?
Several DMA drivers like bcm2835-dma, hisi_dma, idma64, and tegra210-adma
directly access chan->vc.task during driver removal or error paths to kill
the tasklet. Without updating those consumers, they will fail to compile.
> void (*desc_free)(struct virt_dma_desc *);
>
> spinlock_t lock;
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=1
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 02/64] dmaengine: back channel BH helpers with WQ_BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
2026-07-27 20:28 ` [PATCH v2 01/64] dmaengine: add tasklet-backed channel BH helpers Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:28 ` [PATCH v2 03/64] dmaengine: apple-admac: use dma_chan BH callback Allen Pais
` (61 subsequent siblings)
63 siblings, 0 replies; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
Replace the tasklet implementation of the channel BH helpers with a
dedicated WQ_BH | WQ_PERCPU workqueue. The public dma_chan_*_bh() API and
its softirq execution context remain unchanged.
Keep the workqueue operations internal to dmaengine. Drain scheduled work
in dma_chan_kill_bh() to preserve the completion semantics of
tasklet_kill().
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/dmaengine.c | 59 ++++++++++++++++++++++++++++++++-------
include/linux/dmaengine.h | 10 +++----
2 files changed, 54 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index f43aeecb47f9..8b1b17794956 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -54,6 +54,7 @@
#include <linux/rcupdate.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
+#include <linux/workqueue.h>
#include "dmaengine.h"
@@ -61,6 +62,7 @@ static DEFINE_MUTEX(dma_list_mutex);
static DEFINE_IDA(dma_ida);
static LIST_HEAD(dma_device_list);
static long dmaengine_ref_count;
+static struct workqueue_struct *dmaengine_bh_wq;
/* --- debugfs implementation --- */
#ifdef CONFIG_DEBUG_FS
@@ -1428,9 +1430,34 @@ static void dmaengine_destroy_unmap_pool(void)
}
}
-static void dma_chan_bh_entry(struct tasklet_struct *tasklet)
+static void dmaengine_destroy_bh_wq(void)
{
- struct dma_chan *chan = from_tasklet(chan, tasklet, bh_tasklet);
+ if (!dmaengine_bh_wq)
+ return;
+
+ destroy_workqueue(dmaengine_bh_wq);
+ dmaengine_bh_wq = NULL;
+}
+
+static bool dmaengine_queue_bh_work(struct work_struct *work)
+{
+ if (WARN_ON(!dmaengine_bh_wq))
+ return false;
+
+ return queue_work(dmaengine_bh_wq, work);
+}
+
+static void dmaengine_flush_bh_work(struct work_struct *work)
+{
+ if (!work)
+ return;
+
+ flush_work(work);
+}
+
+static void dma_chan_bh_entry(struct work_struct *work)
+{
+ struct dma_chan *chan = container_of(work, struct dma_chan, bh_work);
dma_chan_bh_work_fn fn = READ_ONCE(chan->bh_work_fn);
if (fn)
@@ -1446,7 +1473,7 @@ void dma_chan_init_bh(struct dma_chan *chan, dma_chan_bh_work_fn fn)
return;
chan->bh_work_fn = fn;
- tasklet_setup(&chan->bh_tasklet, dma_chan_bh_entry);
+ INIT_WORK(&chan->bh_work, dma_chan_bh_entry);
chan->bh_work_initialized = true;
}
EXPORT_SYMBOL_GPL(dma_chan_init_bh);
@@ -1456,8 +1483,7 @@ bool dma_chan_schedule_bh(struct dma_chan *chan)
if (WARN_ON(!chan->bh_work_initialized))
return false;
- tasklet_schedule(&chan->bh_tasklet);
- return true;
+ return dmaengine_queue_bh_work(&chan->bh_work);
}
EXPORT_SYMBOL_GPL(dma_chan_schedule_bh);
@@ -1466,7 +1492,7 @@ void dma_chan_kill_bh(struct dma_chan *chan)
if (!chan->bh_work_initialized)
return;
- tasklet_kill(&chan->bh_tasklet);
+ dmaengine_flush_bh_work(&chan->bh_work);
}
EXPORT_SYMBOL_GPL(dma_chan_kill_bh);
@@ -1666,15 +1692,28 @@ EXPORT_SYMBOL_GPL(dma_run_dependencies);
static int __init dma_bus_init(void)
{
- int err = dmaengine_init_unmap_pool();
+ int err;
+ dmaengine_bh_wq = alloc_workqueue("dmaengine_bh",
+ WQ_BH | WQ_PERCPU, 0);
+ if (!dmaengine_bh_wq)
+ return -ENOMEM;
+
+ err = dmaengine_init_unmap_pool();
if (err)
- return err;
+ goto err_destroy_wq;
err = class_register(&dma_devclass);
- if (!err)
- dmaengine_debugfs_init();
+ if (err)
+ goto err_destroy_pool;
+ dmaengine_debugfs_init();
+ return 0;
+
+err_destroy_pool:
+ dmaengine_destroy_unmap_pool();
+err_destroy_wq:
+ dmaengine_destroy_bh_wq();
return err;
}
arch_initcall(dma_bus_init);
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index c670781b3150..c87974d5048b 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -12,7 +12,7 @@
#include <linux/scatterlist.h>
#include <linux/bitmap.h>
#include <linux/types.h>
-#include <linux/interrupt.h>
+#include <linux/workqueue.h>
#include <asm/page.h>
/**
@@ -339,9 +339,9 @@ struct dma_router {
* @router: pointer to the DMA router structure
* @route_data: channel specific data for the router
* @private: private data for certain client-channel associations
- * @bh_tasklet: bottom-half tasklet stored per-channel
- * @bh_work_fn: callback executed when @bh_tasklet runs
- * @bh_work_initialized: indicates whether @bh_tasklet has been initialized
+ * @bh_work: bottom-half work item stored per-channel
+ * @bh_work_fn: callback executed when @bh_work runs
+ * @bh_work_initialized: indicates whether @bh_work has been initialized
*/
struct dma_chan {
struct dma_device *device;
@@ -367,7 +367,7 @@ struct dma_chan {
void *route_data;
void *private;
- struct tasklet_struct bh_tasklet;
+ struct work_struct bh_work;
dma_chan_bh_work_fn bh_work_fn;
bool bh_work_initialized;
};
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* [PATCH v2 03/64] dmaengine: apple-admac: use dma_chan BH callback
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
2026-07-27 20:28 ` [PATCH v2 01/64] dmaengine: add tasklet-backed channel BH helpers Allen Pais
2026-07-27 20:28 ` [PATCH v2 02/64] dmaengine: back channel BH helpers with WQ_BH Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:28 ` [PATCH v2 04/64] dmaengine: at_xdmac: move irq bottom half to dma_chan BH Allen Pais
` (60 subsequent siblings)
63 siblings, 0 replies; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Sven Peter, Janne Grunau, Neal Gompa, asahi, linux-arm-kernel
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/apple-admac.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/dma/apple-admac.c b/drivers/dma/apple-admac.c
index 14a5ee14a481..84483247bde6 100644
--- a/drivers/dma/apple-admac.c
+++ b/drivers/dma/apple-admac.c
@@ -89,7 +89,6 @@ struct admac_chan {
unsigned int no;
struct admac_data *host;
struct dma_chan chan;
- struct tasklet_struct tasklet;
u32 carveout;
@@ -521,10 +520,7 @@ static int admac_terminate_all(struct dma_chan *chan)
list_add_tail(&adchan->current_tx->node, &adchan->to_free);
adchan->current_tx = NULL;
}
- /*
- * Descriptors can only be freed after the tasklet
- * has been killed (in admac_synchronize).
- */
+ /* Descriptors are released once the BH is flushed in admac_synchronize */
list_splice_tail_init(&adchan->submitted, &adchan->to_free);
list_splice_tail_init(&adchan->issued, &adchan->to_free);
spin_unlock_irqrestore(&adchan->lock, flags);
@@ -543,7 +539,7 @@ static void admac_synchronize(struct dma_chan *chan)
list_splice_tail_init(&adchan->to_free, &head);
spin_unlock_irqrestore(&adchan->lock, flags);
- tasklet_kill(&adchan->tasklet);
+ dma_chan_kill_bh(&adchan->chan);
list_for_each_entry_safe(adtx, _adtx, &head, node) {
list_del(&adtx->node);
@@ -662,7 +658,7 @@ static void admac_handle_status_desc_done(struct admac_data *ad, int channo)
tx->reclaimed_pos %= 2 * tx->buf_len;
admac_cyclic_write_desc(ad, channo, tx);
- tasklet_schedule(&adchan->tasklet);
+ dma_chan_schedule_bh(&adchan->chan);
}
spin_unlock_irqrestore(&adchan->lock, flags);
}
@@ -712,9 +708,9 @@ static irqreturn_t admac_interrupt(int irq, void *devid)
return IRQ_HANDLED;
}
-static void admac_chan_tasklet(struct tasklet_struct *t)
+static void admac_chan_bh(struct dma_chan *chan)
{
- struct admac_chan *adchan = from_tasklet(adchan, t, tasklet);
+ struct admac_chan *adchan = to_admac_chan(chan);
struct admac_tx *adtx;
struct dmaengine_desc_callback cb;
struct dmaengine_result tx_result;
@@ -886,7 +882,7 @@ static int admac_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&adchan->issued);
INIT_LIST_HEAD(&adchan->to_free);
list_add_tail(&adchan->chan.device_node, &dma->channels);
- tasklet_setup(&adchan->tasklet, admac_chan_tasklet);
+ dma_chan_init_bh(&adchan->chan, admac_chan_bh);
}
err = reset_control_reset(ad->rstc);
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* [PATCH v2 04/64] dmaengine: at_xdmac: move irq bottom half to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (2 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 03/64] dmaengine: apple-admac: use dma_chan BH callback Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:56 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 05/64] dmaengine: ep93xx: hook callbacks via " Allen Pais
` (59 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Ludovic Desroches, linux-arm-kernel
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/at_xdmac.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index 901971e8bae6..aa918043e37e 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -228,7 +228,6 @@ struct at_xdmac_chan {
u32 save_cndc;
u32 irq_status;
unsigned long status;
- struct tasklet_struct tasklet;
struct dma_slave_config sconfig;
spinlock_t lock;
@@ -1759,9 +1758,9 @@ static void at_xdmac_handle_error(struct at_xdmac_chan *atchan)
/* Then continue with usual descriptor management */
}
-static void at_xdmac_tasklet(struct tasklet_struct *t)
+static void at_xdmac_tasklet(struct dma_chan *chan)
{
- struct at_xdmac_chan *atchan = from_tasklet(atchan, t, tasklet);
+ struct at_xdmac_chan *atchan = to_at_xdmac_chan(chan);
struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
struct at_xdmac_desc *desc;
struct dma_async_tx_descriptor *txd;
@@ -1865,7 +1864,7 @@ static irqreturn_t at_xdmac_interrupt(int irq, void *dev_id)
if (atchan->irq_status & (AT_XDMAC_CIS_RBEIS | AT_XDMAC_CIS_WBEIS))
at_xdmac_write(atxdmac, AT_XDMAC_GD, atchan->mask);
- tasklet_schedule(&atchan->tasklet);
+ dma_chan_schedule_bh(&atchan->chan);
ret = IRQ_HANDLED;
}
@@ -2317,7 +2316,7 @@ static int at_xdmac_probe(struct platform_device *pdev)
return PTR_ERR(atxdmac->clk);
}
- /* Do not use dev res to prevent races with tasklet */
+ /* Do not use devm resources to prevent races with the BH worker */
ret = request_irq(atxdmac->irq, at_xdmac_interrupt, 0, "at_xdmac", atxdmac);
if (ret) {
dev_err(&pdev->dev, "can't request irq\n");
@@ -2397,7 +2396,7 @@ static int at_xdmac_probe(struct platform_device *pdev)
spin_lock_init(&atchan->lock);
INIT_LIST_HEAD(&atchan->xfers_list);
INIT_LIST_HEAD(&atchan->free_descs_list);
- tasklet_setup(&atchan->tasklet, at_xdmac_tasklet);
+ dma_chan_init_bh(&atchan->chan, at_xdmac_tasklet);
/* Clear pending interrupts. */
while (at_xdmac_chan_read(atchan, AT_XDMAC_CIS))
@@ -2458,7 +2457,7 @@ static void at_xdmac_remove(struct platform_device *pdev)
for (i = 0; i < atxdmac->dma.chancnt; i++) {
struct at_xdmac_chan *atchan = &atxdmac->chan[i];
- tasklet_kill(&atchan->tasklet);
+ dma_chan_kill_bh(&atchan->chan);
at_xdmac_free_chan_resources(&atchan->chan);
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 04/64] dmaengine: at_xdmac: move irq bottom half to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 04/64] dmaengine: at_xdmac: move irq bottom half to dma_chan BH Allen Pais
@ 2026-07-27 20:56 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:56 UTC (permalink / raw)
To: Allen Pais; +Cc: Frank.Li, dmaengine, vkoul
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but an initialization race condition exists: `request_irq()` is called before hardware clocks are enabled and DMA channels are initialized, which can cause a kernel panic or infinite loop.
- [High] This problem wasn't introduced by this patch, but there is a resource leak and Use-After-Free due to a bypassed cleanup loop in `at_xdmac_remove()`.
- [High] This problem wasn't introduced by this patch, but the teardown order is flawed: Hardware clocks and PM runtime are disabled before freeing the IRQ and stopping BH workers.
- [High] This problem wasn't introduced by this patch, but out-of-bounds memory accesses and list corruption occur due to an unchecked `list_first_entry()` in the BH worker.
- [High] This problem wasn't introduced by this patch, but there is a sleeping in atomic bug: The error handler calls `pm_runtime_resume_and_get()` while holding an IRQ-disabled spinlock.
--
--- Patch [4]: [PATCH v2 04/64] dmaengine: at_xdmac: move irq bottom half to dma_chan BH ---
commit 91d37fd9a6c41425b793e9ed1792794d52b35014
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: at_xdmac: move irq bottom half to dma_chan BH
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index 901971e8bae69..aa918043e37ec 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
[ ... ]
> @@ -1759,9 +1758,9 @@ static void at_xdmac_handle_error(struct at_xdmac_chan *atchan)
> /* Then continue with usual descriptor management */
> }
>
> -static void at_xdmac_tasklet(struct tasklet_struct *t)
> +static void at_xdmac_tasklet(struct dma_chan *chan)
> {
> - struct at_xdmac_chan *atchan = from_tasklet(atchan, t, tasklet);
> + struct at_xdmac_chan *atchan = to_at_xdmac_chan(chan);
> struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
> struct at_xdmac_desc *desc;
[Severity: High]
This is a pre-existing issue, but does this function lack a list empty check?
Later in at_xdmac_tasklet(), the code does:
desc = list_first_entry(&atchan->xfers_list, struct at_xdmac_desc,
xfer_node);
if (!desc->active_xfer) { ... }
If at_xdmac_device_terminate_all() runs concurrently and empties
xfers_list, resuming the BH worker might resolve an invalid pointer for
the empty list, read out-of-bounds for desc->active_xfer, and corrupt
the list.
[Severity: High]
This is also a pre-existing issue, but can this path sleep while atomic?
In at_xdmac_tasklet(), an IRQ-disabled spinlock is acquired:
spin_lock_irq(&atchan->lock);
...
if (atchan->irq_status & error_mask)
at_xdmac_handle_error(atchan);
Inside at_xdmac_handle_error(), it calls pm_runtime_resume_and_get(),
which can sleep since the device isn't marked as pm_runtime_irq_safe().
Could this lead to a scheduling while atomic bug if the device's
runtime status is not RPM_ACTIVE?
[ ... ]
> @@ -2317,7 +2316,7 @@ static int at_xdmac_probe(struct platform_device *pdev)
> return PTR_ERR(atxdmac->clk);
> }
>
> - /* Do not use dev res to prevent races with tasklet */
> + /* Do not use devm resources to prevent races with the BH worker */
> ret = request_irq(atxdmac->irq, at_xdmac_interrupt, 0, "at_xdmac", atxdmac);
[Severity: High]
This isn't a bug introduced by this patch, but an initialization race
condition appears to exist here. Is it safe to call request_irq() before
the hardware clocks are enabled via clk_prepare_enable() and before
dma_async_device_register() initializes the channel count?
If an early or pending interrupt fires immediately after request_irq(),
the handler at_xdmac_interrupt() will execute while the controller's
clock is gated, which could trigger a bus fault.
Even if clocks were enabled, atxdmac->dma.chancnt is still 0 at this point.
The handler would skip the channel check loop, fail to clear the interrupt,
and spin infinitely in its do-while(pending) loop.
[ ... ]
> @@ -2458,7 +2457,7 @@ static void at_xdmac_remove(struct platform_device *pdev)
> for (i = 0; i < atxdmac->dma.chancnt; i++) {
> struct at_xdmac_chan *atchan = &atxdmac->chan[i];
>
> - tasklet_kill(&atchan->tasklet);
> + dma_chan_kill_bh(&atchan->chan);
> at_xdmac_free_chan_resources(&atchan->chan);
> }
> }
[Severity: High]
This is a pre-existing issue, but does the teardown order in
at_xdmac_remove() and the probe error path risk a bus fault?
In at_xdmac_remove(), clk_disable_unprepare() and pm_runtime_disable() are
called before free_irq(). If a pending interrupt fires before free_irq()
completes, the interrupt handler will attempt to read from the clock-gated
device registers, which could trigger a synchronous external abort.
[Severity: High]
This is another pre-existing issue, but will this cleanup loop ever execute?
Earlier in at_xdmac_remove(), dma_async_device_unregister() is called,
which iterates over all registered channels and decrements device->chancnt
to 0.
When this loop attempts to use atxdmac->dma.chancnt as its limit, it is
completely bypassed, meaning dma_chan_kill_bh() and
at_xdmac_free_chan_resources() are never called, potentially causing a
resource leak and use-after-free for pending BH workers.
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=4
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 05/64] dmaengine: ep93xx: hook callbacks via dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (3 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 04/64] dmaengine: at_xdmac: move irq bottom half to dma_chan BH Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:59 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 06/64] dmaengine: fsldma: migrate tasklet to " Allen Pais
` (58 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/ep93xx_dma.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c
index 311e55a97ba9..8f5200750fe9 100644
--- a/drivers/dma/ep93xx_dma.c
+++ b/drivers/dma/ep93xx_dma.c
@@ -167,7 +167,6 @@ struct ep93xx_dma_chan_cfg {
* @dma_cfg: channel number, direction
* @irq: interrupt number of the channel
* @clk: clock used by this channel
- * @tasklet: channel specific tasklet used for callbacks
* @lock: lock protecting the fields following
* @flags: flags for the channel
* @buffer: which buffer to use next (0/1)
@@ -196,7 +195,6 @@ struct ep93xx_dma_chan {
struct ep93xx_dma_chan_cfg dma_cfg;
int irq;
struct clk *clk;
- struct tasklet_struct tasklet;
/* protects the fields following */
spinlock_t lock;
unsigned long flags;
@@ -801,9 +799,9 @@ static void ep93xx_dma_advance_work(struct ep93xx_dma_chan *edmac)
spin_unlock_irqrestore(&edmac->lock, flags);
}
-static void ep93xx_dma_tasklet(struct tasklet_struct *t)
+static void ep93xx_dma_tasklet(struct dma_chan *chan)
{
- struct ep93xx_dma_chan *edmac = from_tasklet(edmac, t, tasklet);
+ struct ep93xx_dma_chan *edmac = to_ep93xx_dma_chan(chan);
struct ep93xx_dma_desc *desc, *d;
struct dmaengine_desc_callback cb;
LIST_HEAD(list);
@@ -858,12 +856,12 @@ static irqreturn_t ep93xx_dma_interrupt(int irq, void *dev_id)
switch (edmac->edma->hw_interrupt(edmac)) {
case INTERRUPT_DONE:
desc->complete = true;
- tasklet_schedule(&edmac->tasklet);
+ dma_chan_schedule_bh(&edmac->chan);
break;
case INTERRUPT_NEXT_BUFFER:
if (test_bit(EP93XX_DMA_IS_CYCLIC, &edmac->flags))
- tasklet_schedule(&edmac->tasklet);
+ dma_chan_schedule_bh(&edmac->chan);
break;
default:
@@ -1420,7 +1418,7 @@ static struct ep93xx_dma_engine *ep93xx_dma_of_probe(struct platform_device *pde
INIT_LIST_HEAD(&edmac->active);
INIT_LIST_HEAD(&edmac->queue);
INIT_LIST_HEAD(&edmac->free_list);
- tasklet_setup(&edmac->tasklet, ep93xx_dma_tasklet);
+ dma_chan_init_bh(&edmac->chan, ep93xx_dma_tasklet);
list_add_tail(&edmac->chan.device_node,
&dma_dev->channels);
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 05/64] dmaengine: ep93xx: hook callbacks via dma_chan BH
2026-07-27 20:28 ` [PATCH v2 05/64] dmaengine: ep93xx: hook callbacks via " Allen Pais
@ 2026-07-27 20:59 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:59 UTC (permalink / raw)
To: Allen Pais; +Cc: dmaengine, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing `dma_chan_kill_bh()` in `ep93xx_dma_synchronize()` and `ep93xx_dma_free_chan_resources()` violates the dmaengine API and leads to premature callback execution for new clients.
- [High] The ep93xx_dma driver is vulnerable to a Use-After-Free and kernel panic if unbound via sysfs, due to the lack of a .remove callback and missing .suppress_bind_attrs = true.
--
--- Patch [5]: [PATCH v2 05/64] dmaengine: ep93xx: hook callbacks via dma_chan BH ---
commit e03616bfd332cb745207d3436713d688064c9cb5
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: ep93xx: hook callbacks via dma_chan BH
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c
> index 311e55a97ba9f..8f5200750fe98 100644
> --- a/drivers/dma/ep93xx_dma.c
> +++ b/drivers/dma/ep93xx_dma.c
[ ... ]
> @@ -1420,7 +1418,7 @@ static struct ep93xx_dma_engine *ep93xx_dma_of_probe(struct platform_device *pde
> INIT_LIST_HEAD(&edmac->active);
> INIT_LIST_HEAD(&edmac->queue);
> INIT_LIST_HEAD(&edmac->free_list);
> - tasklet_setup(&edmac->tasklet, ep93xx_dma_tasklet);
> + dma_chan_init_bh(&edmac->chan, ep93xx_dma_tasklet);
>
> list_add_tail(&edmac->chan.device_node,
> &dma_dev->channels);
[Severity: High]
This is a pre-existing issue, but I noticed ep93xx_dma_synchronize() and
ep93xx_dma_free_chan_resources() do not cancel the pending BH work.
If a client calls dma_terminate_all(), the active list is emptied, but a
scheduled BH is not cancelled because the driver does not call
dma_chan_kill_bh() (previously missing tasklet_kill()).
When the channel is reallocated, the pending BH can run, see the new client's
descriptor, and unconditionally invoke its callback before completion:
drivers/dma/ep93xx_dma.c:ep93xx_dma_tasklet()
...
desc = ep93xx_dma_get_active(edmac);
if (desc) {
if (desc->complete) {
/* mark descriptor complete for non cyclic case only */
if (!test_bit(EP93XX_DMA_IS_CYCLIC, &edmac->flags))
dma_cookie_complete(&desc->txd);
list_splice_init(&edmac->active, &list);
}
dmaengine_desc_get_callback(&desc->txd, &cb);
}
...
Does this need to call dma_chan_kill_bh() during synchronization or channel
release to prevent state corruption for new DMA clients?
[Severity: High]
This isn't a bug introduced by this patch, but the ep93xx_dma driver appears
vulnerable to a Use-After-Free and kernel panic if unbound via sysfs.
Because the driver lacks a .remove callback and doesn't set
.suppress_bind_attrs = true in ep93xx_dma_driver:
drivers/dma/ep93xx_dma.c:
static struct platform_driver ep93xx_dma_driver = {
.driver = {
.name = "ep93xx-dma",
.of_match_table = ep93xx_dma_of_ids,
},
.probe = ep93xx_dma_probe,
};
If a privileged user unbinds the driver via sysfs, it never calls
dma_async_device_unregister() or frees IRQs. The device core's devres
mechanism will free the edma and edmac structures, but any subsequent IRQ
firing or scheduled BH execution will dereference the freed memory.
Should this driver include a .remove callback or set
.suppress_bind_attrs = true to prevent this?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=5
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 06/64] dmaengine: fsldma: migrate tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (4 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 05/64] dmaengine: ep93xx: hook callbacks via " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:56 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 07/64] dmaengine: fsl_raid: run completions via " Allen Pais
` (57 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Zhang Wei, linuxppc-dev
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/fsldma.c | 10 +++++-----
drivers/dma/fsldma.h | 1 -
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 22d62d958abd..1cb6ce327078 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -968,18 +968,18 @@ static irqreturn_t fsldma_chan_irq(int irq, void *data)
chan_err(chan, "irq: unhandled sr 0x%08x\n", stat);
/*
- * Schedule the tasklet to handle all cleanup of the current
+ * Queue the BH worker to handle all cleanup of the current
* transaction. It will start a new transaction if there is
* one pending.
*/
- tasklet_schedule(&chan->tasklet);
+ dma_chan_schedule_bh(&chan->common);
chan_dbg(chan, "irq: Exit\n");
return IRQ_HANDLED;
}
-static void dma_do_tasklet(struct tasklet_struct *t)
+static void dma_do_tasklet(struct dma_chan *c)
{
- struct fsldma_chan *chan = from_tasklet(chan, t, tasklet);
+ struct fsldma_chan *chan = to_fsl_chan(c);
chan_dbg(chan, "tasklet entry\n");
@@ -1152,7 +1152,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
}
fdev->chan[chan->id] = chan;
- tasklet_setup(&chan->tasklet, dma_do_tasklet);
+ dma_chan_init_bh(&chan->common, dma_do_tasklet);
snprintf(chan->name, sizeof(chan->name), "chan%d", chan->id);
/* Initialize the channel */
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index d7b7a3138b85..cab33d010d51 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -173,7 +173,6 @@ struct fsldma_chan {
struct device *dev; /* Channel device */
int irq; /* Channel IRQ */
int id; /* Raw id of this channel */
- struct tasklet_struct tasklet;
u32 feature;
bool idle; /* DMA controller is idle */
#ifdef CONFIG_PM
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 06/64] dmaengine: fsldma: migrate tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 06/64] dmaengine: fsldma: migrate tasklet to " Allen Pais
@ 2026-07-27 20:56 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:56 UTC (permalink / raw)
To: Allen Pais; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing dma_chan_kill_bh() during channel removal and resource freeing leaves pending bottom-half work queued, leading to use-after-free and NULL pointer dereference crashes.
- [High] Lock inversion deadlock when invoking client callbacks with the channel descriptor lock held.
--
--- Patch [6]: [PATCH v2 06/64] dmaengine: fsldma: migrate tasklet to dma_chan BH ---
commit eba223ea472b760cffa1e53fb0bb48d7fa6057d5
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: fsldma: migrate tasklet to dma_chan BH
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 22d62d958abd9..1cb6ce3270782 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -968,18 +968,18 @@ static irqreturn_t fsldma_chan_irq(int irq, void *data)
[ ... ]
> -static void dma_do_tasklet(struct tasklet_struct *t)
> +static void dma_do_tasklet(struct dma_chan *c)
> {
> - struct fsldma_chan *chan = from_tasklet(chan, t, tasklet);
> + struct fsldma_chan *chan = to_fsl_chan(c);
>
> chan_dbg(chan, "tasklet entry\n");
[Severity: High]
This is a pre-existing issue, but does this cleanup path have a lock inversion
deadlock?
dma_do_tasklet() acquires chan->desc_lock before calling
fsldma_cleanup_descriptors(). While holding this lock, it invokes
fsldma_run_tx_complete_actions(), which executes the client callback via
dmaengine_desc_get_callback_invoke() and processes dependencies via
dma_run_dependencies().
If the client callback submits a new DMA transaction, or if a dependent
transaction is submitted for the same channel, it calls fsl_dma_tx_submit():
drivers/dma/fsldma.c:fsl_dma_tx_submit() {
...
spin_lock_bh(&chan->desc_lock);
...
}
Since the non-recursive lock is already held on the same CPU, won't this
lead to a deadlock in softirq context?
> @@ -1152,7 +1152,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
> }
>
> fdev->chan[chan->id] = chan;
> - tasklet_setup(&chan->tasklet, dma_do_tasklet);
> + dma_chan_init_bh(&chan->common, dma_do_tasklet);
[Severity: High]
This is a pre-existing issue, but is it possible for pending bottom-half work
to run after the channel is removed?
If an IRQ fires and queues the BH right before unbinding,
fsl_dma_chan_remove() frees the channel structure and
fsl_dma_free_chan_resources() destroys the descriptor pool without calling
dma_chan_kill_bh() (or tasklet_kill() previously).
When the pending BH work later executes dma_do_tasklet(), could it dereference
the freed channel or pass a NULL desc_pool to dma_pool_free()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=6
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 07/64] dmaengine: fsl_raid: run completions via dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (5 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 06/64] dmaengine: fsldma: migrate tasklet to " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:55 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 08/64] dmaengine: imx-dma: flip per-chan tasklet to " Allen Pais
` (56 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/fsl_raid.c | 11 ++++++-----
drivers/dma/fsl_raid.h | 1 -
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
index 99945845d8b5..e91c18d3c327 100644
--- a/drivers/dma/fsl_raid.c
+++ b/drivers/dma/fsl_raid.c
@@ -155,9 +155,10 @@ static void fsl_re_cleanup_descs(struct fsl_re_chan *re_chan)
fsl_re_issue_pending(&re_chan->chan);
}
-static void fsl_re_dequeue(struct tasklet_struct *t)
+static void fsl_re_dequeue(struct dma_chan *chan)
{
- struct fsl_re_chan *re_chan = from_tasklet(re_chan, t, irqtask);
+ struct fsl_re_chan *re_chan = container_of(chan, struct fsl_re_chan,
+ chan);
struct fsl_re_desc *desc, *_desc;
struct fsl_re_hw_desc *hwdesc;
unsigned long flags;
@@ -224,7 +225,7 @@ static irqreturn_t fsl_re_isr(int irq, void *data)
/* Clear interrupt */
out_be32(&re_chan->jrregs->jr_interrupt_status, FSL_RE_CLR_INTR);
- tasklet_schedule(&re_chan->irqtask);
+ dma_chan_schedule_bh(&re_chan->chan);
return IRQ_HANDLED;
}
@@ -670,7 +671,7 @@ static int fsl_re_chan_probe(struct platform_device *ofdev,
snprintf(chan->name, sizeof(chan->name), "re_jr%02d", q);
chandev = &chan_ofdev->dev;
- tasklet_setup(&chan->irqtask, fsl_re_dequeue);
+ dma_chan_init_bh(&chan->chan, fsl_re_dequeue);
ret = request_irq(chan->irq, fsl_re_isr, 0, chan->name, chandev);
if (ret) {
@@ -846,7 +847,7 @@ static int fsl_re_probe(struct platform_device *ofdev)
static void fsl_re_remove_chan(struct fsl_re_chan *chan)
{
- tasklet_kill(&chan->irqtask);
+ dma_chan_kill_bh(&chan->chan);
dma_pool_free(chan->re_dev->hw_desc_pool, chan->inb_ring_virt_addr,
chan->inb_phys_addr);
diff --git a/drivers/dma/fsl_raid.h b/drivers/dma/fsl_raid.h
index 69d743c04973..a40001a4db58 100644
--- a/drivers/dma/fsl_raid.h
+++ b/drivers/dma/fsl_raid.h
@@ -275,7 +275,6 @@ struct fsl_re_chan {
struct dma_chan chan;
struct fsl_re_chan_cfg *jrregs;
int irq;
- struct tasklet_struct irqtask;
u32 alloc_count;
/* hw descriptor ring for inbound queue*/
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 07/64] dmaengine: fsl_raid: run completions via dma_chan BH
2026-07-27 20:28 ` [PATCH v2 07/64] dmaengine: fsl_raid: run completions via " Allen Pais
@ 2026-07-27 20:55 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:55 UTC (permalink / raw)
To: Allen Pais; +Cc: dmaengine, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver requests hardware interrupts via request_irq() without devres, but completely fails to call free_irq() in both the probe error paths and device removal path.
- [High] The interrupt is requested before the channel's data structures and driver data are fully initialized, leading to a potential NULL pointer dereference upon early interrupts.
- [High] The driver ignores the return value of fsl_re_chan_probe() during initialization, leading to NULL pointer dereferences in the cleanup path when iterating over failed channels.
- [High] Channel memory is freed before the device is unregistered from the DMA engine, allowing concurrent clients to use freed memory.
--
--- Patch [7]: [PATCH v2 07/64] dmaengine: fsl_raid: run completions via dma_chan BH ---
commit ab56ab807ba67c4bf376a52b60efd1139b5215d9
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: fsl_raid: run completions via dma_chan BH
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
> --- a/drivers/dma/fsl_raid.c
> +++ b/drivers/dma/fsl_raid.c
[ ... ]
> @@ -670,7 +671,7 @@ static int fsl_re_chan_probe(struct platform_device *ofdev,
> snprintf(chan->name, sizeof(chan->name), "re_jr%02d", q);
>
> chandev = &chan_ofdev->dev;
> - tasklet_setup(&chan->irqtask, fsl_re_dequeue);
> + dma_chan_init_bh(&chan->chan, fsl_re_dequeue);
>
> ret = request_irq(chan->irq, fsl_re_isr, 0, chan->name, chandev);
[Severity: High]
This is a pre-existing issue, but is it safe to request the IRQ before the
channel's driver data and locks are initialized?
If an interrupt fires immediately after request_irq() succeeds, fsl_re_isr()
will be executed:
fsl_re_isr()
re_chan = dev_get_drvdata((struct device *)data);
irqstate = in_be32(&re_chan->jrregs->jr_interrupt_status);
Since dev_set_drvdata() hasn't been called yet, dev_get_drvdata() returns
NULL, which is immediately dereferenced.
[Severity: High]
This is a pre-existing issue, but does this function leak the requested IRQ
on failure?
The driver requests the hardware interrupt without devres. If any subsequent
initialization fails, the function jumps to the err_free labels without
calling free_irq().
> if (ret) {
[ ... ]
> @@ -846,7 +847,7 @@ static int fsl_re_probe(struct platform_device *ofdev)
>
> static void fsl_re_remove_chan(struct fsl_re_chan *chan)
> {
> - tasklet_kill(&chan->irqtask);
> + dma_chan_kill_bh(&chan->chan);
[Severity: High]
This is a pre-existing issue, but can this lead to a NULL pointer
dereference if the probe previously failed?
In fsl_re_probe(), the driver ignores the return value of
fsl_re_chan_probe():
fsl_re_probe()
rc = of_device_is_compatible(child, "fsl,raideng-v1.0-job-ring");
if (rc) {
fsl_re_chan_probe(ofdev, child, ridx++, off);
re_priv->total_chans++;
}
If a channel fails to probe, its pointer in the re_jrs array remains NULL
but total_chans is still incremented. Later, when cleaning up, could passing
that NULL pointer into fsl_re_remove_chan() and calling dma_chan_kill_bh()
cause a kernel crash?
[Severity: High]
This is a pre-existing issue, but shouldn't there be a free_irq() call here?
Since the IRQ was requested without devres during probe, omitting free_irq()
in the teardown path leaves the IRQ active after the driver memory structures
are freed. If an interrupt occurs later, could it lead to a use-after-free?
[Severity: High]
This is a pre-existing issue, but does the driver free channel memory while
the DMA device is still registered?
Looking at fsl_re_remove(), it iterates over all channels and calls
fsl_re_remove_chan() before unregistering the DMA device:
fsl_re_remove()
for (i = 0; i < re_priv->total_chans; i++)
fsl_re_remove_chan(re_priv->re_jrs[i]);
dma_async_device_unregister(&re_priv->dma_dev);
Could concurrent clients still submit requests while this is happening,
leading fsl_re_issue_pending() to copy data into a freed ring buffer?
>
> dma_pool_free(chan->re_dev->hw_desc_pool, chan->inb_ring_virt_addr,
> chan->inb_phys_addr);
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=7
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 08/64] dmaengine: imx-dma: flip per-chan tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (6 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 07/64] dmaengine: fsl_raid: run completions via " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:57 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 09/64] dmaengine: ioat: convert cleanup " Allen Pais
` (55 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, imx,
linux-arm-kernel
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/imx-dma.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
index 81c6276436f8..3ff0bfbfa5ff 100644
--- a/drivers/dma/imx-dma.c
+++ b/drivers/dma/imx-dma.c
@@ -145,7 +145,6 @@ struct imxdma_channel {
struct imxdma_engine *imxdma;
unsigned int channel;
- struct tasklet_struct dma_tasklet;
struct list_head ld_free;
struct list_head ld_queue;
struct list_head ld_active;
@@ -344,8 +343,8 @@ static void imxdma_watchdog(struct timer_list *t)
imx_dmav1_writel(imxdma, 0, DMA_CCR(channel));
- /* Tasklet watchdog error handler */
- tasklet_schedule(&imxdmac->dma_tasklet);
+ /* BH watchdog error handler */
+ dma_chan_schedule_bh(&imxdmac->chan);
dev_dbg(imxdma->dev, "channel %d: watchdog timeout!\n",
imxdmac->channel);
}
@@ -390,8 +389,8 @@ static irqreturn_t imxdma_err_handler(int irq, void *dev_id)
imx_dmav1_writel(imxdma, 1 << i, DMA_DBOSR);
errcode |= IMX_DMA_ERR_BUFFER;
}
- /* Tasklet error handler */
- tasklet_schedule(&imxdma->channel[i].dma_tasklet);
+ /* BH error handler */
+ dma_chan_schedule_bh(&imxdma->channel[i].chan);
dev_warn(imxdma->dev,
"DMA timeout on channel %d -%s%s%s%s\n", i,
@@ -448,8 +447,8 @@ static void dma_irq_handle_channel(struct imxdma_channel *imxdmac)
imx_dmav1_writel(imxdma, tmp, DMA_CCR(chno));
if (imxdma_chan_is_doing_cyclic(imxdmac))
- /* Tasklet progression */
- tasklet_schedule(&imxdmac->dma_tasklet);
+ /* BH progression */
+ dma_chan_schedule_bh(&imxdmac->chan);
return;
}
@@ -462,8 +461,8 @@ static void dma_irq_handle_channel(struct imxdma_channel *imxdmac)
out:
imx_dmav1_writel(imxdma, 0, DMA_CCR(chno));
- /* Tasklet irq */
- tasklet_schedule(&imxdmac->dma_tasklet);
+ /* Schedule the IRQ BH */
+ dma_chan_schedule_bh(&imxdmac->chan);
}
static irqreturn_t dma_irq_handler(int irq, void *dev_id)
@@ -592,9 +591,10 @@ static int imxdma_xfer_desc(struct imxdma_desc *d)
return 0;
}
-static void imxdma_tasklet(struct tasklet_struct *t)
+static void imxdma_tasklet(struct dma_chan *chan)
{
- struct imxdma_channel *imxdmac = from_tasklet(imxdmac, t, dma_tasklet);
+ struct imxdma_channel *imxdmac = container_of(chan, struct imxdma_channel,
+ chan);
struct imxdma_engine *imxdma = imxdmac->imxdma;
struct imxdma_desc *desc, *next_desc;
unsigned long flags;
@@ -1142,7 +1142,7 @@ static int __init imxdma_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&imxdmac->ld_free);
INIT_LIST_HEAD(&imxdmac->ld_active);
- tasklet_setup(&imxdmac->dma_tasklet, imxdma_tasklet);
+ dma_chan_init_bh(&imxdmac->chan, imxdma_tasklet);
imxdmac->chan.device = &imxdma->dma_device;
dma_cookie_init(&imxdmac->chan);
imxdmac->channel = i;
@@ -1211,7 +1211,7 @@ static void imxdma_free_irq(struct platform_device *pdev, struct imxdma_engine *
if (!is_imx1_dma(imxdma))
disable_irq(imxdmac->irq);
- tasklet_kill(&imxdmac->dma_tasklet);
+ dma_chan_kill_bh(&imxdmac->chan);
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 08/64] dmaengine: imx-dma: flip per-chan tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 08/64] dmaengine: imx-dma: flip per-chan tasklet to " Allen Pais
@ 2026-07-27 20:57 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:57 UTC (permalink / raw)
To: Allen Pais; +Cc: vkoul, Frank.Li, dmaengine, imx
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
Pre-existing issues:
- [High] The per-channel watchdog timer is not synchronously canceled during driver teardown, leading to a Use-After-Free.
- [High] Pending BH works are not canceled on the probe error path, causing a Use-After-Free.
- [High] Early interrupts cause a NULL pointer dereference because IRQs are requested before channel lists are initialized.
- [High] Descriptor state is accessed after being placed on the free list and unlocking the spinlock, leading to a TOCTOU race with prep functions.
- [High] Channel state and lists are read and modified without proper lock protection in descriptor preparation functions.
- [High] The driver frees DMA descriptors in `imxdma_free_chan_resources` without synchronizing with the completion BH, causing a Use-After-Free.
--
--- Patch [8]: [PATCH v2 08/64] dmaengine: imx-dma: flip per-chan tasklet to dma_chan BH ---
commit 6c1578dd7a98a9ceebdc129c266b46d0c2bdf6e3
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: imx-dma: flip per-chan tasklet to dma_chan BH
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
> index 81c6276436f81..3ff0bfbfa5ffa 100644
> --- a/drivers/dma/imx-dma.c
> +++ b/drivers/dma/imx-dma.c
[ ... ]
> @@ -592,9 +591,10 @@ static int imxdma_xfer_desc(struct imxdma_desc *d)
> return 0;
> }
>
> -static void imxdma_tasklet(struct tasklet_struct *t)
> +static void imxdma_tasklet(struct dma_chan *chan)
> {
> - struct imxdma_channel *imxdmac = from_tasklet(imxdmac, t, dma_tasklet);
> + struct imxdma_channel *imxdmac = container_of(chan, struct imxdma_channel,
> + chan);
> struct imxdma_engine *imxdma = imxdmac->imxdma;
> struct imxdma_desc *desc, *next_desc;
> unsigned long flags;
[Severity: High]
This is a pre-existing issue, but is it safe to invoke the callback after
releasing the spinlock and moving the descriptor to the free list?
At the end of imxdma_tasklet(), the lock is released and the callback is
subsequently invoked directly from the descriptor:
out:
spin_unlock_irqrestore(&imxdma->lock, flags);
dmaengine_desc_get_callback_invoke(&desc->desc, NULL);
}
Since the descriptor is already on the free list, couldn't a concurrent
call to imxdma_prep_dma_memcpy() pick this exact descriptor from ld_free
and clear desc->desc.callback = NULL, leading to execution of a NULL or
corrupted callback?
[Severity: High]
This isn't a bug introduced by this patch, but how are the channel lists
synchronized against concurrent preparation?
The completion BH actively splices and modifies ld_free and ld_active while
holding imxdma->lock. However, prep functions like imxdma_prep_dma_memcpy()
read ld_free and call list_first_entry() without acquiring the channel lock:
drivers/dma/imx-dma.c:imxdma_prep_dma_memcpy() {
...
if (list_empty(&imxdmac->ld_free) ||
imxdma_chan_is_doing_cyclic(imxdmac))
return NULL;
desc = list_first_entry(&imxdmac->ld_free, struct imxdma_desc, node);
...
}
Could this lead to linked list corruption or NULL pointer dereferences if
clients actively submit transfers while completions fire?
[ ... ]
> @@ -1142,7 +1142,7 @@ static int __init imxdma_probe(struct platform_device *pdev)
> INIT_LIST_HEAD(&imxdmac->ld_free);
> INIT_LIST_HEAD(&imxdmac->ld_active);
>
> - tasklet_setup(&imxdmac->dma_tasklet, imxdma_tasklet);
> + dma_chan_init_bh(&imxdmac->chan, imxdma_tasklet);
> imxdmac->chan.device = &imxdma->dma_device;
> dma_cookie_init(&imxdmac->chan);
> imxdmac->channel = i;
[Severity: High]
This is a pre-existing issue, but what happens if the probe fails after
this point?
Looking at the error path later in imxdma_probe():
err_of_dma_controller:
dma_async_device_unregister(&imxdma->dma_device);
disable_dma_ahb_clk:
clk_disable_unprepare(imxdma->dma_ahb);
disable_dma_ipg_clk:
clk_disable_unprepare(imxdma->dma_ipg);
return ret;
It exits without calling dma_chan_kill_bh() or imxdma_free_irq(). If a
spurious or lingering hardware interrupt triggers before
dma_async_device_register() completes, dma_irq_handler() schedules the BH.
If a subsequent initialization step fails, devres frees the device memory
but the scheduled BH is never canceled, which might lead to a use-after-free.
[Severity: High]
This isn't a bug introduced by this patch, but does the early interrupt
setup risk a NULL pointer dereference?
The IRQ is requested earlier in imxdma_probe():
ret = devm_request_irq(&pdev->dev, irq,
dma_irq_handler, 0, "DMA", imxdma);
This happens well before the list heads like ld_active are initialized in
the loop shown in the diff above. If an interrupt fires early,
dma_irq_handle_channel() checks the list:
if (list_empty(&imxdmac->ld_active)) {
spin_unlock_irqrestore(&imxdma->lock, flags);
goto out;
}
Since memory is 0-initialized, ld_active.next is NULL. This makes
list_empty() evaluate to false, leading directly to a crash via
list_first_entry() right below it.
[ ... ]
> @@ -1211,7 +1211,7 @@ static void imxdma_free_irq(struct platform_device *pdev, struct imxdma_engine *
> if (!is_imx1_dma(imxdma))
> disable_irq(imxdmac->irq);
>
> - tasklet_kill(&imxdmac->dma_tasklet);
> + dma_chan_kill_bh(&imxdmac->chan);
> }
> }
[Severity: High]
This is a pre-existing issue, but is it safe to kill the BH here without
also stopping the watchdog timer?
This function stops the BH but omits timer_shutdown_sync(&imxdmac->watchdog),
leaving it active. If the driver is unbound via sysfs while a transfer is
active, imxdma_remove() invokes imxdma_free_irq() and then unregisters
and frees the engine. Couldn't the still-armed timer fire on freed memory
and schedule the destroyed BH workqueue?
[Severity: High]
This isn't a bug introduced by this patch, but does
imxdma_free_chan_resources() safely synchronize with this BH?
When a channel is released, descriptors are indiscriminately freed without
stopping or waiting for the BH to finish:
drivers/dma/imx-dma.c:imxdma_free_chan_resources() {
...
list_for_each_entry_safe(desc, _desc, &imxdmac->ld_free, node) {
kfree(desc);
imxdmac->descs_allocated--;
}
...
}
Since the driver lacks a device_synchronize callback and does not call
dma_chan_kill_bh() during channel release, could the completion BH be
actively running on another CPU trying to invoke the callback for a
descriptor that is being freed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=8
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 09/64] dmaengine: ioat: convert cleanup tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (7 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 08/64] dmaengine: imx-dma: flip per-chan tasklet to " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:38 ` Dave Jiang
2026-07-27 21:02 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 10/64] dmaengine: mmp_pdma: replace per-chan tasklet with " Allen Pais
` (54 subsequent siblings)
63 siblings, 2 replies; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Dave Jiang, Thomas Weißschuh, David Matlack, Lukas Wunner,
Giovanni Cabiddu
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/ioat/dma.c | 14 +++++++-------
drivers/dma/ioat/dma.h | 3 +--
drivers/dma/ioat/init.c | 2 +-
3 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c
index ee93b029f9e3..827b46c6354a 100644
--- a/drivers/dma/ioat/dma.c
+++ b/drivers/dma/ioat/dma.c
@@ -110,7 +110,7 @@ irqreturn_t ioat_dma_do_interrupt(int irq, void *data)
for_each_set_bit(bit, &attnstatus, BITS_PER_LONG) {
ioat_chan = ioat_chan_by_index(instance, bit);
if (test_bit(IOAT_RUN, &ioat_chan->state))
- tasklet_schedule(&ioat_chan->cleanup_task);
+ dma_chan_schedule_bh(&ioat_chan->dma_chan);
}
writeb(intrctrl, instance->reg_base + IOAT_INTRCTRL_OFFSET);
@@ -127,7 +127,7 @@ irqreturn_t ioat_dma_do_interrupt_msix(int irq, void *data)
struct ioatdma_chan *ioat_chan = data;
if (test_bit(IOAT_RUN, &ioat_chan->state))
- tasklet_schedule(&ioat_chan->cleanup_task);
+ dma_chan_schedule_bh(&ioat_chan->dma_chan);
return IRQ_HANDLED;
}
@@ -161,11 +161,11 @@ void ioat_stop(struct ioatdma_chan *ioat_chan)
/* flush inflight timers */
timer_delete_sync(&ioat_chan->timer);
- /* flush inflight tasklet runs */
- tasklet_kill(&ioat_chan->cleanup_task);
+ /* flush inflight BH runs */
+ dma_chan_kill_bh(&ioat_chan->dma_chan);
/* final cleanup now that everything is quiesced and can't re-arm */
- ioat_cleanup_event(&ioat_chan->cleanup_task);
+ ioat_cleanup_event(&ioat_chan->dma_chan);
}
static void __ioat_issue_pending(struct ioatdma_chan *ioat_chan)
@@ -690,9 +690,9 @@ static void ioat_cleanup(struct ioatdma_chan *ioat_chan)
spin_unlock_bh(&ioat_chan->cleanup_lock);
}
-void ioat_cleanup_event(struct tasklet_struct *t)
+void ioat_cleanup_event(struct dma_chan *c)
{
- struct ioatdma_chan *ioat_chan = from_tasklet(ioat_chan, t, cleanup_task);
+ struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
ioat_cleanup(ioat_chan);
if (!test_bit(IOAT_RUN, &ioat_chan->state))
diff --git a/drivers/dma/ioat/dma.h b/drivers/dma/ioat/dma.h
index e8a880f338c6..4bdb1ae937e4 100644
--- a/drivers/dma/ioat/dma.h
+++ b/drivers/dma/ioat/dma.h
@@ -111,7 +111,6 @@ struct ioatdma_chan {
struct ioatdma_device *ioat_dma;
dma_addr_t completion_dma;
u64 *completion;
- struct tasklet_struct cleanup_task;
struct kobject kobj;
/* ioat v2 / v3 channel attributes
@@ -385,7 +384,7 @@ int ioat_reset_hw(struct ioatdma_chan *ioat_chan);
enum dma_status
ioat_tx_status(struct dma_chan *c, dma_cookie_t cookie,
struct dma_tx_state *txstate);
-void ioat_cleanup_event(struct tasklet_struct *t);
+void ioat_cleanup_event(struct dma_chan *c);
void ioat_timer_event(struct timer_list *t);
int ioat_check_space_lock(struct ioatdma_chan *ioat_chan, int num_descs);
void ioat_issue_pending(struct dma_chan *chan);
diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 737496391109..314676e1cbc4 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -765,7 +765,7 @@ ioat_init_channel(struct ioatdma_device *ioat_dma,
list_add_tail(&ioat_chan->dma_chan.device_node, &dma->channels);
ioat_dma->idx[idx] = ioat_chan;
timer_setup(&ioat_chan->timer, ioat_timer_event, 0);
- tasklet_setup(&ioat_chan->cleanup_task, ioat_cleanup_event);
+ dma_chan_init_bh(&ioat_chan->dma_chan, ioat_cleanup_event);
}
#define IOAT_NUM_SRC_TEST 6 /* must be <= 8 */
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 09/64] dmaengine: ioat: convert cleanup tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 09/64] dmaengine: ioat: convert cleanup " Allen Pais
@ 2026-07-27 20:38 ` Dave Jiang
2026-07-27 21:02 ` sashiko-bot
1 sibling, 0 replies; 137+ messages in thread
From: Dave Jiang @ 2026-07-27 20:38 UTC (permalink / raw)
To: Allen Pais, Vinod Koul, Frank Li
Cc: dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Thomas Weißschuh, David Matlack, Lukas Wunner,
Giovanni Cabiddu
On 7/27/26 1:28 PM, Allen Pais wrote:
> Replace the per-channel tasklet with the shared dma_chan BH helper.
> The handler continues to run in softirq context while dmaengine owns
> the common scheduling and teardown mechanism.
>
> Signed-off-by: Allen Pais <allen.lkml@gmail.com>
Acked-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/dma/ioat/dma.c | 14 +++++++-------
> drivers/dma/ioat/dma.h | 3 +--
> drivers/dma/ioat/init.c | 2 +-
> 3 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c
> index ee93b029f9e3..827b46c6354a 100644
> --- a/drivers/dma/ioat/dma.c
> +++ b/drivers/dma/ioat/dma.c
> @@ -110,7 +110,7 @@ irqreturn_t ioat_dma_do_interrupt(int irq, void *data)
> for_each_set_bit(bit, &attnstatus, BITS_PER_LONG) {
> ioat_chan = ioat_chan_by_index(instance, bit);
> if (test_bit(IOAT_RUN, &ioat_chan->state))
> - tasklet_schedule(&ioat_chan->cleanup_task);
> + dma_chan_schedule_bh(&ioat_chan->dma_chan);
> }
>
> writeb(intrctrl, instance->reg_base + IOAT_INTRCTRL_OFFSET);
> @@ -127,7 +127,7 @@ irqreturn_t ioat_dma_do_interrupt_msix(int irq, void *data)
> struct ioatdma_chan *ioat_chan = data;
>
> if (test_bit(IOAT_RUN, &ioat_chan->state))
> - tasklet_schedule(&ioat_chan->cleanup_task);
> + dma_chan_schedule_bh(&ioat_chan->dma_chan);
>
> return IRQ_HANDLED;
> }
> @@ -161,11 +161,11 @@ void ioat_stop(struct ioatdma_chan *ioat_chan)
> /* flush inflight timers */
> timer_delete_sync(&ioat_chan->timer);
>
> - /* flush inflight tasklet runs */
> - tasklet_kill(&ioat_chan->cleanup_task);
> + /* flush inflight BH runs */
> + dma_chan_kill_bh(&ioat_chan->dma_chan);
>
> /* final cleanup now that everything is quiesced and can't re-arm */
> - ioat_cleanup_event(&ioat_chan->cleanup_task);
> + ioat_cleanup_event(&ioat_chan->dma_chan);
> }
>
> static void __ioat_issue_pending(struct ioatdma_chan *ioat_chan)
> @@ -690,9 +690,9 @@ static void ioat_cleanup(struct ioatdma_chan *ioat_chan)
> spin_unlock_bh(&ioat_chan->cleanup_lock);
> }
>
> -void ioat_cleanup_event(struct tasklet_struct *t)
> +void ioat_cleanup_event(struct dma_chan *c)
> {
> - struct ioatdma_chan *ioat_chan = from_tasklet(ioat_chan, t, cleanup_task);
> + struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
>
> ioat_cleanup(ioat_chan);
> if (!test_bit(IOAT_RUN, &ioat_chan->state))
> diff --git a/drivers/dma/ioat/dma.h b/drivers/dma/ioat/dma.h
> index e8a880f338c6..4bdb1ae937e4 100644
> --- a/drivers/dma/ioat/dma.h
> +++ b/drivers/dma/ioat/dma.h
> @@ -111,7 +111,6 @@ struct ioatdma_chan {
> struct ioatdma_device *ioat_dma;
> dma_addr_t completion_dma;
> u64 *completion;
> - struct tasklet_struct cleanup_task;
> struct kobject kobj;
>
> /* ioat v2 / v3 channel attributes
> @@ -385,7 +384,7 @@ int ioat_reset_hw(struct ioatdma_chan *ioat_chan);
> enum dma_status
> ioat_tx_status(struct dma_chan *c, dma_cookie_t cookie,
> struct dma_tx_state *txstate);
> -void ioat_cleanup_event(struct tasklet_struct *t);
> +void ioat_cleanup_event(struct dma_chan *c);
> void ioat_timer_event(struct timer_list *t);
> int ioat_check_space_lock(struct ioatdma_chan *ioat_chan, int num_descs);
> void ioat_issue_pending(struct dma_chan *chan);
> diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
> index 737496391109..314676e1cbc4 100644
> --- a/drivers/dma/ioat/init.c
> +++ b/drivers/dma/ioat/init.c
> @@ -765,7 +765,7 @@ ioat_init_channel(struct ioatdma_device *ioat_dma,
> list_add_tail(&ioat_chan->dma_chan.device_node, &dma->channels);
> ioat_dma->idx[idx] = ioat_chan;
> timer_setup(&ioat_chan->timer, ioat_timer_event, 0);
> - tasklet_setup(&ioat_chan->cleanup_task, ioat_cleanup_event);
> + dma_chan_init_bh(&ioat_chan->dma_chan, ioat_cleanup_event);
> }
>
> #define IOAT_NUM_SRC_TEST 6 /* must be <= 8 */
^ permalink raw reply [flat|nested] 137+ messages in thread* Re: [PATCH v2 09/64] dmaengine: ioat: convert cleanup tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 09/64] dmaengine: ioat: convert cleanup " Allen Pais
2026-07-27 20:38 ` Dave Jiang
@ 2026-07-27 21:02 ` sashiko-bot
1 sibling, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 21:02 UTC (permalink / raw)
To: Allen Pais; +Cc: Frank.Li, dmaengine, vkoul
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The teardown sequence in `ioat_stop()` cancels the timer using `timer_delete_sync()`, but it can be subsequently re-armed by the final cleanup sequence, leading to a use-after-free when channel resources are freed.
- [High] `ioat_shutdown()` does not flush inflight BH runs, which can lead to a use-after-free or crash on module unload.
--
--- Patch [9]: [PATCH v2 09/64] dmaengine: ioat: convert cleanup tasklet to dma_chan BH ---
commit 1d64b3893af8e57fe59c7d2f05e40d26e9aec652
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: ioat: convert cleanup tasklet to dma_chan BH
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c
> index ee93b029f9e3d..827b46c6354ad 100644
> --- a/drivers/dma/ioat/dma.c
> +++ b/drivers/dma/ioat/dma.c
[ ... ]
> @@ -161,11 +161,11 @@ void ioat_stop(struct ioatdma_chan *ioat_chan)
> /* flush inflight timers */
> timer_delete_sync(&ioat_chan->timer);
>
> - /* flush inflight tasklet runs */
> - tasklet_kill(&ioat_chan->cleanup_task);
> + /* flush inflight BH runs */
> + dma_chan_kill_bh(&ioat_chan->dma_chan);
>
> /* final cleanup now that everything is quiesced and can't re-arm */
> - ioat_cleanup_event(&ioat_chan->cleanup_task);
> + ioat_cleanup_event(&ioat_chan->dma_chan);
[Severity: High]
This is a pre-existing issue, but does this sequence inadvertently re-arm
the timer during teardown?
After timer_delete_sync() is called, ioat_cleanup_event() triggers a call
chain that can re-arm the timer unconditionally:
ioat_stop()
timer_delete_sync(&ioat_chan->timer);
ioat_cleanup_event()
ioat_cleanup()
ioat_cleanup_preamble()
mod_timer(&ioat_chan->timer, jiffies + COMPLETION_TIMEOUT);
If the timer is re-armed here, could the delayed timer callback execute
after ioat_free_chan_resources() frees the completion pool memory?
Would upgrading timer_delete_sync() to timer_shutdown_sync() properly
prevent the timer from being re-armed?
[Severity: High]
This is also a pre-existing issue, but does ioat_shutdown() in
drivers/dma/ioat/init.c have a similar teardown problem with the BH?
In ioat_shutdown(), the driver calls timer_delete_sync() and disables
interrupts, but it does not appear to flush inflight BH runs like it does
here in ioat_stop().
Since the IOAT_RUN bit is not cleared in ioat_shutdown():
drivers/dma/ioat/init.c:ioat_shutdown() {
...
timer_delete_sync(&ioat_chan->timer);
ioat_reset_hw(ioat_chan);
...
ioat_disable_interrupts(ioat_dma);
}
Could an interrupt firing just before ioat_disable_interrupts() successfully
schedule the BH via dma_chan_schedule_bh()?
If the module is then unloaded, could the scheduled BH execute after
release_ioatdma() has freed the channel structures, resulting in a
use-after-free?
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=9
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 10/64] dmaengine: mmp_pdma: replace per-chan tasklet with dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (8 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 09/64] dmaengine: ioat: convert cleanup " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:54 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 11/64] dmaengine: mmp_tdma: hook completions to " Allen Pais
` (53 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/mmp_pdma.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 386e85cd4882..3fd9ef59a142 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -116,7 +116,6 @@ struct mmp_pdma_chan {
* is in cyclic mode */
/* channel's basic info */
- struct tasklet_struct tasklet;
u32 dcmd;
u32 drcmr;
u32 dev_addr;
@@ -374,7 +373,7 @@ static irqreturn_t mmp_pdma_chan_handler(int irq, void *dev_id)
if (clear_chan_irq(phy) != 0)
return IRQ_NONE;
- tasklet_schedule(&phy->vchan->tasklet);
+ dma_chan_schedule_bh(&phy->vchan->chan);
return IRQ_HANDLED;
}
@@ -1047,13 +1046,11 @@ static void mmp_pdma_issue_pending(struct dma_chan *dchan)
}
/*
- * dma_do_tasklet
- * Do call back
- * Start pending list
+ * dma_do_tasklet - BH worker to process completions and kick pending work
*/
-static void dma_do_tasklet(struct tasklet_struct *t)
+static void dma_do_tasklet(struct dma_chan *c)
{
- struct mmp_pdma_chan *chan = from_tasklet(chan, t, tasklet);
+ struct mmp_pdma_chan *chan = container_of(c, struct mmp_pdma_chan, chan);
struct mmp_pdma_desc_sw *desc, *_desc;
LIST_HEAD(chain_cleanup);
unsigned long flags;
@@ -1170,7 +1167,7 @@ static int mmp_pdma_chan_init(struct mmp_pdma_device *pdev, int idx, int irq)
spin_lock_init(&chan->desc_lock);
chan->dev = pdev->dev;
chan->chan.device = &pdev->device;
- tasklet_setup(&chan->tasklet, dma_do_tasklet);
+ dma_chan_init_bh(&chan->chan, dma_do_tasklet);
INIT_LIST_HEAD(&chan->chain_pending);
INIT_LIST_HEAD(&chan->chain_running);
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 10/64] dmaengine: mmp_pdma: replace per-chan tasklet with dma_chan BH
2026-07-27 20:28 ` [PATCH v2 10/64] dmaengine: mmp_pdma: replace per-chan tasklet with " Allen Pais
@ 2026-07-27 20:54 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:54 UTC (permalink / raw)
To: Allen Pais; +Cc: Frank.Li, dmaengine, vkoul
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Unconditional dereference of phy->vchan in the interrupt handler races with channel teardown.
- [High] Missing cancellation of scheduled bottom-half work during channel teardown and device removal causes UAF and NULL pointer dereferences.
- [High] Terminating cyclic transfers frees descriptors but leaves a dangling pointer in `chan->cyclic_first`, causing UAF in the bottom-half handler.
--
--- Patch [10]: [PATCH v2 10/64] dmaengine: mmp_pdma: replace per-chan tasklet with dma_chan BH ---
commit 7ee6ebc1ebbe207f60ed8c3f6afa81414e0802c3
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: mmp_pdma: replace per-chan tasklet with dma_chan BH
This commit replaces the per-channel tasklet with the shared dma_chan BH
helper. The handler continues to run in softirq context while delegating
scheduling to the dmaengine's mechanism.
> diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
> index 386e85cd4882a..3fd9ef59a1420 100644
> --- a/drivers/dma/mmp_pdma.c
> +++ b/drivers/dma/mmp_pdma.c
[ ... ]
> @@ -374,7 +373,7 @@ static irqreturn_t mmp_pdma_chan_handler(int irq, void *dev_id)
> if (clear_chan_irq(phy) != 0)
> return IRQ_NONE;
>
> - tasklet_schedule(&phy->vchan->tasklet);
> + dma_chan_schedule_bh(&phy->vchan->chan);
[Severity: High]
This is a pre-existing issue, but does this race with channel teardown?
If mmp_pdma_free_phy() sets phy->vchan to NULL while the shared physical
interrupt remains active:
drivers/dma/mmp_pdma.c:mmp_pdma_free_phy() {
...
pchan->phy->vchan = NULL;
...
}
Could a pending interrupt be serviced immediately afterwards?
If clear_chan_irq() successfully clears the hardware interrupt and returns
zero, it appears this code will unconditionally dereference the NULL pointer
in phy->vchan when scheduling the bottom half.
> return IRQ_HANDLED;
> }
[ ... ]
> @@ -1047,13 +1046,11 @@ static void mmp_pdma_issue_pending(struct dma_chan *dchan)
> }
>
> /*
> - * dma_do_tasklet
> - * Do call back
> - * Start pending list
> + * dma_do_tasklet - BH worker to process completions and kick pending work
> */
> -static void dma_do_tasklet(struct tasklet_struct *t)
> +static void dma_do_tasklet(struct dma_chan *c)
> {
> - struct mmp_pdma_chan *chan = from_tasklet(chan, t, tasklet);
> + struct mmp_pdma_chan *chan = container_of(c, struct mmp_pdma_chan, chan);
[Severity: High]
This is a pre-existing issue, but is the bottom half properly synchronized
against device removal and channel teardown?
Looking at mmp_pdma_free_chan_resources(), the descriptor pool is destroyed
and chan->desc_pool is set to NULL:
drivers/dma/mmp_pdma.c:mmp_pdma_free_chan_resources() {
...
dma_pool_destroy(chan->desc_pool);
chan->desc_pool = NULL;
...
}
Since there isn't a device_synchronize() or explicit cancellation of the
pending bottom-half work during teardown or mmp_pdma_remove(), could this
background work execute asynchronously?
If it does, it looks like it would invoke dma_pool_free() on a NULL or
destroyed pool further down in this function, or access a chan structure
that was already freed by devres.
[Severity: High]
This isn't a bug introduced by this patch, but can this function access a
freed descriptor during cyclic transfers?
In mmp_pdma_terminate_all(), the driver correctly frees running descriptors
back to the DMA pool by calling mmp_pdma_free_desc_list(), but it doesn't
appear to clear chan->cyclic_first:
drivers/dma/mmp_pdma.c:mmp_pdma_terminate_all() {
...
mmp_pdma_free_desc_list(chan, &chan->chain_running);
...
}
If a delayed or spurious bottom-half execution triggers this function,
could it evaluate chan->cyclic_first as non-null and subsequently
use-after-free the memory to invoke the callback?
> struct mmp_pdma_desc_sw *desc, *_desc;
> LIST_HEAD(chain_cleanup);
> unsigned long flags;
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=10
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 11/64] dmaengine: mmp_tdma: hook completions to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (9 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 10/64] dmaengine: mmp_pdma: replace per-chan tasklet with " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:55 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 12/64] dmaengine: mv_xor: convert irq tasklet " Allen Pais
` (52 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/mmp_tdma.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/mmp_tdma.c b/drivers/dma/mmp_tdma.c
index ba03321eeff7..d1ed2886f0e7 100644
--- a/drivers/dma/mmp_tdma.c
+++ b/drivers/dma/mmp_tdma.c
@@ -102,7 +102,6 @@ struct mmp_tdma_chan {
struct device *dev;
struct dma_chan chan;
struct dma_async_tx_descriptor desc;
- struct tasklet_struct tasklet;
struct mmp_tdma_desc *desc_arr;
dma_addr_t desc_arr_phys;
@@ -320,7 +319,7 @@ static irqreturn_t mmp_tdma_chan_handler(int irq, void *dev_id)
struct mmp_tdma_chan *tdmac = dev_id;
if (mmp_tdma_clear_chan_irq(tdmac) == 0) {
- tasklet_schedule(&tdmac->tasklet);
+ dma_chan_schedule_bh(&tdmac->chan);
return IRQ_HANDLED;
} else
return IRQ_NONE;
@@ -346,9 +345,9 @@ static irqreturn_t mmp_tdma_int_handler(int irq, void *dev_id)
return IRQ_NONE;
}
-static void dma_do_tasklet(struct tasklet_struct *t)
+static void dma_do_tasklet(struct dma_chan *c)
{
- struct mmp_tdma_chan *tdmac = from_tasklet(tdmac, t, tasklet);
+ struct mmp_tdma_chan *tdmac = container_of(c, struct mmp_tdma_chan, chan);
dmaengine_desc_get_callback_invoke(&tdmac->desc, NULL);
}
@@ -583,7 +582,7 @@ static int mmp_tdma_chan_init(struct mmp_tdma_device *tdev,
tdmac->pool = pool;
tdmac->status = DMA_COMPLETE;
tdev->tdmac[tdmac->idx] = tdmac;
- tasklet_setup(&tdmac->tasklet, dma_do_tasklet);
+ dma_chan_init_bh(&tdmac->chan, dma_do_tasklet);
/* add the channel to tdma_chan list */
list_add_tail(&tdmac->chan.device_node,
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 11/64] dmaengine: mmp_tdma: hook completions to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 11/64] dmaengine: mmp_tdma: hook completions to " Allen Pais
@ 2026-07-27 20:55 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:55 UTC (permalink / raw)
To: Allen Pais; +Cc: Frank.Li, dmaengine, vkoul
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing teardown of bottom-half work during channel release, leading to potential use-after-free.
- [High] Incorrect devres initialization ordering for shared interrupts causes NULL pointer dereferences on probe and use-after-free on unbind.
- [High] SRAM descriptors allocated from the `gen_pool` are leaked when `mmp_tdma_prep_dma_cyclic` fails or is called multiple times.
--
--- Patch [11]: [PATCH v2 11/64] dmaengine: mmp_tdma: hook completions to dma_chan BH ---
commit c186dbcb9d7ae1ce6b3473be898cbd442e74d154
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: mmp_tdma: hook completions to dma_chan BH
This commit replaces the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine manages the
common scheduling and teardown mechanism.
> diff --git a/drivers/dma/mmp_tdma.c b/drivers/dma/mmp_tdma.c
> index ba03321eeff79..d1ed2886f0e70 100644
> --- a/drivers/dma/mmp_tdma.c
> +++ b/drivers/dma/mmp_tdma.c
[ ... ]
> @@ -320,7 +319,7 @@ static irqreturn_t mmp_tdma_chan_handler(int irq, void *dev_id)
> struct mmp_tdma_chan *tdmac = dev_id;
>
> if (mmp_tdma_clear_chan_irq(tdmac) == 0) {
> - tasklet_schedule(&tdmac->tasklet);
> + dma_chan_schedule_bh(&tdmac->chan);
> return IRQ_HANDLED;
> } else
> return IRQ_NONE;
> @@ -346,9 +345,9 @@ static irqreturn_t mmp_tdma_int_handler(int irq, void *dev_id)
> return IRQ_NONE;
> }
[Severity: High]
This is a pre-existing issue, but is it possible for this interrupt handler to
trigger before the channel structures are fully initialized?
Looking at mmp_tdma_probe(), devm_request_irq() is called with IRQF_SHARED
before the channel structures are allocated and assigned:
drivers/dma/mmp_tdma.c:mmp_tdma_probe() {
...
ret = devm_request_irq(&pdev->dev, irq,
mmp_tdma_int_handler, IRQF_SHARED, "tdma", tdev);
...
}
If the shared IRQ line fires immediately, mmp_tdma_int_handler() iterates over
tdev->tdmac[] and unconditionally passes a potentially uninitialized NULL pointer
to mmp_tdma_chan_handler(). Could this lead to a NULL pointer dereference on
probe, or a use-after-free on unbind since devres reverses the initialization
order?
> -static void dma_do_tasklet(struct tasklet_struct *t)
> +static void dma_do_tasklet(struct dma_chan *c)
> {
> - struct mmp_tdma_chan *tdmac = from_tasklet(tdmac, t, tasklet);
> + struct mmp_tdma_chan *tdmac = container_of(c, struct mmp_tdma_chan, chan);
>
> dmaengine_desc_get_callback_invoke(&tdmac->desc, NULL);
> }
> @@ -583,7 +582,7 @@ static int mmp_tdma_chan_init(struct mmp_tdma_device *tdev,
> tdmac->pool = pool;
> tdmac->status = DMA_COMPLETE;
> tdev->tdmac[tdmac->idx] = tdmac;
> - tasklet_setup(&tdmac->tasklet, dma_do_tasklet);
> + dma_chan_init_bh(&tdmac->chan, dma_do_tasklet);
[Severity: High]
This isn't a bug introduced by this patch, but should the bottom-half work be
explicitly torn down during channel release?
Currently, mmp_tdma_free_chan_resources() frees channel resources without
calling dma_chan_kill_bh():
drivers/dma/mmp_tdma.c:mmp_tdma_free_chan_resources() {
...
}
Similarly, when the device unbinds, devres frees the channel structure (tdmac)
but mmp_tdma_remove() does not explicitly kill the bottom-half work.
Could this leave pending bottom-half tasks to execute after memory is freed by
devres, leading to a use-after-free?
> /* add the channel to tdma_chan list */
> list_add_tail(&tdmac->chan.device_node,
[Severity: High]
This is a pre-existing issue, but can mmp_tdma_prep_dma_cyclic() leak SRAM
descriptors allocated from the gen_pool?
If mmp_tdma_config_write() fails, the function branches to err_out without
freeing the newly allocated descriptor:
drivers/dma/mmp_tdma.c:mmp_tdma_prep_dma_cyclic() {
...
desc = mmp_tdma_alloc_descriptor(tdmac);
if (!desc)
goto err_out;
if (mmp_tdma_config_write(chan, direction, &tdmac->slave_config))
goto err_out;
...
}
Additionally, mmp_tdma_alloc_descriptor() unconditionally assigns the newly
allocated memory to tdmac->desc_arr:
drivers/dma/mmp_tdma.c:mmp_tdma_alloc_descriptor() {
...
tdmac->desc_arr = gen_pool_dma_alloc(gpool, size, &tdmac->desc_arr_phys);
...
}
If mmp_tdma_prep_dma_cyclic() is called multiple times without an intervening
mmp_tdma_free_chan_resources(), the previous descriptor is blindly overwritten
and permanently leaked. Could this exhaust the limited device SRAM pool and
prevent further DMA operations?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=11
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 12/64] dmaengine: mv_xor: convert irq tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (10 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 11/64] dmaengine: mmp_tdma: hook completions to " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:57 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 13/64] dmaengine: mxs-dma: use dma_chan BH scheduling Allen Pais
` (51 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/mv_xor.c | 12 ++++++------
drivers/dma/mv_xor.h | 2 --
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 25ed61f1b089..3a70d565e1ef 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -327,7 +327,7 @@ static void mv_chan_slot_cleanup(struct mv_xor_chan *mv_chan)
* some descriptors are still waiting
* to be cleaned
*/
- tasklet_schedule(&mv_chan->irq_tasklet);
+ dma_chan_schedule_bh(&mv_chan->dmachan);
}
}
}
@@ -336,9 +336,9 @@ static void mv_chan_slot_cleanup(struct mv_xor_chan *mv_chan)
mv_chan->dmachan.completed_cookie = cookie;
}
-static void mv_xor_tasklet(struct tasklet_struct *t)
+static void mv_xor_tasklet(struct dma_chan *c)
{
- struct mv_xor_chan *chan = from_tasklet(chan, t, irq_tasklet);
+ struct mv_xor_chan *chan = to_mv_xor_chan(c);
spin_lock(&chan->lock);
mv_chan_slot_cleanup(chan);
@@ -372,7 +372,7 @@ mv_chan_alloc_slot(struct mv_xor_chan *mv_chan)
spin_unlock_bh(&mv_chan->lock);
/* try to free some slots if the allocation fails */
- tasklet_schedule(&mv_chan->irq_tasklet);
+ dma_chan_schedule_bh(&mv_chan->dmachan);
return NULL;
}
@@ -737,7 +737,7 @@ static irqreturn_t mv_xor_interrupt_handler(int irq, void *data)
if (intr_cause & XOR_INTR_ERRORS)
mv_chan_err_interrupt_handler(chan, intr_cause);
- tasklet_schedule(&chan->irq_tasklet);
+ dma_chan_schedule_bh(&chan->dmachan);
mv_chan_clear_eoc_cause(chan);
@@ -1107,7 +1107,7 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
mv_chan->mmr_base = xordev->xor_base;
mv_chan->mmr_high_base = xordev->xor_high_base;
- tasklet_setup(&mv_chan->irq_tasklet, mv_xor_tasklet);
+ dma_chan_init_bh(&mv_chan->dmachan, mv_xor_tasklet);
/* clear errors before enabling interrupts */
mv_chan_clear_err_status(mv_chan);
diff --git a/drivers/dma/mv_xor.h b/drivers/dma/mv_xor.h
index c87cefd38a07..27774abe34a6 100644
--- a/drivers/dma/mv_xor.h
+++ b/drivers/dma/mv_xor.h
@@ -98,7 +98,6 @@ struct mv_xor_device {
* @device: parent device
* @common: common dmaengine channel object members
* @slots_allocated: records the actual size of the descriptor slot pool
- * @irq_tasklet: bottom half where mv_xor_slot_cleanup runs
* @op_in_desc: new mode of driver, each op is written to descriptor.
*/
struct mv_xor_chan {
@@ -118,7 +117,6 @@ struct mv_xor_chan {
struct dma_device dmadev;
struct dma_chan dmachan;
int slots_allocated;
- struct tasklet_struct irq_tasklet;
int op_in_desc;
char dummy_src[MV_XOR_MIN_BYTE_COUNT];
char dummy_dst[MV_XOR_MIN_BYTE_COUNT];
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 12/64] dmaengine: mv_xor: convert irq tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 12/64] dmaengine: mv_xor: convert irq tasklet " Allen Pais
@ 2026-07-27 20:57 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:57 UTC (permalink / raw)
To: Allen Pais; +Cc: dmaengine, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver fails to explicitly cancel pending bottom-half work during module removal and initialization error paths, leading to a Use-After-Free.
- [High] Interrupts are requested and unmasked before the channel spinlock is initialized, creating a race condition that can lead to locking an uninitialized spinlock.
- [High] Recursive deadlock when submitting dependent operations from the completion handler.
--
--- Patch [12]: [PATCH v2 12/64] dmaengine: mv_xor: convert irq tasklet to dma_chan BH ---
commit a6fdd06d107411d5248196d2bc3b3e3a0282f10d
Author: Allen Pais <allen.lkml@gmail.com>
Subject: dmaengine: mv_xor: convert irq tasklet to dma_chan BH
This patch replaces the per-channel tasklet with the shared dma_chan BH
helper. The handler continues to run in softirq context while dmaengine
owns the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
> index 25ed61f1b0899..3a70d565e1ef0 100644
> --- a/drivers/dma/mv_xor.c
> +++ b/drivers/dma/mv_xor.c
[ ... ]
> @@ -336,9 +336,9 @@ static void mv_chan_slot_cleanup(struct mv_xor_chan *mv_chan)
> mv_chan->dmachan.completed_cookie = cookie;
> }
>
> -static void mv_xor_tasklet(struct tasklet_struct *t)
> +static void mv_xor_tasklet(struct dma_chan *c)
> {
> - struct mv_xor_chan *chan = from_tasklet(chan, t, irq_tasklet);
> + struct mv_xor_chan *chan = to_mv_xor_chan(c);
>
> spin_lock(&chan->lock);
> mv_chan_slot_cleanup(chan);
> spin_unlock(&chan->lock);
> }
[Severity: High]
This is a pre-existing issue, but can this sequence lead to a recursive
deadlock? Looking at mv_xor_tasklet(), it holds the lock while running
cleanup:
mv_xor_tasklet()
spin_lock(&chan->lock);
mv_chan_slot_cleanup(chan);
If mv_chan_slot_cleanup() processes completed slots and invokes
dma_run_dependencies(), the dependent operations might be submitted to the
same channel:
mv_chan_slot_cleanup()
mv_desc_run_tx_complete_actions()
dma_run_dependencies()
mv_xor_tx_submit()
spin_lock_bh(&mv_chan->lock);
Would this recursive lock attempt cause a system hang, or is there something
preventing dependencies from being queued on the same channel?
[ ... ]
> @@ -1107,7 +1107,7 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
>
> mv_chan->mmr_base = xordev->xor_base;
> mv_chan->mmr_high_base = xordev->xor_high_base;
> - tasklet_setup(&mv_chan->irq_tasklet, mv_xor_tasklet);
> + dma_chan_init_bh(&mv_chan->dmachan, mv_xor_tasklet);
>
> /* clear errors before enabling interrupts */
> mv_chan_clear_err_status(mv_chan);
[Severity: High]
This is a pre-existing issue, but does the initialization error path properly
cancel the bottom half work before the channel structure is freed?
If a failure occurs later in mv_xor_channel_add() (like self-tests failing),
the code jumps to err_free_irq:
err_free_irq:
free_irq(mv_chan->irq, mv_chan);
err_free_dma:
...
Should dma_chan_kill_bh() be called here and in mv_xor_channel_remove() to
ensure scheduled work doesn't execute after mv_chan is freed?
[Severity: High]
This is also a pre-existing issue, but is it possible for an interrupt to
fire before the channel spinlock is initialized?
In mv_xor_channel_add(), request_irq() is called and interrupts are unmasked
before the lock is initialized:
mv_xor_channel_add()
request_irq(..., mv_xor_interrupt_handler, ...);
mv_chan_unmask_interrupts(mv_chan);
...
spin_lock_init(&mv_chan->lock);
If a spurious interrupt fires immediately and schedules the BH work, could
mv_xor_tasklet() attempt to acquire an uninitialized lock?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=12
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 13/64] dmaengine: mxs-dma: use dma_chan BH scheduling
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (11 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 12/64] dmaengine: mv_xor: convert irq tasklet " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:59 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 14/64] dmaengine: nbpfaxi: switch callbacks to dma_chan BH Allen Pais
` (50 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, imx,
linux-arm-kernel
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/mxs-dma.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 7acb3d29dad3..8cecc0add82d 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -109,7 +109,6 @@ struct mxs_dma_chan {
struct mxs_dma_engine *mxs_dma;
struct dma_chan chan;
struct dma_async_tx_descriptor desc;
- struct tasklet_struct tasklet;
unsigned int chan_irq;
struct mxs_dma_ccw *ccw;
dma_addr_t ccw_phys;
@@ -300,9 +299,9 @@ static dma_cookie_t mxs_dma_tx_submit(struct dma_async_tx_descriptor *tx)
return dma_cookie_assign(tx);
}
-static void mxs_dma_tasklet(struct tasklet_struct *t)
+static void mxs_dma_tasklet(struct dma_chan *chan)
{
- struct mxs_dma_chan *mxs_chan = from_tasklet(mxs_chan, t, tasklet);
+ struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
dmaengine_desc_get_callback_invoke(&mxs_chan->desc, NULL);
}
@@ -386,8 +385,8 @@ static irqreturn_t mxs_dma_int_handler(int irq, void *dev_id)
dma_cookie_complete(&mxs_chan->desc);
}
- /* schedule tasklet on this channel */
- tasklet_schedule(&mxs_chan->tasklet);
+ /* schedule BH on this channel */
+ dma_chan_schedule_bh(&mxs_chan->chan);
return IRQ_HANDLED;
}
@@ -781,7 +780,7 @@ static int mxs_dma_probe(struct platform_device *pdev)
mxs_chan->chan.device = &mxs_dma->dma_device;
dma_cookie_init(&mxs_chan->chan);
- tasklet_setup(&mxs_chan->tasklet, mxs_dma_tasklet);
+ dma_chan_init_bh(&mxs_chan->chan, mxs_dma_tasklet);
/* Add the channel to mxs_chan list */
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 13/64] dmaengine: mxs-dma: use dma_chan BH scheduling
2026-07-27 20:28 ` [PATCH v2 13/64] dmaengine: mxs-dma: use dma_chan BH scheduling Allen Pais
@ 2026-07-27 20:59 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:59 UTC (permalink / raw)
To: Allen Pais; +Cc: dmaengine, Frank.Li, vkoul, imx
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing synchronization/flushing of bottom-half work during channel teardown leads to a use-after-free of client resources.
- [High] Requesting IRQ before clock enable and descriptor initialization causes a kernel panic on pending interrupt.
- [High] The driver allocates the DMA engine state with devm_kzalloc but lacks a device_release callback, risking UAF on unbind.
--
--- Patch [13]: [PATCH v2 13/64] dmaengine: mxs-dma: use dma_chan BH scheduling ---
commit cc33b3c421743dc24481c221e9955791a7689087
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: mxs-dma: use dma_chan BH scheduling
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
> The handler continues to run in softirq context while dmaengine owns
> the common scheduling and teardown mechanism.
[Severity: High]
This is a pre-existing issue, but since the commit message notes that the
teardown mechanism is being updated, does mxs_dma_free_chan_resources()
need to call dma_chan_kill_bh() to prevent use-after-free conditions?
If a client releases the channel concurrently with an interrupt,
mxs_dma_free_chan_resources() currently returns without flushing pending
work:
drivers/dma/mxs-dma.c:mxs_dma_free_chan_resources() {
...
mxs_dma_disable_chan(chan);
free_irq(mxs_chan->chan_irq, mxs_dma);
}
Could this allow the asynchronous work to outlive the channel allocation
and dereference a freed client callback when mxs_dma_tasklet() runs?
> diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> index 7acb3d29dad30..8cecc0add82dc 100644
> --- a/drivers/dma/mxs-dma.c
> +++ b/drivers/dma/mxs-dma.c
[ ... ]
> @@ -385,8 +385,8 @@ static irqreturn_t mxs_dma_int_handler(int irq, void *dev_id)
> dma_cookie_complete(&mxs_chan->desc);
> }
>
> - /* schedule tasklet on this channel */
> - tasklet_schedule(&mxs_chan->tasklet);
> + /* schedule BH on this channel */
> + dma_chan_schedule_bh(&mxs_chan->chan);
>
> return IRQ_HANDLED;
> }
[Severity: High]
This is a pre-existing issue, but can this handler be triggered before the
hardware clock is enabled and the descriptor is initialized?
Looking at mxs_dma_alloc_chan_resources(), the IRQ is requested before the
clock is enabled:
drivers/dma/mxs-dma.c:mxs_dma_alloc_chan_resources() {
...
ret = request_irq(mxs_chan->chan_irq, mxs_dma_int_handler,
0, "mxs-dma", mxs_dma);
if (ret)
goto err_irq;
ret = clk_prepare_enable(mxs_dma->clk);
...
}
If a spurious or bootloader-pending interrupt fires immediately, could
mxs_dma_int_handler() process a DMA_COMPLETE status and hit a BUG_ON when
it calls dma_cookie_complete(&mxs_chan->desc) because the cookie is
still 0?
[ ... ]
> @@ -780,7 +780,7 @@ static int mxs_dma_probe(struct platform_device *pdev)
> mxs_chan->chan.device = &mxs_dma->dma_device;
> dma_cookie_init(&mxs_chan->chan);
>
> - tasklet_setup(&mxs_chan->tasklet, mxs_dma_tasklet);
> + dma_chan_init_bh(&mxs_chan->chan, mxs_dma_tasklet);
>
>
> /* Add the channel to mxs_chan list */
[Severity: High]
This is a pre-existing issue, but is the driver state vulnerable to a
use-after-free if the device is unbound?
The driver allocates mxs_dma using devm_kzalloc() but appears to register
the async device without providing a device_release callback:
drivers/dma/mxs-dma.c:mxs_dma_probe() {
...
mxs_dma = devm_kzalloc(dev, sizeof(*mxs_dma), GFP_KERNEL);
if (!mxs_dma)
return -ENOMEM;
...
ret = dmaenginem_async_device_register(&mxs_dma->dma_device);
...
}
If the driver is unbound via sysfs while a client still holds a reference
to a DMA channel, will devres immediately free mxs_dma, causing a panic
when the client later accesses the channel?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=13
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 14/64] dmaengine: nbpfaxi: switch callbacks to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (12 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 13/64] dmaengine: mxs-dma: use dma_chan BH scheduling Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:28 ` [PATCH v2 15/64] dmaengine: pch_dma: convert tasklet " Allen Pais
` (49 subsequent siblings)
63 siblings, 0 replies; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/nbpfaxi.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/nbpfaxi.c b/drivers/dma/nbpfaxi.c
index 05d7321629cc..aea94451258a 100644
--- a/drivers/dma/nbpfaxi.c
+++ b/drivers/dma/nbpfaxi.c
@@ -174,7 +174,6 @@ struct nbpf_desc_page {
/**
* struct nbpf_channel - one DMAC channel
* @dma_chan: standard dmaengine channel object
- * @tasklet: channel specific tasklet used for callbacks
* @base: register address base
* @nbpf: DMAC
* @name: IRQ name
@@ -200,7 +199,6 @@ struct nbpf_desc_page {
*/
struct nbpf_channel {
struct dma_chan dma_chan;
- struct tasklet_struct tasklet;
void __iomem *base;
struct nbpf_device *nbpf;
char name[16];
@@ -1125,9 +1123,10 @@ static struct dma_chan *nbpf_of_xlate(struct of_phandle_args *dma_spec,
return dchan;
}
-static void nbpf_chan_tasklet(struct tasklet_struct *t)
+static void nbpf_chan_tasklet(struct dma_chan *c)
{
- struct nbpf_channel *chan = from_tasklet(chan, t, tasklet);
+ struct nbpf_channel *chan = container_of(c, struct nbpf_channel,
+ dma_chan);
struct nbpf_desc *desc, *tmp;
struct dmaengine_desc_callback cb;
@@ -1228,7 +1227,7 @@ static irqreturn_t nbpf_chan_irq(int irq, void *dev)
spin_unlock(&chan->lock);
if (bh)
- tasklet_schedule(&chan->tasklet);
+ dma_chan_schedule_bh(&chan->dma_chan);
return ret;
}
@@ -1272,7 +1271,7 @@ static int nbpf_chan_probe(struct nbpf_device *nbpf, int n)
snprintf(chan->name, sizeof(chan->name), "nbpf %d", n);
- tasklet_setup(&chan->tasklet, nbpf_chan_tasklet);
+ dma_chan_init_bh(&chan->dma_chan, nbpf_chan_tasklet);
ret = devm_request_irq(dma_dev->dev, chan->irq,
nbpf_chan_irq, IRQF_SHARED,
chan->name, chan);
@@ -1478,7 +1477,7 @@ static void nbpf_remove(struct platform_device *pdev)
devm_free_irq(&pdev->dev, chan->irq, chan);
- tasklet_kill(&chan->tasklet);
+ dma_chan_kill_bh(&chan->dma_chan);
}
of_dma_controller_free(pdev->dev.of_node);
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* [PATCH v2 15/64] dmaengine: pch_dma: convert tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (13 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 14/64] dmaengine: nbpfaxi: switch callbacks to dma_chan BH Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:57 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 16/64] dmaengine: ppc4xx: replace irq tasklet with " Allen Pais
` (48 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/pch_dma.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
index bf805f1024f6..b0044bea8fe4 100644
--- a/drivers/dma/pch_dma.c
+++ b/drivers/dma/pch_dma.c
@@ -91,7 +91,6 @@ struct pch_dma_chan {
struct dma_chan chan;
void __iomem *membase;
enum dma_transfer_direction dir;
- struct tasklet_struct tasklet;
unsigned long err_status;
spinlock_t lock;
@@ -665,9 +664,10 @@ static int pd_device_terminate_all(struct dma_chan *chan)
return 0;
}
-static void pdc_tasklet(struct tasklet_struct *t)
+static void pdc_tasklet(struct dma_chan *c)
{
- struct pch_dma_chan *pd_chan = from_tasklet(pd_chan, t, tasklet);
+ struct pch_dma_chan *pd_chan = container_of(c, struct pch_dma_chan,
+ chan);
unsigned long flags;
if (!pdc_is_idle(pd_chan)) {
@@ -707,7 +707,7 @@ static irqreturn_t pd_irq(int irq, void *devid)
if (sts0 & DMA_STATUS0_ERR(i))
set_bit(0, &pd_chan->err_status);
- tasklet_schedule(&pd_chan->tasklet);
+ dma_chan_schedule_bh(&pd_chan->chan);
ret0 = IRQ_HANDLED;
}
} else {
@@ -715,7 +715,7 @@ static irqreturn_t pd_irq(int irq, void *devid)
if (sts2 & DMA_STATUS2_ERR(i))
set_bit(0, &pd_chan->err_status);
- tasklet_schedule(&pd_chan->tasklet);
+ dma_chan_schedule_bh(&pd_chan->chan);
ret2 = IRQ_HANDLED;
}
}
@@ -877,7 +877,7 @@ static int pch_dma_probe(struct pci_dev *pdev,
INIT_LIST_HEAD(&pd_chan->queue);
INIT_LIST_HEAD(&pd_chan->free_list);
- tasklet_setup(&pd_chan->tasklet, pdc_tasklet);
+ dma_chan_init_bh(&pd_chan->chan, pdc_tasklet);
list_add_tail(&pd_chan->chan.device_node, &pd->dma.channels);
}
@@ -930,7 +930,7 @@ static void pch_dma_remove(struct pci_dev *pdev)
device_node) {
pd_chan = to_pd_chan(chan);
- tasklet_kill(&pd_chan->tasklet);
+ dma_chan_kill_bh(&pd_chan->chan);
}
dma_pool_destroy(pd->pool);
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 15/64] dmaengine: pch_dma: convert tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 15/64] dmaengine: pch_dma: convert tasklet " Allen Pais
@ 2026-07-27 20:57 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:57 UTC (permalink / raw)
To: Allen Pais; +Cc: vkoul, dmaengine, Frank.Li
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing cleanup of scheduled BH work on the probe error path can lead to a use-after-free.
- [High] Early registration of a shared IRQ before channel count setup leads to an unhandled IRQ storm.
- [High] Client completion callbacks are invoked with the channel spinlock held, leading to a potential self-deadlock.
- [High] Process-context functions acquire the channel spinlock without disabling softirqs, risking a deadlock if preempted by the BH work.
- [High] Checking hardware idle state outside the lock causes a TOCTOU race leading to premature completion and use-after-free.
--
--- Patch [15]: [PATCH v2 15/64] dmaengine: pch_dma: convert tasklet to dma_chan BH ---
commit a2ed59e859fb44b7074fd4693b53c278ebc94f3d
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: pch_dma: convert tasklet to dma_chan BH
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
> index bf805f1024f64..b0044bea8fe4c 100644
> --- a/drivers/dma/pch_dma.c
> +++ b/drivers/dma/pch_dma.c
[ ... ]
> @@ -665,9 +664,10 @@ static int pd_device_terminate_all(struct dma_chan *chan)
> return 0;
> }
>
> -static void pdc_tasklet(struct tasklet_struct *t)
> +static void pdc_tasklet(struct dma_chan *c)
> {
> - struct pch_dma_chan *pd_chan = from_tasklet(pd_chan, t, tasklet);
> + struct pch_dma_chan *pd_chan = container_of(c, struct pch_dma_chan,
> + chan);
> unsigned long flags;
>
> if (!pdc_is_idle(pd_chan)) {
[Severity: High]
This isn't a bug introduced by this patch, but might checking the hardware
idle state outside the lock cause a race condition?
If a concurrent pd_issue_pending() acquires the lock first and starts the
next request, could the preempted thread later acquire the lock, see a stale
idle state, and improperly complete the newly started descriptor?
This same check appears unprotected in pd_issue_pending():
drivers/dma/pch_dma.c:pd_issue_pending() {
...
if (pdc_is_idle(pd_chan)) {
...
}
[Severity: High]
This is a pre-existing issue, but would acquiring the channel lock with a
plain spin_lock() in process-context functions risk a deadlock?
If a hardware interrupt preempts process context here in pd_tx_submit() or
pd_issue_pending():
drivers/dma/pch_dma.c:pd_tx_submit() {
...
spin_lock(&pd_chan->lock);
...
}
and schedules this pdc_tasklet() work which attempts to acquire the exact
same lock using spin_lock_irqsave(), might this deadlock the CPU?
[Severity: High]
This is a pre-existing issue, but is there a risk of a self-deadlock if the
client callback submits a new DMA transaction?
If pdc_tasklet() acquires the lock, and then pdc_chain_complete() invokes
the callback:
drivers/dma/pch_dma.c:pdc_chain_complete() {
...
if (callback)
callback(param);
...
}
and that callback attempts to submit a new transaction via pd_tx_submit(),
it will attempt to acquire the same pd_chan->lock.
[ ... ]
> @@ -877,7 +877,7 @@ static int pch_dma_probe(struct pci_dev *pdev,
> INIT_LIST_HEAD(&pd_chan->queue);
> INIT_LIST_HEAD(&pd_chan->free_list);
>
> - tasklet_setup(&pd_chan->tasklet, pdc_tasklet);
> + dma_chan_init_bh(&pd_chan->chan, pdc_tasklet);
> list_add_tail(&pd_chan->chan.device_node, &pd->dma.channels);
> }
[Severity: High]
This isn't a bug introduced by this patch, but can requesting the shared
IRQ before dma_async_device_register() initializes the channel count lead
to an unhandled IRQ storm?
In pch_dma_probe(), request_irq() is called with IRQF_SHARED before the
channel count is initialized.
The interrupt handler relies entirely on pd->dma.chancnt:
drivers/dma/pch_dma.c:pd_irq() {
...
for (i = 0; i < pd->dma.chancnt; i++) {
...
}
If pd->dma.chancnt is initially 0, could it loop 0 times and return
IRQ_NONE without clearing the hardware interrupt status?
[Severity: High]
This is a pre-existing issue, but does the probe error path fail to kill
the scheduled bottom-half work before freeing the device structure?
If dma_async_device_register() fails, the error path frees the memory but
appears to skip cancelling pending BH works:
drivers/dma/pch_dma.c:pch_dma_probe() {
...
err_free_pool:
dma_pool_destroy(pd->pool);
err_free_irq:
free_irq(pdev->irq, pd);
...
}
If an interrupt fires and schedules the BH work during this window, could
it execute on freed memory?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=15
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 16/64] dmaengine: ppc4xx: replace irq tasklet with dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (14 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 15/64] dmaengine: pch_dma: convert tasklet " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:54 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 17/64] dmaengine: ste_dma40: convert per-channel tasklet to " Allen Pais
` (47 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Colin Ian King
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/ppc4xx/adma.c | 14 +++++++-------
drivers/dma/ppc4xx/adma.h | 2 --
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
index 279a431ccae3..98cfd012ca5b 100644
--- a/drivers/dma/ppc4xx/adma.c
+++ b/drivers/dma/ppc4xx/adma.c
@@ -1658,9 +1658,9 @@ static void __ppc440spe_adma_slot_cleanup(struct ppc440spe_adma_chan *chan)
/**
* ppc440spe_adma_tasklet - clean up watch-dog initiator
*/
-static void ppc440spe_adma_tasklet(struct tasklet_struct *t)
+static void ppc440spe_adma_tasklet(struct dma_chan *c)
{
- struct ppc440spe_adma_chan *chan = from_tasklet(chan, t, irq_tasklet);
+ struct ppc440spe_adma_chan *chan = to_ppc440spe_adma_chan(c);
spin_lock_nested(&chan->lock, SINGLE_DEPTH_NESTING);
__ppc440spe_adma_slot_cleanup(chan);
@@ -1754,7 +1754,7 @@ static struct ppc440spe_adma_desc_slot *ppc440spe_adma_alloc_slots(
goto retry;
/* try to free some slots if the allocation fails */
- tasklet_schedule(&chan->irq_tasklet);
+ dma_chan_schedule_bh(&chan->common);
return NULL;
}
@@ -3595,7 +3595,7 @@ static irqreturn_t ppc440spe_adma_eot_handler(int irq, void *data)
dev_dbg(chan->device->common.dev,
"ppc440spe adma%d: %s\n", chan->device->id, __func__);
- tasklet_schedule(&chan->irq_tasklet);
+ dma_chan_schedule_bh(&chan->common);
ppc440spe_adma_device_clear_eot_status(chan);
return IRQ_HANDLED;
@@ -3612,7 +3612,7 @@ static irqreturn_t ppc440spe_adma_err_handler(int irq, void *data)
dev_dbg(chan->device->common.dev,
"ppc440spe adma%d: %s\n", chan->device->id, __func__);
- tasklet_schedule(&chan->irq_tasklet);
+ dma_chan_schedule_bh(&chan->common);
ppc440spe_adma_device_clear_eot_status(chan);
return IRQ_HANDLED;
@@ -4137,7 +4137,7 @@ static int ppc440spe_adma_probe(struct platform_device *ofdev)
chan->common.device = &adev->common;
dma_cookie_init(&chan->common);
list_add_tail(&chan->common.device_node, &adev->common.channels);
- tasklet_setup(&chan->irq_tasklet, ppc440spe_adma_tasklet);
+ dma_chan_init_bh(&chan->common, ppc440spe_adma_tasklet);
/* allocate and map helper pages for async validation or
* async_mult/async_sum_product operations on DMA0/1.
@@ -4247,7 +4247,7 @@ static void ppc440spe_adma_remove(struct platform_device *ofdev)
device_node) {
ppc440spe_chan = to_ppc440spe_adma_chan(chan);
ppc440spe_adma_release_irqs(adev, ppc440spe_chan);
- tasklet_kill(&ppc440spe_chan->irq_tasklet);
+ dma_chan_kill_bh(&ppc440spe_chan->common);
if (adev->id != PPC440SPE_XOR_ID) {
dma_unmap_page(&ofdev->dev, ppc440spe_chan->pdest,
PAGE_SIZE, DMA_BIDIRECTIONAL);
diff --git a/drivers/dma/ppc4xx/adma.h b/drivers/dma/ppc4xx/adma.h
index f8a5d7c1fb40..9af1feaf0b9a 100644
--- a/drivers/dma/ppc4xx/adma.h
+++ b/drivers/dma/ppc4xx/adma.h
@@ -80,7 +80,6 @@ struct ppc440spe_adma_device {
* @pending: allows batching of hardware operations
* @slots_allocated: records the actual size of the descriptor slot pool
* @hw_chain_inited: h/w descriptor chain initialization flag
- * @irq_tasklet: bottom half where ppc440spe_adma_slot_cleanup runs
* @needs_unmap: if buffers should not be unmapped upon final processing
* @pdest_page: P destination page for async validate operation
* @qdest_page: Q destination page for async validate operation
@@ -97,7 +96,6 @@ struct ppc440spe_adma_chan {
int pending;
int slots_allocated;
int hw_chain_inited;
- struct tasklet_struct irq_tasklet;
u8 needs_unmap;
struct page *pdest_page;
struct page *qdest_page;
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 16/64] dmaengine: ppc4xx: replace irq tasklet with dma_chan BH
2026-07-27 20:28 ` [PATCH v2 16/64] dmaengine: ppc4xx: replace irq tasklet with " Allen Pais
@ 2026-07-27 20:54 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:54 UTC (permalink / raw)
To: Allen Pais; +Cc: vkoul, dmaengine, Frank.Li
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing dma_chan_kill_bh() on probe error paths leads to use-after-free.
- [High] Lockless traversal of chan->chain in hardirq context races with list modification, causing panics.
- [High] Missing channel lock in ppc440spe_adma_issue_pending causes concurrent list/hardware corruption.
- [High] Direct freeing of DMA structures in remove() without waiting for active clients causes Use-After-Free.
--
--- Patch [16]: [PATCH v2 16/64] dmaengine: ppc4xx: replace irq tasklet with dma_chan BH ---
commit f5f6478c931c241a7fdae2271fa2dee782cc4570
Author: Allen Pais <allen.lkml@gmail.com>
Subject: dmaengine: ppc4xx: replace irq tasklet with dma_chan BH
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
> index 279a431ccae38..98cfd012ca5bf 100644
> --- a/drivers/dma/ppc4xx/adma.c
> +++ b/drivers/dma/ppc4xx/adma.c
> @@ -1658,9 +1658,9 @@ static void __ppc440spe_adma_slot_cleanup(struct ppc440spe_adma_chan *chan)
> /**
> * ppc440spe_adma_tasklet - clean up watch-dog initiator
> */
> -static void ppc440spe_adma_tasklet(struct tasklet_struct *t)
> +static void ppc440spe_adma_tasklet(struct dma_chan *c)
> {
> - struct ppc440spe_adma_chan *chan = from_tasklet(chan, t, irq_tasklet);
> + struct ppc440spe_adma_chan *chan = to_ppc440spe_adma_chan(c);
>
> spin_lock_nested(&chan->lock, SINGLE_DEPTH_NESTING);
> __ppc440spe_adma_slot_cleanup(chan);
[Severity: High]
This is a pre-existing issue, but does this list modification race with
hardirq context?
The hardirq handlers (ppc440spe_adma_eot_handler and
ppc440spe_adma_err_handler) call ppc440spe_adma_device_clear_eot_status(),
which traverses chan->chain locklessly:
drivers/dma/ppc4xx/adma.c:ppc440spe_adma_device_clear_eot_status() {
...
list_for_each_entry(iter, &chan->chain, chain_node) {
if (iter->phys == phys)
break;
}
...
}
Meanwhile, this softirq/BH code modifies the list without disabling local
interrupts (using spin_lock_nested or spin_lock_bh). If a hardware interrupt
fires on the same CPU while the list is being modified, couldn't the hardirq
handler traverse a corrupted list and cause memory corruption or a panic?
[ ... ]
> @@ -4137,7 +4137,7 @@ static int ppc440spe_adma_probe(struct platform_device *ofdev)
> chan->common.device = &adev->common;
> dma_cookie_init(&chan->common);
> list_add_tail(&chan->common.device_node, &adev->common.channels);
> - tasklet_setup(&chan->irq_tasklet, ppc440spe_adma_tasklet);
> + dma_chan_init_bh(&chan->common, ppc440spe_adma_tasklet);
>
> /* allocate and map helper pages for async validation or
[Severity: High]
This isn't a bug introduced by this patch, but does the probe error path
need to call dma_chan_kill_bh()?
In ppc440spe_adma_probe(), dma_chan_init_bh() initializes the bottom-half
work. Later, hardware IRQs are requested. If an interrupt fires and schedules
the BH via dma_chan_schedule_bh(), but a subsequent initialization step fails,
the error path uses free_irq() and then frees the channel structure without
canceling the pending work:
err_page_alloc:
kfree(chan);
When the background work queue executes later, won't it access the freed
chan memory and cause a use-after-free?
[ ... ]
> @@ -4247,7 +4247,7 @@ static void ppc440spe_adma_remove(struct platform_device *ofdev)
> device_node) {
> ppc440spe_chan = to_ppc440spe_adma_chan(chan);
> ppc440spe_adma_release_irqs(adev, ppc440spe_chan);
> - tasklet_kill(&ppc440spe_chan->irq_tasklet);
> + dma_chan_kill_bh(&ppc440spe_chan->common);
> if (adev->id != PPC440SPE_XOR_ID) {
> dma_unmap_page(&ofdev->dev, ppc440spe_chan->pdest,
> PAGE_SIZE, DMA_BIDIRECTIONAL);
[Severity: High]
This is a pre-existing issue, but does this function safely wait for all
clients to drop their references?
In ppc440spe_adma_remove(), the driver calls dma_async_device_unregister()
which unregisters the device but does not synchronously block for active
client references to be dropped.
Immediately after, it frees the channel and device structures:
list_del(&chan->device_node);
kfree(ppc440spe_chan);
...
kfree(adev);
If a client (like the network stack or filesystem) still holds a reference
and attempts to use the channel or release it later, will it dereference
these freed structures and cause a use-after-free?
[ ... ]
[Severity: High]
This is another pre-existing issue in the driver. Is channel locking missing
in ppc440spe_adma_issue_pending()?
The DMA engine callback executes without acquiring chan->lock when checking
chan->pending and calling ppc440spe_chan_append():
drivers/dma/ppc4xx/adma.c:ppc440spe_adma_issue_pending() {
...
if (ppc440spe_chan->pending) {
ppc440spe_chan->pending = 0;
ppc440spe_chan_append(ppc440spe_chan);
}
...
}
Other parts of the driver (like ppc440spe_adma_tx_submit) correctly lock
around these calls using spin_lock_bh. Because issue_pending is an
asynchronous dmaengine API that can be called concurrently across multiple
CPUs, couldn't this race and corrupt hardware MMIO registers and descriptor
lists?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=16
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 17/64] dmaengine: ste_dma40: convert per-channel tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (15 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 16/64] dmaengine: ppc4xx: replace irq tasklet with " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 21:00 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 18/64] dmaengine: xgene-dma: wire descriptor cleanup " Allen Pais
` (46 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Linus Walleij, linux-arm-kernel
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/ste_dma40.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 0d9ffa3e2663..f279a093a81f 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -461,8 +461,6 @@ struct d40_base;
* @phy_chan: Pointer to physical channel which this instance runs on. If this
* point is NULL, then the channel is not allocated.
* @chan: DMA engine handle.
- * @tasklet: Tasklet that gets scheduled from interrupt context to complete a
- * transfer and call client callback.
* @client: Cliented owned descriptor list.
* @pending_queue: Submitted jobs, to be issued by issue_pending()
* @active: Active descriptor.
@@ -489,7 +487,6 @@ struct d40_chan {
bool busy;
struct d40_phy_res *phy_chan;
struct dma_chan chan;
- struct tasklet_struct tasklet;
struct list_head client;
struct list_head pending_queue;
struct list_head active;
@@ -1587,13 +1584,13 @@ static void dma_tc_handle(struct d40_chan *d40c)
}
d40c->pending_tx++;
- tasklet_schedule(&d40c->tasklet);
+ dma_chan_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;
@@ -1641,7 +1638,7 @@ static void dma_tasklet(struct tasklet_struct *t)
d40c->pending_tx--;
if (d40c->pending_tx)
- tasklet_schedule(&d40c->tasklet);
+ dma_chan_schedule_bh(&d40c->chan);
spin_unlock_irqrestore(&d40c->lock, flags);
@@ -2815,7 +2812,7 @@ 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);
+ dma_chan_init_bh(&d40c->chan, dma_tasklet);
list_add_tail(&d40c->chan.device_node,
&dma->channels);
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 17/64] dmaengine: ste_dma40: convert per-channel tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 17/64] dmaengine: ste_dma40: convert per-channel tasklet to " Allen Pais
@ 2026-07-27 21:00 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 21:00 UTC (permalink / raw)
To: Allen Pais; +Cc: vkoul, dmaengine, Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing `free_irq()` on `d40_probe` error paths leads to a leaked IRQ handler and a potential Use-After-Free of `base`.
- [High] Missing `device_synchronize` callback causes a race condition during channel release, potentially invoking callbacks in unloaded client modules.
- [High] Probe failure after exposing channels causes a Use-After-Free because devres forcefully frees memory while clients or scheduled BH work may be active.
--
--- Patch [17]: [PATCH v2 17/64] dmaengine: ste_dma40: convert per-channel tasklet to dma_chan BH ---
commit 7d6ec649e7f14a8affbdbde4ad9aa8498383284e
Author: Allen Pais <allen.lkml@gmail.com>
Subject: dmaengine: ste_dma40: convert per-channel tasklet to dma_chan BH
This commit replaces the per-channel tasklet with the shared dma_chan BH
helper. The handler continues to run in softirq context while dmaengine
manages the scheduling and teardown mechanism.
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 0d9ffa3e26639..f279a093a81f6 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
[ ... ]
> @@ -1641,7 +1638,7 @@ static void dma_tasklet(struct tasklet_struct *t)
> d40c->pending_tx--;
>
> if (d40c->pending_tx)
> - tasklet_schedule(&d40c->tasklet);
> + dma_chan_schedule_bh(&d40c->chan);
>
> spin_unlock_irqrestore(&d40c->lock, flags);
>
[Severity: High]
This is a pre-existing issue, but does this create a race condition during
channel release?
The driver doesn't seem to implement the device_synchronize callback, making
dmaengine_synchronize() a no-op. When a DMA client module unloads and
releases its channel, it relies on this synchronization to flush any pending
async work.
Could dma_release_channel() return and the client module unload while the
bh_work is still executing here? If the work is preempted right after
dropping the channel lock and before invoking the client callback:
spin_unlock_irqrestore(&d40c->lock, flags);
if (callback_active)
dmaengine_desc_callback_invoke(&cb, NULL);
wouldn't it resume and attempt to execute the callback residing in the
now-freed module's .text segment?
Additionally, I noticed a couple of pre-existing issues in d40_probe():
[Severity: High]
This isn't a bug introduced by this patch, but does d40_probe() leak the
IRQ handler on error paths?
drivers/dma/ste_dma40.c:d40_probe() {
...
ret = request_irq(base->irq, d40_handle_interrupt, 0, D40_NAME, base);
if (ret) {
d40_err(dev, "No IRQ defined\n");
goto destroy_cache;
}
...
if (base->plat_data->use_esram_lcla) {
base->lcpa_regulator = regulator_get(base->dev, "lcla_esram");
if (IS_ERR(base->lcpa_regulator)) {
d40_err(dev, "Failed to get lcpa_regulator\n");
ret = PTR_ERR(base->lcpa_regulator);
base->lcpa_regulator = NULL;
goto destroy_cache;
}
...
destroy_cache:
if (base->lcla_pool.dma_addr)
dma_unmap_single(...)
...
report_failure:
d40_err(dev, "probe failed\n");
return ret;
}
If d40_probe() fails after request_irq() (for example, in regulator_enable()
or of_dma_controller_register()), it branches to the destroy_cache label and
returns an error without freeing the registered IRQ.
Because base is automatically freed by the devres framework, if a spurious
or lingering interrupt fires afterwards, won't the still-registered
d40_handle_interrupt() execute and dereference the freed base pointer?
[Severity: High]
This isn't a bug introduced by this patch, but does a probe failure after
exposing channels cause a Use-After-Free?
drivers/dma/ste_dma40.c:d40_probe() {
...
ret = d40_dmaengine_init(base, num_reserved_chans);
if (ret)
goto destroy_cache;
...
ret = of_dma_controller_register(np, d40_xlate, NULL);
if (ret) {
dev_err(dev,
"could not register of_dma_controller\n");
goto destroy_cache;
}
...
}
In d40_probe(), the driver exposes channels globally by calling
dmaenginem_async_device_register() midway through the function via
d40_dmaengine_init().
If a subsequent initialization step fails, like of_dma_controller_register(),
the function returns an error. The devres framework then automatically
unregisters the DMA device and frees the driver's memory.
In the window between registration and failure, could a concurrent client
driver request a channel and start a transfer, or an interrupt fire and
schedule the channel's BH work?
Since dma_async_device_unregister() doesn't wait for active clients to drop
references when called from devres, will the client or BH work access the
forcefully freed structures?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=17
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 18/64] dmaengine: xgene-dma: wire descriptor cleanup to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (16 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 17/64] dmaengine: ste_dma40: convert per-channel tasklet to " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:59 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 19/64] dmaengine: xilinx-dma: use dma_chan BH instead of tasklets Allen Pais
` (45 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/xgene-dma.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/dma/xgene-dma.c b/drivers/dma/xgene-dma.c
index fa1173e49900..8d0428c2944e 100644
--- a/drivers/dma/xgene-dma.c
+++ b/drivers/dma/xgene-dma.c
@@ -260,7 +260,6 @@ struct xgene_dma_desc_sw {
* These descriptors have already had their cleanup actions run. They
* are waiting for the ACK bit to be set by the async tx API.
* @desc_pool: descriptor pool for DMA operations
- * @tasklet: bottom half where all completed descriptors cleans
* @tx_ring: transmit ring descriptor that we use to prepare actual
* descriptors for further executions
* @rx_ring: receive ring descriptor that we use to get completed DMA
@@ -280,7 +279,6 @@ struct xgene_dma_chan {
struct list_head ld_running;
struct list_head ld_completed;
struct dma_pool *desc_pool;
- struct tasklet_struct tasklet;
struct xgene_dma_ring tx_ring;
struct xgene_dma_ring rx_ring;
};
@@ -975,9 +973,10 @@ static enum dma_status xgene_dma_tx_status(struct dma_chan *dchan,
return dma_cookie_status(dchan, cookie, txstate);
}
-static void xgene_dma_tasklet_cb(struct tasklet_struct *t)
+static void xgene_dma_tasklet_cb(struct dma_chan *c)
{
- struct xgene_dma_chan *chan = from_tasklet(chan, t, tasklet);
+ struct xgene_dma_chan *chan = container_of(c, struct xgene_dma_chan,
+ dma_chan);
/* Run all cleanup for descriptors which have been completed */
xgene_dma_cleanup_descriptors(chan);
@@ -999,11 +998,11 @@ static irqreturn_t xgene_dma_chan_ring_isr(int irq, void *id)
disable_irq_nosync(chan->rx_irq);
/*
- * Schedule the tasklet to handle all cleanup of the current
+ * Schedule the bottom half to handle all cleanup of the current
* transaction. It will start a new transaction if there is
* one pending.
*/
- tasklet_schedule(&chan->tasklet);
+ dma_chan_schedule_bh(&chan->dma_chan);
return IRQ_HANDLED;
}
@@ -1539,7 +1538,7 @@ static int xgene_dma_async_register(struct xgene_dma *pdma, int id)
INIT_LIST_HEAD(&chan->ld_pending);
INIT_LIST_HEAD(&chan->ld_running);
INIT_LIST_HEAD(&chan->ld_completed);
- tasklet_setup(&chan->tasklet, xgene_dma_tasklet_cb);
+ dma_chan_init_bh(&chan->dma_chan, xgene_dma_tasklet_cb);
chan->pending = 0;
chan->desc_pool = NULL;
@@ -1556,7 +1555,7 @@ static int xgene_dma_async_register(struct xgene_dma *pdma, int id)
ret = dma_async_device_register(dma_dev);
if (ret) {
chan_err(chan, "Failed to register async device %d", ret);
- tasklet_kill(&chan->tasklet);
+ dma_chan_kill_bh(&chan->dma_chan);
return ret;
}
@@ -1579,7 +1578,7 @@ static int xgene_dma_init_async(struct xgene_dma *pdma)
if (ret) {
for (j = 0; j < i; j++) {
dma_async_device_unregister(&pdma->dma_dev[j]);
- tasklet_kill(&pdma->chan[j].tasklet);
+ dma_chan_kill_bh(&pdma->chan[j].dma_chan);
}
return ret;
@@ -1790,7 +1789,7 @@ static void xgene_dma_remove(struct platform_device *pdev)
for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) {
chan = &pdma->chan[i];
- tasklet_kill(&chan->tasklet);
+ dma_chan_kill_bh(&chan->dma_chan);
xgene_dma_delete_chan_rings(chan);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 18/64] dmaengine: xgene-dma: wire descriptor cleanup to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 18/64] dmaengine: xgene-dma: wire descriptor cleanup " Allen Pais
@ 2026-07-27 20:59 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:59 UTC (permalink / raw)
To: Allen Pais; +Cc: Frank.Li, dmaengine, vkoul
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] During device teardown, the driver frees the IRQ before flushing the bottom half workqueue. This results in the bottom half handler calling `enable_irq()` on a freed IRQ descriptor, which violates the API and can cause warnings or oopses in the IRQ subsystem.
- [High] Potential spinlock recursion deadlock in `xgene_dma_cleanup_descriptors()` when called from process context via `xgene_dma_free_chan_resources()`.
- [High] Hardware interrupts are unmasked before the bottom halves are initialized in the probe sequence, allowing an early interrupt to permanently hang the channel.
- [High] In the probe error path, bottom halves are flushed before hardware interrupts are masked, leading to a race condition that can result in a Use-After-Free of driver structures when the workqueue later executes.
- [High] The driver returns ring slots to the hardware before clearing the descriptor completion signature, risking permanently lost completions.
--
--- Patch [18]: [PATCH v2 18/64] dmaengine: xgene-dma: wire descriptor cleanup to dma_chan BH ---
commit 3558adb2da1479ded074f1f3135c3903cdea9f57
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: xgene-dma: wire descriptor cleanup to dma_chan BH
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/xgene-dma.c b/drivers/dma/xgene-dma.c
> index fa1173e499009..8d0428c2944ef 100644
> --- a/drivers/dma/xgene-dma.c
> +++ b/drivers/dma/xgene-dma.c
> @@ -975,9 +973,10 @@ static enum dma_status xgene_dma_tx_status(struct dma_chan *dchan,
> return dma_cookie_status(dchan, cookie, txstate);
> }
>
> -static void xgene_dma_tasklet_cb(struct tasklet_struct *t)
> +static void xgene_dma_tasklet_cb(struct dma_chan *c)
> {
> - struct xgene_dma_chan *chan = from_tasklet(chan, t, tasklet);
> + struct xgene_dma_chan *chan = container_of(c, struct xgene_dma_chan,
> + dma_chan);
>
> /* Run all cleanup for descriptors which have been completed */
> xgene_dma_cleanup_descriptors(chan);
[Severity: High]
This is a pre-existing issue, but can this cause a spinlock recursion
deadlock?
Looking at xgene_dma_free_chan_resources(), it runs in process context and
calls xgene_dma_cleanup_descriptors(), which acquires chan->lock using
spin_lock() without disabling softirqs.
If a hardware interrupt occurs on the same CPU, it schedules the bottom half.
On interrupt exit, the softirq executes xgene_dma_tasklet_cb(), which also
calls xgene_dma_cleanup_descriptors() and attempts to acquire the
already-held chan->lock.
[Severity: High]
This is a pre-existing issue, but does this out-of-order execution risk
permanently losing DMA completions?
In xgene_dma_cleanup_descriptors(), the driver writes to ring->cmd to notify
the hardware that a descriptor slot is free, and then clears the descriptor's
completion signature in memory:
xgene_dma_cleanup_descriptors() {
...
iowrite32(-1, ring->cmd);
desc_hw->m0 = cpu_to_le64(XGENE_DMA_DESC_EMPTY_SIGNATURE);
...
}
Since the hardware engine is active, could it immediately write a new
completion signature into that slot before the CPU executes the assignment,
causing the CPU to unconditionally overwrite the valid completion signature
with the empty signature?
[ ... ]
> @@ -1539,7 +1538,7 @@ static int xgene_dma_async_register(struct xgene_dma *pdma, int id)
> INIT_LIST_HEAD(&chan->ld_pending);
> INIT_LIST_HEAD(&chan->ld_running);
> INIT_LIST_HEAD(&chan->ld_completed);
> - tasklet_setup(&chan->tasklet, xgene_dma_tasklet_cb);
> + dma_chan_init_bh(&chan->dma_chan, xgene_dma_tasklet_cb);
[Severity: High]
This is a pre-existing issue, but can an early interrupt permanently hang
the channel during probe?
In xgene_dma_probe(), xgene_dma_init_hw() enables the DMA engine and unmasks
interrupts before xgene_dma_init_async() initializes the bottom halves.
If a pending or spurious interrupt fires, the ISR attempts to schedule the
uninitialized BH, which fails. Because the BH never runs, enable_irq() is
never called, leaving the channel's IRQ permanently disabled.
[ ... ]
> @@ -1579,7 +1578,7 @@ static int xgene_dma_init_async(struct xgene_dma *pdma)
> if (ret) {
> for (j = 0; j < i; j++) {
> dma_async_device_unregister(&pdma->dma_dev[j]);
> - tasklet_kill(&pdma->chan[j].tasklet);
> + dma_chan_kill_bh(&pdma->chan[j].dma_chan);
> }
[Severity: High]
This is a pre-existing issue, but could this error path lead to a
use-after-free of the driver structures?
If xgene_dma_init_async() fails, it calls dma_chan_kill_bh() to clean up
initialized channels, but does not mask hardware interrupts.
An interrupt could fire immediately after, re-queuing the BH. The probe
function then calls xgene_dma_free_irqs(), returns an error, and devm
frees the pdma memory.
Could the global workqueue subsequently execute the re-scheduled BH,
dereferencing the freed chan structure?
[ ... ]
> @@ -1790,7 +1789,7 @@ static void xgene_dma_remove(struct platform_device *pdev)
>
> for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) {
> chan = &pdma->chan[i];
> - tasklet_kill(&chan->tasklet);
> + dma_chan_kill_bh(&chan->dma_chan);
> xgene_dma_delete_chan_rings(chan);
> }
[Severity: High]
This is a pre-existing issue, but is there a risk of accessing a freed IRQ
descriptor during this teardown sequence?
xgene_dma_remove() calls xgene_dma_free_irqs() which frees chan->rx_irq, and
then calls dma_chan_kill_bh().
If the bottom half was already queued, it executes after the IRQ is freed
and unconditionally calls enable_irq(chan->rx_irq), which violates the API
and could cause an oops in the IRQ subsystem.
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=18
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 19/64] dmaengine: xilinx-dma: use dma_chan BH instead of tasklets
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (17 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 18/64] dmaengine: xgene-dma: wire descriptor cleanup " Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:59 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 20/64] dmaengine: xilinx-dpdma: kill vchan BH on remove Allen Pais
` (44 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Michal Simek, Suraj Gupta, Marek Vasut, Folker Schwesinger,
Tomi Valkeinen, Krzysztof Kozlowski, linux-arm-kernel
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/xilinx/xilinx_dma.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 404235c17353..bf9f22ed1265 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -402,7 +402,6 @@ struct xilinx_dma_tx_descriptor {
* @err: Channel has errors
* @idle: Check for channel idle
* @terminating: Check for channel being synchronized by user
- * @tasklet: Cleanup work after irq
* @config: Device configuration info
* @flush_on_fsync: Flush on Frame sync
* @desc_pendingcount: Descriptor pending count
@@ -441,7 +440,6 @@ struct xilinx_dma_chan {
bool err;
bool idle;
bool terminating;
- struct tasklet_struct tasklet;
struct xilinx_vdma_config config;
bool flush_on_fsync;
u32 desc_pendingcount;
@@ -1115,11 +1113,12 @@ static void xilinx_dma_chan_desc_cleanup(struct xilinx_dma_chan *chan)
/**
* xilinx_dma_do_tasklet - Schedule completion tasklet
- * @t: Pointer to the Xilinx DMA channel structure
+ * @c: Pointer to the Xilinx DMA channel structure
*/
-static void xilinx_dma_do_tasklet(struct tasklet_struct *t)
+static void xilinx_dma_do_tasklet(struct dma_chan *c)
{
- struct xilinx_dma_chan *chan = from_tasklet(chan, t, tasklet);
+ struct xilinx_dma_chan *chan = container_of(c,
+ struct xilinx_dma_chan, common);
xilinx_dma_chan_desc_cleanup(chan);
}
@@ -1898,7 +1897,7 @@ static irqreturn_t xilinx_mcdma_irq_handler(int irq, void *data)
spin_unlock(&chan->lock);
}
- tasklet_hi_schedule(&chan->tasklet);
+ dma_chan_schedule_bh(&chan->common);
return IRQ_HANDLED;
}
@@ -1955,7 +1954,7 @@ static irqreturn_t xilinx_dma_irq_handler(int irq, void *data)
spin_unlock(&chan->lock);
}
- tasklet_schedule(&chan->tasklet);
+ dma_chan_schedule_bh(&chan->common);
return IRQ_HANDLED;
}
@@ -2654,7 +2653,7 @@ static void xilinx_dma_synchronize(struct dma_chan *dchan)
{
struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
- tasklet_kill(&chan->tasklet);
+ dma_chan_kill_bh(&chan->common);
}
/**
@@ -2745,7 +2744,7 @@ static void xilinx_dma_chan_remove(struct xilinx_dma_chan *chan)
if (chan->irq > 0)
free_irq(chan->irq, chan);
- tasklet_kill(&chan->tasklet);
+ dma_chan_kill_bh(&chan->common);
list_del(&chan->common.device_node);
}
@@ -3075,8 +3074,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
str_enabled_disabled(chan->has_sg));
}
- /* Initialize the tasklet */
- tasklet_setup(&chan->tasklet, xilinx_dma_do_tasklet);
+ dma_chan_init_bh(&chan->common, xilinx_dma_do_tasklet);
/*
* Initialize the DMA channel and add it to the DMA engine channels
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 19/64] dmaengine: xilinx-dma: use dma_chan BH instead of tasklets
2026-07-27 20:28 ` [PATCH v2 19/64] dmaengine: xilinx-dma: use dma_chan BH instead of tasklets Allen Pais
@ 2026-07-27 20:59 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:59 UTC (permalink / raw)
To: Allen Pais; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] request_irq() is called before channel initialization is complete, creating a race condition if an early interrupt fires.
- [High] Uninitialized channel access in xilinx_mcdma_irq_handler during probe.
- [High] Recursive spinlock deadlock when running dependencies on the same DMA channel.
--
--- Patch [19]: [PATCH v2 19/64] dmaengine: xilinx-dma: use dma_chan BH instead of tasklets ---
commit 2b1e6b23fed3e56575f2e8169196ee83cbca26e8
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: xilinx-dma: use dma_chan BH instead of tasklets
This patch replaces the per-channel tasklet with the shared dma_chan BH
helper. The handler continues to run in softirq context while dmaengine
owns the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 404235c173538..bf9f22ed12659 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
[ ... ]
> @@ -1115,11 +1113,12 @@ static void xilinx_dma_chan_desc_cleanup(struct xilinx_dma_chan *chan)
>
> /**
> * xilinx_dma_do_tasklet - Schedule completion tasklet
> - * @t: Pointer to the Xilinx DMA channel structure
> + * @c: Pointer to the Xilinx DMA channel structure
> */
> -static void xilinx_dma_do_tasklet(struct tasklet_struct *t)
> +static void xilinx_dma_do_tasklet(struct dma_chan *c)
> {
> - struct xilinx_dma_chan *chan = from_tasklet(chan, t, tasklet);
> + struct xilinx_dma_chan *chan = container_of(c,
> + struct xilinx_dma_chan, common);
>
> xilinx_dma_chan_desc_cleanup(chan);
> }
[Severity: High]
This is a pre-existing issue, but can this lead to a recursive spinlock
deadlock?
Looking at xilinx_dma_chan_desc_cleanup(), the channel lock is acquired
before running dependencies:
xilinx_dma_chan_desc_cleanup() {
...
spin_lock_irqsave(&chan->lock, flags);
...
dma_run_dependencies(&desc->async_tx);
...
}
If a dependent transaction belongs to the same channel, dma_run_dependencies()
will invoke xilinx_dma_tx_submit(), which unconditionally attempts to acquire
the exact same chan->lock. Since standard spinlocks are not recursive, would
this cause the CPU to deadlock?
[ ... ]
> @@ -1898,7 +1897,7 @@ static irqreturn_t xilinx_mcdma_irq_handler(int irq, void *data)
> spin_unlock(&chan->lock);
> }
>
> - tasklet_hi_schedule(&chan->tasklet);
> + dma_chan_schedule_bh(&chan->common);
> return IRQ_HANDLED;
> }
[Severity: High]
This is a pre-existing issue, but could this handler dereference a NULL
pointer during device probe?
If a shared interrupt fires while the driver is still probing channels,
xilinx_mcdma_irq_handler() might compute a chan_offset for a channel that
has not been probed yet:
xilinx_mcdma_irq_handler() {
...
chan = chan->xdev->chan[chan_offset];
status = dma_ctrl_read(chan, XILINX_MCDMA_CHAN_SR_OFFSET(chan->tdest));
...
}
If the channel array entry is still NULL, would dma_ctrl_read() trigger a
crash?
[ ... ]
> @@ -3075,8 +3074,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
> str_enabled_disabled(chan->has_sg));
> }
>
> - /* Initialize the tasklet */
> - tasklet_setup(&chan->tasklet, xilinx_dma_do_tasklet);
> + dma_chan_init_bh(&chan->common, xilinx_dma_do_tasklet);
[Severity: High]
This is a pre-existing issue, but does calling request_irq() earlier in
xilinx_dma_chan_probe() create a race condition here?
In xilinx_dma_chan_probe(), request_irq() is called with IRQF_SHARED
before chan->start_transfer and the bottom-half work are initialized:
xilinx_dma_chan_probe() {
...
err = request_irq(chan->irq, xdev->dma_config->irq_handler,
IRQF_SHARED, "xilinx-dma-controller", chan);
...
chan->start_transfer = xilinx_dma_start_transfer;
...
dma_chan_init_bh(&chan->common, xilinx_dma_do_tasklet);
...
}
If an interrupt fires on the shared line immediately after request_irq()
succeeds, the handler could try to invoke chan->start_transfer() or
dma_chan_schedule_bh() while they are still uninitialized. Should the IRQ
registration be moved after the channel is fully initialized?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=19
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 20/64] dmaengine: xilinx-dpdma: kill vchan BH on remove
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (18 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 19/64] dmaengine: xilinx-dma: use dma_chan BH instead of tasklets Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:59 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 21/64] dmaengine: zynqmp-dma: switch completion tasklet to dma_chan BH Allen Pais
` (43 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Laurent Pinchart, Michal Simek, linux-arm-kernel
virt-dma now dispatches completion callbacks through per-channel BH
work. Cancel that work when removing a channel, while retaining the
driver's separate error-handling tasklet.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/xilinx/xilinx_dpdma.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
index d9a3542c4531..2ec82064fd2e 100644
--- a/drivers/dma/xilinx/xilinx_dpdma.c
+++ b/drivers/dma/xilinx/xilinx_dpdma.c
@@ -1685,6 +1685,7 @@ static void xilinx_dpdma_chan_remove(struct xilinx_dpdma_chan *chan)
return;
tasklet_kill(&chan->err_task);
+ dma_chan_kill_bh(&chan->vchan.chan);
list_del(&chan->vchan.chan.device_node);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 20/64] dmaengine: xilinx-dpdma: kill vchan BH on remove
2026-07-27 20:28 ` [PATCH v2 20/64] dmaengine: xilinx-dpdma: kill vchan BH on remove Allen Pais
@ 2026-07-27 20:59 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:59 UTC (permalink / raw)
To: Allen Pais; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Premature clock disable and delayed IRQ free expose the driver to synchronous external aborts during teardown and probe error paths.
- [High] Hardware registers are accessed during probe before the AXI clock is prepared and enabled.
- [High] Concurrent modification of virt-dma `desc_issued` list due to missing `chan->vchan.lock`.
--
--- Patch [20]: [PATCH v2 20/64] dmaengine: xilinx-dpdma: kill vchan BH on remove ---
commit b6ee3342e2955cfec74b9591a7918040bbe546f8
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: xilinx-dpdma: kill vchan BH on remove
virt-dma now dispatches completion callbacks through per-channel BH
work. Cancel that work when removing a channel, while retaining the
driver's separate error-handling tasklet.
> diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
> index d9a3542c4531c..2ec82064fd2ed 100644
> --- a/drivers/dma/xilinx/xilinx_dpdma.c
> +++ b/drivers/dma/xilinx/xilinx_dpdma.c
> @@ -1685,6 +1685,7 @@ static void xilinx_dpdma_chan_remove(struct xilinx_dpdma_chan *chan)
> return;
>
> tasklet_kill(&chan->err_task);
> + dma_chan_kill_bh(&chan->vchan.chan);
> list_del(&chan->vchan.chan.device_node);
> }
[Severity: High]
This is a pre-existing issue, but does the driver disable the clock too
early during teardown and probe error paths?
Looking at xilinx_dpdma_remove(), the AXI clock is disabled before calling
xilinx_dpdma_chan_remove() to kill the tasklets and BHs:
drivers/dma/xilinx/xilinx_dpdma.c:xilinx_dpdma_remove() {
...
clk_disable_unprepare(xdev->axi_clk);
for (i = 0; i < ARRAY_SIZE(xdev->chan); i++)
xilinx_dpdma_chan_remove(xdev->chan[i]);
...
}
Similarly, in the xilinx_dpdma_probe() error path, the clock is disabled
before tasklets are killed and before the shared IRQ is freed:
drivers/dma/xilinx/xilinx_dpdma.c:xilinx_dpdma_probe() {
...
error_dma_async:
clk_disable_unprepare(xdev->axi_clk);
error:
for (i = 0; i < ARRAY_SIZE(xdev->chan); i++)
xilinx_dpdma_chan_remove(xdev->chan[i]);
free_irq(xdev->irq, xdev);
...
}
Could this cause synchronous external aborts if an active shared IRQ handler,
tasklet, or BH attempts to access hardware registers while the clock is
disabled?
[Severity: High]
This is a pre-existing issue, but are hardware registers accessed during
probe before the AXI clock is prepared and enabled?
In xilinx_dpdma_probe(), dpdma_hw_init() is called early to disable
interrupts and clear statuses, which accesses hardware registers:
drivers/dma/xilinx/xilinx_dpdma.c:xilinx_dpdma_probe() {
...
dpdma_hw_init(xdev);
xdev->irq = platform_get_irq(pdev, 0);
...
}
However, the clock is enabled much later in the probe sequence:
drivers/dma/xilinx/xilinx_dpdma.c:xilinx_dpdma_probe() {
...
ret = clk_prepare_enable(xdev->axi_clk);
if (ret) {
...
}
Will this cause a synchronous external abort on device probe if the
bootloader hasn't already enabled the clock?
[Severity: High]
This is a pre-existing issue, but does xilinx_dpdma_chan_handle_err()
modify the virt-dma desc_issued list without the correct lock?
It appears to modify chan->vchan.desc_issued without holding
chan->vchan.lock:
drivers/dma/xilinx/xilinx_dpdma.c:xilinx_dpdma_chan_handle_err() {
...
if (!chan->desc.pending &&
list_empty(&chan->vchan.desc_issued)) {
active->error = true;
list_add_tail(&active->vdesc.node,
&chan->vchan.desc_issued);
...
}
The virt-dma layer expects chan->vchan.lock to protect the desc_issued list,
as seen in xilinx_dpdma_terminate_all() where chan->vchan.lock is taken:
drivers/dma/xilinx/xilinx_dpdma.c:xilinx_dpdma_terminate_all() {
...
spin_lock_irqsave(&chan->vchan.lock, flags);
vchan_get_all_descriptors(&chan->vchan, &descriptors);
spin_unlock_irqrestore(&chan->vchan.lock, flags);
...
}
Could this concurrent modification lead to linked list corruption if a
hardware channel error occurs while a client calls
dmaengine_terminate_async()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=20
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 21/64] dmaengine: zynqmp-dma: switch completion tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (19 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 20/64] dmaengine: xilinx-dpdma: kill vchan BH on remove Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:54 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 22/64] dmaengine: tegra20-apb: use channel BH helpers Allen Pais
` (42 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Michal Simek, Abin Joseph, Radhey Shyam Pandey, Sakari Ailus,
linux-arm-kernel
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/xilinx/zynqmp_dma.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index f6a812e49ddc..6291126cdc9f 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -207,7 +207,6 @@ struct zynqmp_dma_desc_sw {
* @dev: The dma device
* @irq: Channel IRQ
* @is_dmacoherent: Tells whether dma operations are coherent or not
- * @tasklet: Cleanup work after irq
* @idle : Channel status;
* @desc_size: Size of the low level descriptor
* @err: Channel has errors
@@ -232,7 +231,6 @@ struct zynqmp_dma_chan {
struct device *dev;
int irq;
bool is_dmacoherent;
- struct tasklet_struct tasklet;
bool idle;
size_t desc_size;
bool err;
@@ -735,7 +733,7 @@ static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
writel(isr, chan->regs + ZYNQMP_DMA_ISR);
if (status & ZYNQMP_DMA_INT_DONE) {
- tasklet_schedule(&chan->tasklet);
+ dma_chan_schedule_bh(&chan->common);
ret = IRQ_HANDLED;
}
@@ -744,7 +742,7 @@ static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
if (status & ZYNQMP_DMA_INT_ERR) {
chan->err = true;
- tasklet_schedule(&chan->tasklet);
+ dma_chan_schedule_bh(&chan->common);
dev_err(chan->dev, "Channel %p has errors\n", chan);
ret = IRQ_HANDLED;
}
@@ -760,11 +758,12 @@ static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
/**
* zynqmp_dma_do_tasklet - Schedule completion tasklet
- * @t: Pointer to the ZynqMP DMA channel structure
+ * @c: Pointer to the ZynqMP DMA channel structure
*/
-static void zynqmp_dma_do_tasklet(struct tasklet_struct *t)
+static void zynqmp_dma_do_tasklet(struct dma_chan *c)
{
- struct zynqmp_dma_chan *chan = from_tasklet(chan, t, tasklet);
+ struct zynqmp_dma_chan *chan = container_of(c,
+ struct zynqmp_dma_chan, common);
u32 count;
unsigned long irqflags;
@@ -815,7 +814,7 @@ static void zynqmp_dma_synchronize(struct dma_chan *dchan)
{
struct zynqmp_dma_chan *chan = to_chan(dchan);
- tasklet_kill(&chan->tasklet);
+ dma_chan_kill_bh(&chan->common);
}
/**
@@ -887,7 +886,7 @@ static void zynqmp_dma_chan_remove(struct zynqmp_dma_chan *chan)
if (chan->irq)
devm_free_irq(chan->zdev->dev, chan->irq, chan);
- tasklet_kill(&chan->tasklet);
+ dma_chan_kill_bh(&chan->common);
list_del(&chan->common.device_node);
}
@@ -937,7 +936,7 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
chan->is_dmacoherent = of_property_read_bool(node, "dma-coherent");
zdev->chan = chan;
- tasklet_setup(&chan->tasklet, zynqmp_dma_do_tasklet);
+ dma_chan_init_bh(&chan->common, zynqmp_dma_do_tasklet);
spin_lock_init(&chan->lock);
INIT_LIST_HEAD(&chan->active_list);
INIT_LIST_HEAD(&chan->pending_list);
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 21/64] dmaengine: zynqmp-dma: switch completion tasklet to dma_chan BH
2026-07-27 20:28 ` [PATCH v2 21/64] dmaengine: zynqmp-dma: switch completion tasklet to dma_chan BH Allen Pais
@ 2026-07-27 20:54 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 20:54 UTC (permalink / raw)
To: Allen Pais; +Cc: Frank.Li, dmaengine, vkoul
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] On channel error, the driver incorrectly marks the currently active descriptor as successful and silently drops all other pending descriptors, causing clients to hang indefinitely.
- [High] The DMA completion handler `zynqmp_dma_chan_desc_cleanup` drops `chan->lock` during callback execution while using `list_for_each_entry_safe`, allowing list corruption if a callback calls `dmaengine_terminate_async`.
- [High] `zynqmp_dma_device_terminate_all` fails to actually halt the hardware DMA engine, leading to hardware silently overwriting memory after the client assumes the transfer is stopped.
--
--- Patch [21]: [PATCH v2 21/64] dmaengine: zynqmp-dma: switch completion tasklet to dma_chan BH ---
commit c7b5b53ae6dbca9c51452b38046b4252492aa34c
Author: Allen Pais <allen.lkml@gmail.com>
Subject: dmaengine: zynqmp-dma: switch completion tasklet to dma_chan BH
Replace the per-channel tasklet with the shared dma_chan BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.
> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index f6a812e49ddca..6291126cdc9f8 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
[ ... ]
> @@ -760,11 +758,12 @@ static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
>
> /**
> * zynqmp_dma_do_tasklet - Schedule completion tasklet
> - * @t: Pointer to the ZynqMP DMA channel structure
> + * @c: Pointer to the ZynqMP DMA channel structure
> */
> -static void zynqmp_dma_do_tasklet(struct tasklet_struct *t)
> +static void zynqmp_dma_do_tasklet(struct dma_chan *c)
> {
> - struct zynqmp_dma_chan *chan = from_tasklet(chan, t, tasklet);
> + struct zynqmp_dma_chan *chan = container_of(c,
> + struct zynqmp_dma_chan, common);
> u32 count;
> unsigned long irqflags;
>
[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential problem in
how channel errors are handled when this bottom half runs.
When chan->err is set, we call zynqmp_dma_reset(). Inside zynqmp_dma_reset():
drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_reset() {
...
zynqmp_dma_complete_descriptor(chan);
...
zynqmp_dma_free_descriptors(chan);
...
}
Does this incorrectly move the active descriptor to done_list and mark it
successful?
Also, it appears zynqmp_dma_free_descriptors() silently moves all remaining
descriptors from active_list and pending_list to the free_list without
invoking their callbacks.
Could this cause clients waiting on these DMA transfers to hang indefinitely?
[Severity: High]
This is a pre-existing issue, but could there be a list corruption risk in
zynqmp_dma_chan_desc_cleanup() which is called shortly after in this bottom
half?
drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_chan_desc_cleanup() {
...
list_for_each_entry_safe(desc, next, &chan->done_list, node) {
...
if (dmaengine_desc_callback_valid(&cb)) {
spin_unlock_irqrestore(&chan->lock, irqflags);
dmaengine_desc_callback_invoke(&cb, NULL);
spin_lock_irqsave(&chan->lock, irqflags);
}
...
}
The lock is dropped during callback execution. If the client calls
dmaengine_terminate_async() in the callback, all descriptors (including those
on done_list) are moved to free_list.
When the lock is reacquired, the loop continues using the next pointer which
is now on the free_list.
Could this lead to double-frees and traversing invalid entries?
[Severity: High]
This is a pre-existing issue, but does zynqmp_dma_device_terminate_all()
actually halt the hardware DMA engine?
drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_device_terminate_all() {
...
writel(ZYNQMP_DMA_IDS_DEFAULT_MASK, chan->regs + ZYNQMP_DMA_IDS);
zynqmp_dma_free_descriptors(chan);
...
}
It masks interrupts and frees software descriptors, but doesn't seem to clear
the ZYNQMP_DMA_ENABLE bit or issue a hardware reset.
If the hardware controller remains active, could it silently overwrite memory
after the client assumes the transfer is stopped and unmaps the DMA buffers?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=21
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 22/64] dmaengine: tegra20-apb: use channel BH helpers
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (20 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 21/64] dmaengine: zynqmp-dma: switch completion tasklet to dma_chan BH Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 20:28 ` [PATCH v2 23/64] dmaengine: timb_dma: route callbacks via channel BH Allen Pais
` (41 subsequent siblings)
63 siblings, 0 replies; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook,
Laxman Dewangan, Jon Hunter, Thierry Reding, linux-tegra
Switch the per-channel tasklet to dma_chan_*_bh so completion handling
runs through the shared per-channel deferral mechanism.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/tegra20-apb-dma.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 640b8a218c9a..de15b8d3e8cb 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -194,9 +194,8 @@ struct tegra_dma_channel {
struct list_head free_dma_desc;
struct list_head cb_desc;
- /* ISR handler and tasklet for bottom half of isr handling */
+ /* ISR handler for bottom-half processing */
dma_isr_handler isr_handler;
- struct tasklet_struct tasklet;
/* Channel-slave specific configuration */
unsigned int slave_id;
@@ -632,9 +631,10 @@ static void handle_cont_sngl_cycle_dma_done(struct tegra_dma_channel *tdc,
}
}
-static void tegra_dma_tasklet(struct tasklet_struct *t)
+static void tegra_dma_tasklet(struct dma_chan *chan)
{
- struct tegra_dma_channel *tdc = from_tasklet(tdc, t, tasklet);
+ struct tegra_dma_channel *tdc = container_of(chan, struct tegra_dma_channel,
+ dma_chan);
struct dmaengine_desc_callback cb;
struct tegra_dma_desc *dma_desc;
unsigned int cb_count;
@@ -670,7 +670,7 @@ static irqreturn_t tegra_dma_isr(int irq, void *dev_id)
if (status & TEGRA_APBDMA_STATUS_ISE_EOC) {
tdc_write(tdc, TEGRA_APBDMA_CHAN_STATUS, status);
tdc->isr_handler(tdc, false);
- tasklet_schedule(&tdc->tasklet);
+ dma_chan_schedule_bh(&tdc->dma_chan);
wake_up_all(&tdc->wq);
spin_unlock(&tdc->lock);
return IRQ_HANDLED;
@@ -819,7 +819,7 @@ static void tegra_dma_synchronize(struct dma_chan *dc)
*/
wait_event(tdc->wq, tegra_dma_eoc_interrupt_deasserted(tdc));
- tasklet_kill(&tdc->tasklet);
+ dma_chan_kill_bh(&tdc->dma_chan);
pm_runtime_put(tdc->tdma->dev);
}
@@ -1317,7 +1317,7 @@ static void tegra_dma_free_chan_resources(struct dma_chan *dc)
dev_dbg(tdc2dev(tdc), "Freeing channel %d\n", tdc->id);
tegra_dma_terminate_all(dc);
- tasklet_kill(&tdc->tasklet);
+ dma_chan_kill_bh(&tdc->dma_chan);
list_splice_init(&tdc->pending_sg_req, &sg_req_list);
list_splice_init(&tdc->free_sg_req, &sg_req_list);
@@ -1511,7 +1511,7 @@ static int tegra_dma_probe(struct platform_device *pdev)
tdc->id = i;
tdc->slave_id = TEGRA_APBDMA_SLAVE_ID_INVALID;
- tasklet_setup(&tdc->tasklet, tegra_dma_tasklet);
+ dma_chan_init_bh(&tdc->dma_chan, tegra_dma_tasklet);
spin_lock_init(&tdc->lock);
init_waitqueue_head(&tdc->wq);
@@ -1617,7 +1617,7 @@ static int __maybe_unused tegra_dma_dev_suspend(struct device *dev)
for (i = 0; i < tdma->chip_data->nr_channels; i++) {
struct tegra_dma_channel *tdc = &tdma->channels[i];
- tasklet_kill(&tdc->tasklet);
+ dma_chan_kill_bh(&tdc->dma_chan);
spin_lock_irqsave(&tdc->lock, flags);
busy = tdc->busy;
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* [PATCH v2 23/64] dmaengine: timb_dma: route callbacks via channel BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (21 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 22/64] dmaengine: tegra20-apb: use channel BH helpers Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 21:09 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 24/64] dmaengine: txx9dmac: " Allen Pais
` (40 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
Schedule per-channel BH work for completions so callbacks run through
the dma_chan deferral mechanism while keeping the controller tasklet for
IRQ handling.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/timb_dma.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/timb_dma.c b/drivers/dma/timb_dma.c
index 4fc5119854ec..7d43e5712ffe 100644
--- a/drivers/dma/timb_dma.c
+++ b/drivers/dma/timb_dma.c
@@ -438,6 +438,8 @@ static void td_free_chan_resources(struct dma_chan *chan)
dev_dbg(chan2dev(chan), "%s: Entry\n", __func__);
+ dma_chan_kill_bh(&td_chan->chan);
+
/* check that all descriptors are free */
BUG_ON(!list_empty(&td_chan->active_list));
BUG_ON(!list_empty(&td_chan->queue));
@@ -580,17 +582,25 @@ static void td_tasklet(struct tasklet_struct *t)
for (i = 0; i < td->dma.chancnt; i++)
if (ipr & (1 << i)) {
struct timb_dma_chan *td_chan = td->channels + i;
- spin_lock(&td_chan->lock);
- __td_finish(td_chan);
- if (!list_empty(&td_chan->queue))
- __td_start_next(td_chan);
- spin_unlock(&td_chan->lock);
+ dma_chan_schedule_bh(&td_chan->chan);
}
ier = __td_ier_mask(td);
iowrite32(ier, td->membase + TIMBDMA_IER);
}
+static void timb_dma_chan_bh(struct dma_chan *chan)
+{
+ struct timb_dma_chan *td_chan = container_of(chan, struct timb_dma_chan,
+ chan);
+
+ spin_lock(&td_chan->lock);
+ __td_finish(td_chan);
+ if (!list_empty(&td_chan->queue))
+ __td_start_next(td_chan);
+ spin_unlock(&td_chan->lock);
+}
+
static irqreturn_t td_irq(int irq, void *devid)
{
@@ -697,6 +707,7 @@ static int td_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&td_chan->active_list);
INIT_LIST_HEAD(&td_chan->queue);
INIT_LIST_HEAD(&td_chan->free_list);
+ dma_chan_init_bh(&td_chan->chan, timb_dma_chan_bh);
td_chan->descs = pchan->descriptors;
td_chan->desc_elems = pchan->descriptor_elements;
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 23/64] dmaengine: timb_dma: route callbacks via channel BH
2026-07-27 20:28 ` [PATCH v2 23/64] dmaengine: timb_dma: route callbacks via channel BH Allen Pais
@ 2026-07-27 21:09 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 21:09 UTC (permalink / raw)
To: Allen Pais; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Data race and out-of-bounds read in __td_ier_mask due to lockless traversal of active_list racing with asynchronous workqueue.
Pre-existing issues:
- [High] Premature completion of active transfers due to a race between td_issue_pending and timb_dma_chan_bh.
--
--- Patch [23]: [PATCH v2 23/64] dmaengine: timb_dma: route callbacks via channel BH ---
commit 2a07cb9b339578f9caa75d42037da515bda38de2
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: timb_dma: route callbacks via channel BH
This patch schedules per-channel BH work for completions so callbacks run
through the dma_chan deferral mechanism while keeping the controller
tasklet for IRQ handling.
> diff --git a/drivers/dma/timb_dma.c b/drivers/dma/timb_dma.c
> --- a/drivers/dma/timb_dma.c
> +++ b/drivers/dma/timb_dma.c
[ ... ]
> @@ -580,17 +582,25 @@ static void td_tasklet(struct tasklet_struct *t)
> for (i = 0; i < td->dma.chancnt; i++)
> if (ipr & (1 << i)) {
> struct timb_dma_chan *td_chan = td->channels + i;
> - spin_lock(&td_chan->lock);
> - __td_finish(td_chan);
> - if (!list_empty(&td_chan->queue))
> - __td_start_next(td_chan);
> - spin_unlock(&td_chan->lock);
> + dma_chan_schedule_bh(&td_chan->chan);
> }
>
> ier = __td_ier_mask(td);
[Severity: High]
Can this lead to an out-of-bounds memory read?
With completion processing moved to an asynchronous workqueue via
dma_chan_schedule_bh(), td_tasklet() and timb_dma_chan_bh() can now run
concurrently. The tasklet calls __td_ier_mask() right here, which
locklessly evaluates td_chan->ongoing and traverses td_chan->active_list.
If __td_ier_mask() reads ongoing as true right before the workqueue removes
the final descriptor from active_list in __td_finish(), wouldn't list_entry()
calculate a bogus pointer relative to the empty list head? This could cause a
read of unmapped memory or garbage when accessing td_desc->interrupt.
> iowrite32(ier, td->membase + TIMBDMA_IER);
> }
>
> +static void timb_dma_chan_bh(struct dma_chan *chan)
> +{
> + struct timb_dma_chan *td_chan = container_of(chan, struct timb_dma_chan,
> + chan);
> +
> + spin_lock(&td_chan->lock);
> + __td_finish(td_chan);
[Severity: High]
This is a pre-existing issue, but does moving to an asynchronous workqueue
widen a race condition here?
Currently, if td_issue_pending() races with a hardware interrupt, it can
acknowledge the interrupt and complete the transfer before the completion
path runs.
If td_issue_pending() also started the next transfer, this newly scheduled
timb_dma_chan_bh() workqueue will blindly call __td_finish() and complete
whatever is currently in the active_list. By changing the completion from a
tasklet (which had a very short race window) to a scheduled workqueue, the
delay is much longer. Will this result in erroneously aborting and completing
the newly started transfer?
> + if (!list_empty(&td_chan->queue))
> + __td_start_next(td_chan);
> + spin_unlock(&td_chan->lock);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785183549.git.allen.lkml@gmail.com?part=23
^ permalink raw reply [flat|nested] 137+ messages in thread
* [PATCH v2 24/64] dmaengine: txx9dmac: route callbacks via channel BH
2026-07-27 20:28 ` [PATCH v2 00/64] dmaengine: migrate channel tasklets to WQ_BH Allen Pais
` (22 preceding siblings ...)
2026-07-27 20:28 ` [PATCH v2 23/64] dmaengine: timb_dma: route callbacks via channel BH Allen Pais
@ 2026-07-27 20:28 ` Allen Pais
2026-07-27 21:00 ` sashiko-bot
2026-07-27 20:28 ` [PATCH v2 25/64] dmaengine: mv_xor_v2: use channel BH helpers Allen Pais
` (39 subsequent siblings)
63 siblings, 1 reply; 137+ messages in thread
From: Allen Pais @ 2026-07-27 20:28 UTC (permalink / raw)
To: Vinod Koul, Frank Li
Cc: Allen Pais, dmaengine, linux-kernel, Arnd Bergmann, Kees Cook
Use the per-channel BH helpers for descriptor completion handling and
schedule per-channel work from both channel and controller IRQ paths.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/dma/txx9dmac.c | 12 ++++++------
drivers/dma/txx9dmac.h | 1 -
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/txx9dmac.c b/drivers/dma/txx9dmac.c
index 05622b68a936..6269baa28c51 100644
--- a/drivers/dma/txx9dmac.c
+++ b/drivers/dma/txx9dmac.c
@@ -601,13 +601,13 @@ static void txx9dmac_scan_descriptors(struct txx9dmac_chan *dc)
}
}
-static void txx9dmac_chan_tasklet(struct tasklet_struct *t)
+static void txx9dmac_chan_tasklet(struct dma_chan *chan)
{
int irq;
u32 csr;
struct txx9dmac_chan *dc;
- dc = from_tasklet(dc, t, tasklet);
+ dc = container_of(chan, struct txx9dmac_chan, chan);
csr = channel_readl(dc, CSR);
dev_vdbg(chan2dev(&dc->chan), "tasklet: status=%x\n", csr);
@@ -628,7 +628,7 @@ static irqreturn_t txx9dmac_chan_interrupt(int irq, void *dev_id)
dev_vdbg(chan2dev(&dc->chan), "interrupt: status=%#x\n",
channel_readl(dc, CSR));
- tasklet_schedule(&dc->tasklet);
+ dma_chan_schedule_bh(&dc->chan);
/*
* Just disable the interrupts. We'll turn them back on in the
* softirq handler.
@@ -659,7 +659,7 @@ static void txx9dmac_tasklet(struct tasklet_struct *t)
spin_lock(&dc->lock);
if (csr & (TXX9_DMA_CSR_ABCHC | TXX9_DMA_CSR_NCHNC |
TXX9_DMA_CSR_NTRNFC))
- txx9dmac_scan_descriptors(dc);
+ dma_chan_schedule_bh(&dc->chan);
spin_unlock(&dc->lock);
}
}
@@ -1113,7 +1113,7 @@ static int __init txx9dmac_chan_probe(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0)
return irq;
- tasklet_setup(&dc->tasklet, txx9dmac_chan_tasklet);
+ dma_chan_init_bh(&dc->chan, txx9dmac_chan_tasklet);
dc->irq = irq;
err = devm_request_irq(&pdev->dev, dc->irq,
txx9dmac_chan_interrupt, 0, dev_name(&pdev->dev), dc);
@@ -1159,7 +1159,7 @@ static void txx9dmac_chan_remove(struct platform_device *pdev)
dma_async_device_unregister(&dc->dma);
if (dc->irq >= 0) {
devm_free_irq(&pdev->dev, dc->irq, dc);
- tasklet_kill(&dc->tasklet);
+ dma_chan_kill_bh(&dc->chan);
}
dc->ddev->chan[pdev->id % TXX9_DMA_MAX_NR_CHANNELS] = NULL;
}
diff --git a/drivers/dma/txx9dmac.h b/drivers/dma/txx9dmac.h
index aa53eafb1519..f2abedb73d49 100644
--- a/drivers/dma/txx9dmac.h
+++ b/drivers/dma/txx9dmac.h
@@ -162,7 +162,6 @@ struct txx9dmac_chan {
struct dma_device dma;
struct txx9dmac_dev *ddev;
void __iomem *ch_regs;
- struct tasklet_struct tasklet;
int irq;
u32 ccr;
--
2.43.0
^ permalink raw reply related [flat|nested] 137+ messages in thread* Re: [PATCH v2 24/64] dmaengine: txx9dmac: route callbacks via channel BH
2026-07-27 20:28 ` [PATCH v2 24/64] dmaengine: txx9dmac: " Allen Pais
@ 2026-07-27 21:00 ` sashiko-bot
0 siblings, 0 replies; 137+ messages in thread
From: sashiko-bot @ 2026-07-27 21:00 UTC (permalink / raw)
To: Allen Pais; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Uninitialized BH work and missing cleanup in shared IRQ mode
- [High] Interrupt storm and livelock due to premature IRQ unmasking in shared IRQ mode
- [High] Invalid `enable_irq()` parameter in shared IRQ mode
--
--- Patch [24]: [PATCH v2 24/64] dmaengine: txx9dmac: route callbacks via channel BH ---
commit ba661d7c1d6f85fdbcef7fb4031ee54ec6ee5c5a
Author: Allen Pais <allen.lkml@gmail.com>
dmaengine: txx9dmac: route callbacks via channel BH
Use the per-channel BH helpers for descriptor completion handling and
schedule per-channel work from both channel and controller IRQ paths.
> diff --git a/drivers/dma/txx9dmac.c b/drivers/dma/txx9dmac.c
> index 05622b68a9363..6269baa28c513 100644
> --- a/drivers/dma/txx9dmac.c
> +++ b/drivers/dma/txx9dmac.c
> @@ -601,13 +601,13 @@ static void txx9dmac_scan_descriptors(struct txx9dmac_chan *dc)
> }
> }
>
> -static void txx9dmac_chan_tasklet(struct tasklet_struct *t)
> +static void txx9dmac_chan_tasklet(struct dma_chan *chan)
> {
> int irq;
> u32 csr;
> struct txx9dmac_chan *dc;
>
> - dc = from_tasklet(dc, t, tasklet);
> + dc = container_of(chan, struct txx9dmac_chan, chan);
> csr = channel_readl(dc, CSR);
> dev_vdbg(chan2dev(&dc->chan), "tasklet: status=%x\n", csr);
>
[Severity: High]
Does this code handle shared IRQs correctly?
When running in shared IRQ mode, dc->irq is set to -1. Now that the shared
controller path schedules this channel BH, txx9dmac_chan_tasklet() will run
and unconditionally call enable_irq(dc->irq) at the end of the function:
irq = dc->irq;
enable_irq(irq);
Could this end up calling enable_irq(-1) and causing an issue in the core IRQ
code?
[ ... ]
> @@ -659,7 +659,7 @@ static void txx9dmac_tasklet(struct tasklet_struct *t)
> spin_lock(&dc->lock);
> if (csr & (TXX9_DMA_CSR_ABCHC | TXX9_DMA_CSR_NCHNC |
> TXX9_DMA_CSR_NTRNFC))
> - txx9dmac_scan_descriptors(dc);
> + dma_chan_schedule_bh(&dc->chan);
> spin_unlock(&dc->lock);
> }
> }
[Severity: High]
Could this result in an interrupt storm?
Previously, txx9dmac_scan_descriptors() would synchronously clear the hardware
interrupt condition before the shared tasklet unmasked the IRQ.
Now, the work is deferred to the async BH handler, but enable_irq(irq) is
still called synchronously at the end of txx9dmac_tasklet():
irq = ddev->irq;
enable_irq(irq);
Will the unmasked IRQ immediately fire again before the BH has a chance to run
and clear the hardware interrupt condition?
[ ... ]
> @@ -1113,7 +1113,7 @@ static int __init txx9dmac_chan_probe(struct platform_device *pdev)
> irq = platform_get_irq(pdev, 0);
> if (irq < 0)
> return irq;
> - tasklet_setup(&dc->tasklet, txx9dmac_chan_tasklet);
> + dma_chan_init_bh(&dc->chan, txx9dmac_chan_tasklet);
> dc->irq = irq;
> err = devm_request_irq(&pdev->dev, dc->irq,
> txx9dmac_chan_interrupt, 0, dev_name(&pdev->dev), dc);
[Severity: High]
Is the BH work properly initialized when operating in shared IRQ mode?
dma_chan_init_bh() is only called if dc->ddev->irq < 0:
if (dc->ddev->irq < 0) {
...
dma_chan_init_bh(&dc->chan, txx9dmac_chan_tasklet);
...
} else {
dc->irq = -1;
}
If the controller uses a shared IRQ, the BH work remains uninitialized.
When the shared controller tasklet attempts to schedule it, will it trigger a
WARN_ON(!chan->bh_work_initialized) and drop the completion request entirely?
[ ... ]
> @@ -1159,7 +1159,7 @@ static void txx9dmac_chan_remove(struct platform_device *pdev)
> dma_async_device_unregister(&dc->dma);
> if (dc->irq >= 0) {
> devm_free_irq(&pdev->dev, dc->irq, dc);
> - tasklet_kill(&dc->tasklet);
> + dma_chan_kill_bh(&dc->chan);
> }
> dc->ddev->chan[pdev->id % TXX9_DMA_MAX_NR_CHANNELS] = NULL;
> }
[Severity: High]
Could we leak resources here in shared IRQ mode?
Similar to the probe function, the cleanup relies on dc->irq >= 0. If the
initialization issue above is fixed so the BH work is correctly used in shared
IRQ mode, dma_chan_kill_bh() would still be skipped here sin