[PATCH 0/4] dmatest: shared channel fixes and fatal errors option

[PATCH 0/4] dmatest: shared channel fixes and fatal errors option

[Nathan Lynch via B4 Relay]

Sending several fixes for issues in dmatest that arise when running
multiple threads per channel. I have encountered these issues while
testing the sdxi driver.

Patch 1 "Don't forcibly terminate channel in polled mode" is a resend of a
fix I sent a while back that didn't get picked up. [1]

Also included is a feature introducing a "fatal errors" parameter that
provides the option of panicking the kernel after a given number of errors
have occurred in a run.

Claude Code w/Opus 4.8 was used to investigate the problems fixed by
patches 2 and 3, and it assisted in writing the changes in patch 2-4.

[1] https://lore.kernel.org/dmaengine/20250408-dmaengine-dmatest-poll-mode-fix-v1-1-754a446a5363@amd.com/

Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>
---
Nathan Lynch (4):
      dmaengine: dmatest: Don't forcibly terminate channel in polled mode
      dmaengine: dmatest: Suppress bogus failures when stopping a run
      dmaengine: dmatest: Terminate channel before freeing threads
      dmaengine: dmatest: Optionally make test errors fatal

 drivers/dma/dmatest.c | 67 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 11 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260722-dmatest-fixes-and-fatal-errors-3b3653d5ac9b

Best regards,
--  
Nathan Lynch <nathan.lynch@amd.com>

[PATCH 1/4] dmaengine: dmatest: Don't forcibly terminate channel in polled mode

[Nathan Lynch via B4 Relay]

From: Nathan Lynch <nathan.lynch@amd.com>

Invoking dmaengine_terminate_sync() against the channel after each
submitted descriptor is strange. It's benign when the test is
single-threaded, but disruptive when multiple threads are submitting
descriptors to the channel concurrently:

  # cd /sys/module/dmatest/parameters/
  # echo 1 > polled
  # echo 2 > threads_per_chan
  # echo dma0chan0 > channel
  # echo 1 > run
...
  dmatest: Added 2 threads using dma0chan0
  dmatest: Started 2 threads using dma0chan0
  xilinx-zynqmp-dma ffa80000.dma: dma_sync_wait: timeout!
  xilinx-zynqmp-dma ffa80000.dma: dma_sync_wait: timeout!
  dmatest: dma0chan0-copy0: result #3: 'test timed out' with
    src_off=0x487 dst_off=0xf8 len=0x171b (0)
  dmatest: dma0chan0-copy1: result #6: 'test timed out' with
    src_off=0x227d dst_off=0xf99 len=0xf7a (0)

Remove the call to dmaengine_terminate_sync() from the main thread
loop.

Fixes: fb9816f9d05f ("dmaengine: dmatest: Add support for completion polling")
Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>
---
 drivers/dma/dmatest.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 2ae3469397f3..60bc448f42ee 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -837,7 +837,6 @@ static int dmatest_func(void *data)
 
 		if (params->polled) {
 			status = dma_sync_wait(chan, cookie);
-			dmaengine_terminate_sync(chan);
 			if (status == DMA_COMPLETE)
 				done->done = true;
 		} else {

-- 
2.54.0

Re: [PATCH 1/4] dmaengine: dmatest: Don't forcibly terminate channel in polled mode

[sashiko-bot]

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

New issues:
- [High] Removing dmaengine_terminate_sync(chan) from the polled mode path causes the test to unmap DMA buffers while the hardware might still be actively processing the queued descriptor.

Pre-existing issues:
- [High] Test data buffers are freed before pending DMA transfers are terminated at the end of the test thread, creating a race window where active DMA hardware can corrupt freed slab memory.
--

--- Patch [1]: [PATCH 1/4] dmaengine: dmatest: Don't forcibly terminate channel in polled mode ---
commit ff3488aa5c4eb22b5aa3631448708f7560e37c84
Author: Nathan Lynch <nathan.lynch@amd.com>

dmaengine: dmatest: Don't forcibly terminate channel in polled mode
    
This commit removes the call to dmaengine_terminate_sync() from the
main thread loop for polled transfers. This prevents disruptive
terminations when multiple threads are concurrently submitting descriptors.

> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index 2ae3469397f37..60bc448f42eeb 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -837,7 +837,6 @@ static int dmatest_func(void *data)
>  
>  		if (params->polled) {
>  			status = dma_sync_wait(chan, cookie);
> -			dmaengine_terminate_sync(chan);
>  			if (status == DMA_COMPLETE)

[Severity: High]
By removing the termination here, does this code leave the hardware running
if dma_sync_wait() times out?

If dma_sync_wait() returns DMA_ERROR on a timeout, done->done remains false.
The loop then eventually branches to error_unmap_continue without terminating
the pending transfer.

This error path calls dmaengine_unmap_put(), unmapping the buffers while the
hardware might still be actively processing the queued descriptor. Could this
lead to active DMA memory corruption?

[Severity: High]
This is a pre-existing issue, but does dmatest_func() free test buffers
before terminating pending DMA transfers during thread cleanup?

Looking at the error handling and exit path at the end of the test thread:

dmatest_func() {
    ...
    err_dst:
        dmatest_free_test_data(dst);
    err_src:
        dmatest_free_test_data(src);
    ...
    err_thread_type:
        dmaengine_terminate_sync(chan);
    ...
}

If a transfer had timed out and was left running, could the DMA hardware
execute the pending descriptor in the window between freeing the slab
memory and terminating the channel?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-dmatest-fixes-and-fatal-errors-v1-0-8287709987ea@amd.com?part=1

[PATCH 2/4] dmaengine: dmatest: Suppress bogus failures when stopping a run

[Nathan Lynch via B4 Relay]

From: Nathan Lynch <nathan.lynch@amd.com>

Writing 0 to the module 'run' attribute stops the test threads with
kthread_stop(), waking threads out of wait_event_freezable_timeout()
before the last transfer's completion callback has run. The stopping
thread samples the transfer status and then checks done->done at two
different instants, so the last transaction gets misreported two ways:

- "test timed out": the wait returns early with done->done still clear
  even though the transfer is simply still finishing — not a real
  timeout.

- "completion busy status": status is read as DMA_IN_PROGRESS a moment
  before the transfer completes and latches done->done, so the stale
  status is reported as a failure against a transfer that actually
  succeeded.

This is not a recent regression: the threads have used an interruptible,
freezable timeout wait since long before the wait queue was moved into
thread context by commit 6f6a23a213be ("dmaengine: dmatest: move callback
wait queue to thread context"), so a stop has always been able to wake
the wait before the final callback runs.

When kthread_should_stop() returns true, use dma_sync_wait() to settle
the pending transaction. Real errors still yield DMA_ERROR.

Assisted-by: Claude:claude-opus-4-8 [Claude-Code]
Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>
---
 drivers/dma/dmatest.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 60bc448f42ee..3267c7e86f5d 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -702,6 +702,7 @@ static int dmatest_func(void *data)
 		struct dmaengine_unmap_data *um;
 		dma_addr_t *dsts;
 		unsigned int len;
+		bool stopping;
 
 		total_tests++;
 
@@ -839,6 +840,7 @@ static int dmatest_func(void *data)
 			status = dma_sync_wait(chan, cookie);
 			if (status == DMA_COMPLETE)
 				done->done = true;
+			stopping = kthread_should_stop();
 		} else {
 			dma_async_issue_pending(chan);
 
@@ -846,14 +848,19 @@ static int dmatest_func(void *data)
 					done->done,
 					msecs_to_jiffies(params->timeout));
 
-			status = dma_async_is_tx_complete(chan, cookie, NULL,
-							  NULL);
+			stopping = kthread_should_stop();
+			status = stopping ?
+				dma_sync_wait(chan, cookie) :
+				dma_async_is_tx_complete(chan, cookie, NULL, NULL);
 		}
 
 		if (!done->done) {
-			result("test timed out", total_tests, src->off, dst->off,
-			       len, 0);
-			goto error_unmap_continue;
+			/* stopping: dma_sync_wait() let the transfer finish, not a timeout */
+			if (!stopping) {
+				result("test timed out", total_tests, src->off,
+				       dst->off, len, 0);
+				goto error_unmap_continue;
+			}
 		} else if (status != DMA_COMPLETE &&
 			   !(dma_has_cap(DMA_COMPLETION_NO_ORDER,
 					 dev->cap_mask) &&

-- 
2.54.0

Re: [PATCH 2/4] dmaengine: dmatest: Suppress bogus failures when stopping a run

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Bypassing the error path during thread stop prevents dmaengine_terminate_sync() from being called, leading to a Use-After-Free of the dmatest_thread struct by the completion callback.
- [Medium] Bypassed DMA status check when stopping results in unhandled DMA errors.
--

--- Patch [2]: [PATCH 2/4] dmaengine: dmatest: Suppress bogus failures when stopping a run ---
commit 65f43e5ca342a54064656c1ebe548edaa5c088bd
Author: Nathan Lynch <nathan.lynch@amd.com>

dmaengine: dmatest: Suppress bogus failures when stopping a run

Writing 0 to the module 'run' attribute stops the test threads with
kthread_stop(). This commit introduces checks against kthread_should_stop()
to suppress stale failures against transfers that actually succeeded.

> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
[ ... ]
> @@ -848,14 +848,19 @@ static int dmatest_func(void *data)
>  					done->done,
>  					msecs_to_jiffies(params->timeout));
>  
> -			status = dma_async_is_tx_complete(chan, cookie, NULL,
> -							  NULL);
> +			stopping = kthread_should_stop();
> +			status = stopping ?
> +				dma_sync_wait(chan, cookie) :
> +				dma_async_is_tx_complete(chan, cookie, NULL, NULL);
>  		}
>  
>  		if (!done->done) {
> -			result("test timed out", total_tests, src->off, dst->off,
> -			       len, 0);
> -			goto error_unmap_continue;
> +			/* stopping: dma_sync_wait() let the transfer finish, not a timeout */
> +			if (!stopping) {
> +				result("test timed out", total_tests, src->off,
> +				       dst->off, len, 0);
> +				goto error_unmap_continue;
> +			}

[Severity: Medium]
By skipping this error path when stopping is true, does this bypass evaluating
the status returned from dma_sync_wait()?

If a legitimate hardware error or timeout occurs exactly when the thread is
stopped, it looks like it would either falsely report a successful test (if
noverify is set) or misreport it as a data mismatch.

[Severity: High]
Could skipping the goto error_unmap_continue here lead to a use-after-free
in a delayed callback?

If a delayed callback leaves done->done false when stopping, failed_tests
won't be incremented. Later, when dmatest_func() exits:

    if (ret || failed_tests)
        dmaengine_terminate_sync(chan);

Since failed_tests remained 0, the thread exits without synchronizing the
channel. Then dmatest_cleanup_channel() executes:

    put_task_struct(thread->task);
    kfree(thread);
}

/* terminate all transfers on specified channels */
dmaengine_terminate_sync(dtc->chan);

Is there a window where a pending tasklet executes dmatest_callback()
after kfree(thread) but before dmaengine_terminate_sync() is finally called,
leading to a use-after-free on the thread structure?

>  		} else if (status != DMA_COMPLETE &&
>  			   !(dma_has_cap(DMA_COMPLETION_NO_ORDER,
>  					 dev->cap_mask) &&

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-dmatest-fixes-and-fatal-errors-v1-0-8287709987ea@amd.com?part=2

[PATCH 3/4] dmaengine: dmatest: Terminate channel before freeing threads

[Nathan Lynch via B4 Relay]

From: Nathan Lynch <nathan.lynch@amd.com>

On a channel shared by multiple threads, the completion callbacks run from
one tasklet, and dmaengine_terminate_sync() flushes it. Freeing a thread
before that flush can leave a pending callback pointing at freed memory
(dmatest_callback() reads thread->done).

Stop all channel threads and then flush the channel before freeing the
threads.

Fixes: 6f6a23a213be ("dmaengine: dmatest: move callback wait queue to thread context")
Assisted-by: Claude:claude-opus-4-8 [Claude-Code]
Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>
---
 drivers/dma/dmatest.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 3267c7e86f5d..0d7752c2451a 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -959,18 +959,23 @@ static void dmatest_cleanup_channel(struct dmatest_chan *dtc)
 	struct dmatest_thread	*_thread;
 	int			ret;
 
-	list_for_each_entry_safe(thread, _thread, &dtc->threads, node) {
+	/* stop all threads first */
+	list_for_each_entry(thread, &dtc->threads, node) {
 		ret = kthread_stop(thread->task);
 		pr_debug("thread %s exited with status %d\n",
 			 thread->task->comm, ret);
+	}
+
+	/* flush the channel (e.g. vchan_synchronize() -> tasklet_kill()) */
+	dmaengine_terminate_sync(dtc->chan);
+
+	/* now it is safe to free the thread memory */
+	list_for_each_entry_safe(thread, _thread, &dtc->threads, node) {
 		list_del(&thread->node);
 		put_task_struct(thread->task);
 		kfree(thread);
 	}
 
-	/* terminate all transfers on specified channels */
-	dmaengine_terminate_sync(dtc->chan);
-
 	kfree(dtc);
 }
 

-- 
2.54.0

Re: [PATCH 3/4] dmaengine: dmatest: Terminate channel before freeing threads

[sashiko-bot]

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

New issues:
- [High] Spurious WARN_ON due to racing callback execution and thread shutdown.

Pre-existing issues:
- [High] Incomplete fix leaves Use-After-Free for drivers lacking terminate_all support.
--

--- Patch [3]: [PATCH 3/4] dmaengine: dmatest: Terminate channel before freeing threads ---
commit 45380ad31d07f2d946845ad6c99217e3ae5ea8ae
Author: Nathan Lynch <nathan.lynch@amd.com>

dmaengine: dmatest: Terminate channel before freeing threads

This commit modifies dmatest_cleanup_channel() to stop all threads first,
flush the channel, and only then free the thread memory. This prevents
pending callbacks from accessing freed memory.

> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index 3267c7e86f5d5..0d7752c2451a9 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -959,18 +959,23 @@ static void dmatest_cleanup_channel(struct dmatest_chan *dtc)
>  	struct dmatest_thread	*_thread;
>  	int			ret;
>  
> -	list_for_each_entry_safe(thread, _thread, &dtc->threads, node) {
> +	/* stop all threads first */
> +	list_for_each_entry(thread, &dtc->threads, node) {
>  		ret = kthread_stop(thread->task);
>  		pr_debug("thread %s exited with status %d\n",
>  			 thread->task->comm, ret);
> +	}
> +
> +	/* flush the channel (e.g. vchan_synchronize() -> tasklet_kill()) */
> +	dmaengine_terminate_sync(dtc->chan);

[Severity: High]
Could this trigger a spurious WARN_ON when flushing pending callbacks?

When a test is stopped, kthread_stop() runs, and dmatest_func() sets
thread->done to true before exiting.

dmatest_cleanup_channel() then calls dmaengine_terminate_sync() which flushes
any pending callbacks.

Since thread->done is already true, will the pending dmatest_callback() read
it and incorrectly fall into the warning path?

drivers/dma/dmatest.c:dmatest_callback() {
    ...
    } else {
        /* ... */
        WARN(1, "dmatest: Kernel memory may be corrupted!!\n");
    }
}

[Severity: High]
This is a pre-existing issue, but does ignoring the return value of
dmaengine_terminate_sync() leave a use-after-free risk for drivers that lack
terminate_all support?

If a driver doesn't support device_terminate_all, dmaengine_terminate_sync()
returns -EINVAL and skips dmaengine_synchronize():

include/linux/dmaengine.h:dmaengine_terminate_sync() {
    ...
    ret = dmaengine_terminate_async(chan);
    if (ret)
        return ret;

    dmaengine_synchronize(chan);
    return 0;
}

If synchronization is skipped, couldn't the hardware transfer still finish
and fire the tasklet after we proceed to kfree(thread) below, causing
dmatest_callback() to access freed memory?

> +
> +	/* now it is safe to free the thread memory */
> +	list_for_each_entry_safe(thread, _thread, &dtc->threads, node) {
>  		list_del(&thread->node);
>  		put_task_struct(thread->task);
>  		kfree(thread);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-dmatest-fixes-and-fatal-errors-v1-0-8287709987ea@amd.com?part=3

[PATCH 4/4] dmaengine: dmatest: Optionally make test errors fatal

[Nathan Lynch via B4 Relay]

From: Nathan Lynch <nathan.lynch@amd.com>

For automated testing it is useful to have dmatest halt the machine when
errors occur rather than logging them and continuing, so that the system
state at the point of failure can be captured instead of allowing the run
to carry on.

Add a 'fatal_errors' module parameter, defaulting to 0. When set to a
non-zero value N, panic the kernel once N test errors have accumulated in
a run; the count is reset at the start of each run. The panic is triggered
after the existing result() line is logged, so the specific error (data
mismatch, mapping/prep/submit error, timeout, or bad completion status) is
recorded before the crash.

Assisted-by: Claude:claude-opus-4-8 [Claude-Code]
Signed-off-by: Nathan Lynch <nathan.lynch@amd.com>
---
 drivers/dma/dmatest.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 0d7752c2451a..a070ce6f2fa9 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -7,6 +7,7 @@
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/atomic.h>
 #include <linux/err.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
@@ -92,6 +93,11 @@ static bool polled;
 module_param(polled, bool, 0644);
 MODULE_PARM_DESC(polled, "Use polling for completion instead of interrupts");
 
+static unsigned int fatal_errors;
+module_param(fatal_errors, uint, 0644);
+MODULE_PARM_DESC(fatal_errors,
+		"Panic after this many test errors in a run; 0 = never (default: 0)");
+
 /**
  * struct dmatest_params - test parameters.
  * @nobounce:		prevent using swiotlb buffer
@@ -109,6 +115,7 @@ MODULE_PARM_DESC(polled, "Use polling for completion instead of interrupts");
  * @alignment:		custom data address alignment taken as 2^alignment
  * @transfer_size:	custom transfer size in bytes
  * @polled:		use polling for completion instead of interrupts
+ * @fatal_errors:	panic after this many test errors in a run (0 = never)
  */
 struct dmatest_params {
 	bool		nobounce;
@@ -126,6 +133,7 @@ struct dmatest_params {
 	int		alignment;
 	unsigned int	transfer_size;
 	bool		polled;
+	unsigned int	fatal_errors;
 };
 
 /**
@@ -133,6 +141,7 @@ struct dmatest_params {
  * @params:		test parameters
  * @channels:		channels under test
  * @nr_channels:	number of channels under test
+ * @error_count:	test errors in the current run (for fatal_errors)
  * @lock:		access protection to the fields of this structure
  * @did_init:		module has been initialized completely
  * @last_error:		test has faced configuration issues
@@ -144,6 +153,7 @@ struct dmatest_info {
 	/* Internal state */
 	struct list_head	channels;
 	unsigned int		nr_channels;
+	atomic_t		error_count;
 	int			last_error;
 	struct mutex		lock;
 	bool			did_init;
@@ -487,6 +497,24 @@ static void dbg_result(const char *err, unsigned int n, unsigned int src_off,
 		dbg_result(err, n, src_off, dst_off, len, data);\
 })
 
+/*
+ * Record a test error whose details were just logged via result(). When
+ * fatal_errors is set, panic once that many errors have accumulated in the
+ * current run.
+ */
+static void dmatest_note_error(struct dmatest_info *info)
+{
+	unsigned int errors;
+
+	if (!info->params.fatal_errors)
+		return;
+
+	errors = atomic_inc_return(&info->error_count);
+	if (errors >= info->params.fatal_errors)
+		panic("dmatest: fatal_errors=%u and %u error(s) encountered\n",
+		      info->params.fatal_errors, errors);
+}
+
 static unsigned long long dmatest_persec(s64 runtime, unsigned int val)
 {
 	unsigned long long per_sec = 1000000;
@@ -583,7 +611,7 @@ static int dmatest_func(void *data)
 	struct dma_device	*dev;
 	struct device		*dma_dev;
 	unsigned int		error_count;
-	unsigned int		failed_tests = 0;
+	unsigned int		failed_tests = 0;	/* for this thread */
 	unsigned int		total_tests = 0;
 	dma_cookie_t		cookie;
 	enum dma_status		status;
@@ -755,6 +783,7 @@ static int dmatest_func(void *data)
 			failed_tests++;
 			result("unmap data NULL", total_tests,
 			       src->off, dst->off, len, ret);
+			dmatest_note_error(info);
 			continue;
 		}
 
@@ -910,6 +939,7 @@ static int dmatest_func(void *data)
 			result("data error", total_tests, src->off, dst->off,
 			       len, error_count);
 			failed_tests++;
+			dmatest_note_error(info);
 		} else {
 			verbose_result("test passed", total_tests, src->off,
 				       dst->off, len, 0);
@@ -920,6 +950,7 @@ static int dmatest_func(void *data)
 error_unmap_continue:
 		dmaengine_unmap_put(um);
 		failed_tests++;
+		dmatest_note_error(info);
 	}
 	ktime = ktime_sub(ktime_get(), ktime);
 	ktime = ktime_sub(ktime, comparetime);
@@ -1135,6 +1166,7 @@ static void add_threaded_test(struct dmatest_info *info)
 	params->alignment = alignment;
 	params->transfer_size = transfer_size;
 	params->polled = polled;
+	params->fatal_errors = fatal_errors;
 
 	request_channels(info, DMA_MEMCPY);
 	request_channels(info, DMA_MEMSET);
@@ -1147,6 +1179,8 @@ static void run_pending_tests(struct dmatest_info *info)
 	struct dmatest_chan *dtc;
 	unsigned int thread_count = 0;
 
+	atomic_set(&info->error_count, 0);
+
 	list_for_each_entry(dtc, &info->channels, node) {
 		struct dmatest_thread *thread;
 

-- 
2.54.0

Re: [PATCH 4/4] dmaengine: dmatest: Optionally make test errors fatal

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition on `info->params.fatal_errors` can cause an unintended kernel panic when the parameter is concurrently set to 0 ('never panic').
--

--- Patch [4]: [PATCH 4/4] dmaengine: dmatest: Optionally make test errors fatal ---
commit 289cdfd12c204ee46d7f18f6c08608f36bd53956
Author: Nathan Lynch <nathan.lynch@amd.com>

dmaengine: dmatest: Optionally make test errors fatal

This commit adds a module parameter fatal_errors to dmatest. When set to a
non-zero value, the kernel will panic after accumulating that many test errors
during a run.

> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index 0d7752c2451a9..a070ce6f2fa94 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c

[ ... ]

> @@ -487,6 +497,24 @@ static void dbg_result(const char *err, unsigned int n, unsigned int src_off,
>  		dbg_result(err, n, src_off, dst_off, len, data);\
>  })
>  
> +/*
> + * Record a test error whose details were just logged via result(). When
> + * fatal_errors is set, panic once that many errors have accumulated in the
> + * current run.
> + */
> +static void dmatest_note_error(struct dmatest_info *info)
> +{
> +	unsigned int errors;
> +
> +	if (!info->params.fatal_errors)
> +		return;
> +
> +	errors = atomic_inc_return(&info->error_count);
> +	if (errors >= info->params.fatal_errors)
> +		panic("dmatest: fatal_errors=%u and %u error(s) encountered\n",
> +		      info->params.fatal_errors, errors);

[Severity: High]
Can a concurrent modification of the fatal_errors parameter cause a spurious
kernel panic here?

If a user modifies the fatal_errors parameter to 0 (meaning 'never panic') via
sysfs while test threads are executing, info->params.fatal_errors could be
concurrently overwritten with 0 via add_threaded_test().

If this happens between the initial check and the second read:

CPU1 in dmatest_note_error()
    if (!info->params.fatal_errors) // reads > 0
        return;

    errors = atomic_inc_return(&info->error_count); // acts as a barrier

CPU2 modifying sysfs
    info->params.fatal_errors = 0;

CPU1 in dmatest_note_error()
    if (errors >= info->params.fatal_errors) // reads 0

Will the condition errors >= 0 evaluate to true and inadvertently trigger the
panic? Could we read info->params.fatal_errors into a local variable once to
prevent this race?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-dmatest-fixes-and-fatal-errors-v1-0-8287709987ea@amd.com?part=4