[PATCH v3] lib/tests: test_ratelimit: fix stress test thread lifecycle and leak

[PATCH v3] lib/tests: test_ratelimit: fix stress test thread lifecycle and leak

[Jia He]

The stress test's WARN_ON_ONCE(!sktp->tp) check in the child thread is
racy and unnecessary: since kthread_run() wakes the thread before
returning, the child can run before sktp[i].tp has been assigned.
Moreover, sktp->tp is never actually used in the child function, so the
check serves no purpose. Remove it and keep the original kthread_run()

Also add a common cleanup path for thread creation failures. If creating
one of the later threads fails, stop all threads that were already
started and free the allocated array instead of leaving orphan kthreads
and leaked memory behind.

Finally, replace the module-static doneflag with kthread_should_stop().
With the doneflag, child threads may exit before the parent calls
kthread_stop(), so the task lifetime is no longer guaranteed when the
parent later tries to stop them. Using kthread_should_stop() keeps each
child alive until kthread_stop() synchronously terminates it.

Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Jia He <justin.he@arm.com>
---

v3: keep kthread_run() and remove the racy WARN_ON_ONCE(!sktp->tp)
    check instead, as suggested by Petr Mladek
v2: replace the module-static doneflag with kthread_should_stop() to
    avoid UAF (sashiko-bot).

 lib/tests/test_ratelimit.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/lib/tests/test_ratelimit.c b/lib/tests/test_ratelimit.c
index 33cea5f3d28b..e244f8cd47d7 100644
--- a/lib/tests/test_ratelimit.c
+++ b/lib/tests/test_ratelimit.c
@@ -68,7 +68,6 @@ static void test_ratelimit_smoke(struct kunit *test)
 static struct ratelimit_state stressrl = RATELIMIT_STATE_INIT_FLAGS("stressrl", HZ / 10, 3,
 								    RATELIMIT_MSG_ON_RELEASE);
 
-static int doneflag;
 static const int stress_duration = 2 * HZ;
 
 struct stress_kthread {
@@ -84,9 +83,8 @@ static int test_ratelimit_stress_child(void *arg)
 	struct stress_kthread *sktp = arg;
 
 	set_user_nice(current, MAX_NICE);
-	WARN_ON_ONCE(!sktp->tp);
 
-	while (!READ_ONCE(doneflag)) {
+	while (!kthread_should_stop()) {
 		sktp->nattempts++;
 		if (___ratelimit(&stressrl, __func__))
 			sktp->nunlimited++;
@@ -105,26 +103,37 @@ static void test_ratelimit_stress(struct kunit *test)
 	const int n_stress_kthread = cpumask_weight(cpu_online_mask);
 	struct stress_kthread skt = { 0 };
 	struct stress_kthread *sktp = kzalloc_objs(*sktp, n_stress_kthread);
+	int n_started = 0;
 
-	KUNIT_EXPECT_NOT_NULL_MSG(test, sktp, "Memory allocation failure");
+	KUNIT_ASSERT_NOT_NULL_MSG(test, sktp, "Memory allocation failure");
 	for (i = 0; i < n_stress_kthread; i++) {
 		sktp[i].tp = kthread_run(test_ratelimit_stress_child, &sktp[i], "%s/%i",
 					 "test_ratelimit_stress_child", i);
-		KUNIT_EXPECT_NOT_NULL_MSG(test, sktp, "kthread creation failure");
+		if (IS_ERR(sktp[i].tp)) {
+			KUNIT_FAIL(test, "kthread_run failed: %ld", PTR_ERR(sktp[i].tp));
+			goto out_stop;
+		}
+		n_started++;
 		pr_alert("Spawned test_ratelimit_stress_child %d\n", i);
 	}
 	schedule_timeout_idle(stress_duration);
-	WRITE_ONCE(doneflag, 1);
-	for (i = 0; i < n_stress_kthread; i++) {
+
+out_stop:
+	for (i = 0; i < n_started; i++) {
 		kthread_stop(sktp[i].tp);
 		skt.nattempts += sktp[i].nattempts;
 		skt.nunlimited += sktp[i].nunlimited;
 		skt.nlimited += sktp[i].nlimited;
 		skt.nmissed += sktp[i].nmissed;
 	}
-	KUNIT_ASSERT_EQ_MSG(test, skt.nunlimited + skt.nlimited, skt.nattempts,
-			    "Outcomes not equal to attempts");
-	KUNIT_ASSERT_EQ_MSG(test, skt.nlimited, skt.nmissed, "Misses not equal to limits");
+	if (n_started == n_stress_kthread) {
+		KUNIT_ASSERT_EQ_MSG(test, skt.nunlimited + skt.nlimited, skt.nattempts,
+				    "Outcomes not equal to attempts");
+		KUNIT_ASSERT_EQ_MSG(test, skt.nlimited, skt.nmissed,
+				    "Misses not equal to limits");
+	}
+
+	kfree(sktp);
 }
 
 static struct kunit_case ratelimit_test_cases[] = {
-- 
2.34.1

Re: [PATCH v3] lib/tests: test_ratelimit: fix stress test thread lifecycle and leak

[Petr Mladek]

On Fri 2026-05-29 08:58:55, Jia He wrote:
> The stress test's WARN_ON_ONCE(!sktp->tp) check in the child thread is
> racy and unnecessary: since kthread_run() wakes the thread before
> returning, the child can run before sktp[i].tp has been assigned.
> Moreover, sktp->tp is never actually used in the child function, so the
> check serves no purpose. Remove it and keep the original kthread_run()
> 
> Also add a common cleanup path for thread creation failures. If creating
> one of the later threads fails, stop all threads that were already
> started and free the allocated array instead of leaving orphan kthreads
> and leaked memory behind.
> 
> Finally, replace the module-static doneflag with kthread_should_stop().
> With the doneflag, child threads may exit before the parent calls
> kthread_stop(), so the task lifetime is no longer guaranteed when the
> parent later tries to stop them. Using kthread_should_stop() keeps each
> child alive until kthread_stop() synchronously terminates it.
> 
> Suggested-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Jia He <justin.he@arm.com>

Looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>

Now, the question is who would push it.

I could take it via the printk tree. Or Paul would you prefer to
take it yourself?

Best Regards,
Petr

Re: [PATCH v3] lib/tests: test_ratelimit: fix stress test thread lifecycle and leak

[Paul E. McKenney]

On Wed, Jun 24, 2026 at 05:12:03PM +0200, Petr Mladek wrote:
> On Fri 2026-05-29 08:58:55, Jia He wrote:
> > The stress test's WARN_ON_ONCE(!sktp->tp) check in the child thread is
> > racy and unnecessary: since kthread_run() wakes the thread before
> > returning, the child can run before sktp[i].tp has been assigned.
> > Moreover, sktp->tp is never actually used in the child function, so the
> > check serves no purpose. Remove it and keep the original kthread_run()
> > 
> > Also add a common cleanup path for thread creation failures. If creating
> > one of the later threads fails, stop all threads that were already
> > started and free the allocated array instead of leaving orphan kthreads
> > and leaked memory behind.
> > 
> > Finally, replace the module-static doneflag with kthread_should_stop().
> > With the doneflag, child threads may exit before the parent calls
> > kthread_stop(), so the task lifetime is no longer guaranteed when the
> > parent later tries to stop them. Using kthread_should_stop() keeps each
> > child alive until kthread_stop() synchronously terminates it.
> > 
> > Suggested-by: Petr Mladek <pmladek@suse.com>
> > Signed-off-by: Jia He <justin.he@arm.com>
> 
> Looks good to me:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Tested-by: Petr Mladek <pmladek@suse.com>
> 
> Now, the question is who would push it.
> 
> I could take it via the printk tree. Or Paul would you prefer to
> take it yourself?

I am happy for you to take it:

Acked-by: Paul E. McKenney <paulmck@kernel.org>

Re: [PATCH v3] lib/tests: test_ratelimit: fix stress test thread lifecycle and leak

[Petr Mladek]

On Fri 2026-05-29 08:58:55, Jia He wrote:
> The stress test's WARN_ON_ONCE(!sktp->tp) check in the child thread is
> racy and unnecessary: since kthread_run() wakes the thread before
> returning, the child can run before sktp[i].tp has been assigned.
> Moreover, sktp->tp is never actually used in the child function, so the
> check serves no purpose. Remove it and keep the original kthread_run()
> 
> Also add a common cleanup path for thread creation failures. If creating
> one of the later threads fails, stop all threads that were already
> started and free the allocated array instead of leaving orphan kthreads
> and leaked memory behind.
> 
> Finally, replace the module-static doneflag with kthread_should_stop().
> With the doneflag, child threads may exit before the parent calls
> kthread_stop(), so the task lifetime is no longer guaranteed when the
> parent later tries to stop them. Using kthread_should_stop() keeps each
> child alive until kthread_stop() synchronously terminates it.
> 
> Suggested-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Jia He <justin.he@arm.com>

JFYI, the patch has been committed into printk/linux.git,
branch for-7.3.

Best Regards,
Petr

PS: I have already sent pull request for 7.2. And this is
    not serious enough to trigger another one.