[PATCH] dmaengine: dmatest: fix race between wait for thread and thread start

[PATCH] dmaengine: dmatest: fix race between wait for thread and thread start

[Cosmin Tanislav]

Each dmatest worker thread has a pending and a done flag which describe
the thread's lifecycle.

pending is initialized to true when the thread is added to the threads
list. pending is set to false once the thread is actually running. done
is set to true once the thread has finished its testing.

When the wait module parameter is set to true, dmatest_init() waits for
the !is_threaded_test_run() condition to become true.

If the worker thread is not started and running by the time
dmatest_init() waits, the !is_threaded_test_run() condition will be true
and the wait will stop early, before the worker thread is even started.
dmatest_init() exits, defeating the whole purpose of the wait.

If then we try to remove the module, kthread_stop() will try to cleanup
the worker threads, interrupting wait_event_freezable_timeout(), which
will return -ERESTARTSYS, causing the "test timed out" message to be
printed.

Clear the pending flag once we wake up the worker thread to avoid this
race.

Fixes: d53513d5dc28 ("dmaengine: dmatest: Add support for multi channel testing")
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---
 drivers/dma/dmatest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 2ae3469397f3..2d2eb43eee7c 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -610,7 +610,6 @@ static int dmatest_func(void *data)
 	ret = -ENOMEM;
 
 	smp_rmb();
-	thread->pending = false;
 	info = thread->info;
 	params = &info->params;
 	chan = thread->chan;
@@ -1142,6 +1141,7 @@ static void run_pending_tests(struct dmatest_info *info)
 		thread_count = 0;
 		list_for_each_entry(thread, &dtc->threads, node) {
 			wake_up_process(thread->task);
+			thread->pending = false;
 			thread_count++;
 		}
 		pr_info("Started %u threads using %s\n",
-- 
2.55.0

Re: [PATCH] dmaengine: dmatest: fix race between wait for thread and thread start

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] TOCTOU race in pending flag update allows premature wait completion.

Pre-existing issues:
- [High] Lockless list traversal in wait condition causes Use-After-Free kernel crash.
--

--- Patch [1]: [PATCH] dmaengine: dmatest: fix race between wait for thread and thread start ---
commit 6a5e8f24ddb4b9e471d1801d58f2c91b07f83d13
Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>

dmaengine: dmatest: fix race between wait for thread and thread start

This commit addresses a race condition in the dmatest module where the wait
module parameter could cause an early exit if the worker thread has not yet
started. It attempts to resolve this by clearing the pending flag immediately
after waking up the worker thread.

> When the wait module parameter is set to true, dmatest_init() waits for
> the !is_threaded_test_run() condition to become true.

[Severity: High]
This is a pre-existing issue, but does evaluating this condition repeatedly
in wait_event() without holding a lock lead to a use-after-free?

Looking at is_threaded_test_run(), it iterates over info->channels and
dtc->threads without holding info->lock:

drivers/dma/dmatest.c:is_threaded_test_run() {
    ...
    list_for_each_entry(dtc, &info->channels, node) {
        ...
    }
    ...
}

If stop_threaded_test() is executed concurrently via sysfs, it can modify
the list nodes. Since the traversal in is_threaded_test_run() happens
without holding the lock, could this hit freed memory and result in a
direct use-after-free?

> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index 2ae3469397f37..2d2eb43eee7cc 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
[ ... ]
> @@ -1142,6 +1141,7 @@ static void run_pending_tests(struct dmatest_info *info)
>  		thread_count = 0;
>  		list_for_each_entry(thread, &dtc->threads, node) {
>  			wake_up_process(thread->task);
> +			thread->pending = false;

[Severity: Medium]
Could this order of operations introduce a race window?

If wake_up_process() makes the thread runnable immediately, could a
concurrent wait_event() evaluate a stale pending state before it is
cleared here?

Would swapping these operations to clear the pending flag strictly before
the wakeup prevent concurrent wait_event() evaluations from exiting
prematurely?

>  			thread_count++;
>  		}
>  		pr_info("Started %u threads using %s\n",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817192557.437474-1-cosmin-gabriel.tanislav.xa@renesas.com?part=1

RE: [PATCH] dmaengine: dmatest: fix race between wait for thread and thread start

[Cosmin-Gabriel Tanislav]

> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Monday, August 17, 2026 10:41 PM
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Medium] TOCTOU race in pending flag update allows premature wait completion.
> 
> Pre-existing issues:
> - [High] Lockless list traversal in wait condition causes Use-After-Free kernel crash.
> --
> 
> --- Patch [1]: [PATCH] dmaengine: dmatest: fix race between wait for thread and thread start ---
> commit 6a5e8f24ddb4b9e471d1801d58f2c91b07f83d13
> Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> 
> dmaengine: dmatest: fix race between wait for thread and thread start
> 
> This commit addresses a race condition in the dmatest module where the wait
> module parameter could cause an early exit if the worker thread has not yet
> started. It attempts to resolve this by clearing the pending flag immediately
> after waking up the worker thread.
> 
> > When the wait module parameter is set to true, dmatest_init() waits for
> > the !is_threaded_test_run() condition to become true.
> 
> [Severity: High]
> This is a pre-existing issue, but does evaluating this condition repeatedly
> in wait_event() without holding a lock lead to a use-after-free?
> 
> Looking at is_threaded_test_run(), it iterates over info->channels and
> dtc->threads without holding info->lock:
> 
> drivers/dma/dmatest.c:is_threaded_test_run() {
>     ...
>     list_for_each_entry(dtc, &info->channels, node) {
>         ...
>     }
>     ...
> }
> 
> If stop_threaded_test() is executed concurrently via sysfs, it can modify
> the list nodes. Since the traversal in is_threaded_test_run() happens
> without holding the lock, could this hit freed memory and result in a
> direct use-after-free?
> 
> > diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> > index 2ae3469397f37..2d2eb43eee7cc 100644
> > --- a/drivers/dma/dmatest.c
> > +++ b/drivers/dma/dmatest.c
> [ ... ]
> > @@ -1142,6 +1141,7 @@ static void run_pending_tests(struct dmatest_info *info)
> >               thread_count = 0;
> >               list_for_each_entry(thread, &dtc->threads, node) {
> >                       wake_up_process(thread->task);
> > +                     thread->pending = false;
> 
> [Severity: Medium]
> Could this order of operations introduce a race window?
> 
> If wake_up_process() makes the thread runnable immediately, could a
> concurrent wait_event() evaluate a stale pending state before it is
> cleared here?
> 
> Would swapping these operations to clear the pending flag strictly before
> the wakeup prevent concurrent wait_event() evaluations from exiting
> prematurely?
> 

I didn't hit this race during testing but Sashiko's suggestion sounds
correct. I will re-test with it and submit V2.

> >                       thread_count++;
> >               }
> >               pr_info("Started %u threads using %s\n",
> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260817192557.437474-1-cosmin-
> gabriel.tanislav.xa@renesas.com?part=1