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

> From: [email protected] <[email protected]>
> 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 <[email protected]>
> 
> 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-
> [email protected]?part=1