[PATCHv2 1/1] lib/tests: test_ratelimit: fix stress test thread lifecycle and leak

[PATCHv2 1/1] lib/tests: test_ratelimit: fix stress test thread lifecycle and leak

[Jia He]

The stress test checks the kthread pointer in the child thread, but the
thread was created with kthread_run(). Since kthread_run() wakes the
thread before returning, the child can run before sktp[i].tp is assigned,
triggering the WARN_ON_ONCE() check on an arm64 N2 server with 128 cores.

Use kthread_create() instead, assign sktp[i].tp after successful
creation, and then wake the thread explicitly. This guarantees that the
child observes an initialized task pointer.

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.

Signed-off-by: Jia He <justin.he@arm.com>
---
v2: replace the module-static doneflag with kthread_should_stop() to avoid
UAF (sashiko-bot)
 lib/tests/test_ratelimit.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/lib/tests/test_ratelimit.c b/lib/tests/test_ratelimit.c
index 33cea5f3d28b..d95c49e914c7 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 {
@@ -86,7 +85,7 @@ static int test_ratelimit_stress_child(void *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 +104,42 @@ 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");
+		struct task_struct *tp;
+
+		tp = kthread_create(test_ratelimit_stress_child, &sktp[i],
+				    "%s/%i", "test_ratelimit_stress_child", i);
+		if (IS_ERR(tp)) {
+			KUNIT_FAIL(test, "kthread_create failed: %ld", PTR_ERR(tp));
+			goto out_stop;
+		}
+
+		sktp[i].tp = tp;
+		wake_up_process(tp);
+		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: [PATCHv2 1/1] lib/tests: test_ratelimit: fix stress test thread lifecycle and leak

[Petr Mladek]

On Tue 2026-05-19 09:37:45, Jia He wrote:
> The stress test checks the kthread pointer in the child thread, but the
> thread was created with kthread_run(). Since kthread_run() wakes the
> thread before returning, the child can run before sktp[i].tp is assigned,
> triggering the WARN_ON_ONCE() check on an arm64 N2 server with 128 cores.

True.

> Use kthread_create() instead, assign sktp[i].tp after successful
> creation, and then wake the thread explicitly. This guarantees that the
> child observes an initialized task pointer.

IMHO, this does not guarantee that the task pointer has been assigned.
AFAIK, there might be spurious wakeups, for example, by the
suspend/resume or livepatching code.

I have another idea, see below.

> 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.

This part looks good to me. 

> Signed-off-by: Jia He <justin.he@arm.com>
> ---
> v2: replace the module-static doneflag with kthread_should_stop() to avoid
> UAF (sashiko-bot)
>  lib/tests/test_ratelimit.c | 37 ++++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/tests/test_ratelimit.c b/lib/tests/test_ratelimit.c
> index 33cea5f3d28b..d95c49e914c7 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 {
> @@ -86,7 +85,7 @@ static int test_ratelimit_stress_child(void *arg)
>  	set_user_nice(current, MAX_NICE);
>  	WARN_ON_ONCE(!sktp->tp);

It seem that "sktp->tp" is not really used in this function.
I am not sure why the WARN_ON_ONCE() is there.

It would simply remove this check and keep the original kthread_run().

>  
> -	while (!READ_ONCE(doneflag)) {
> +	while (!kthread_should_stop()) {
>  		sktp->nattempts++;
>  		if (___ratelimit(&stressrl, __func__))
>  			sktp->nunlimited++;

Best Regards,
Petr

RE: [PATCHv2 1/1] lib/tests: test_ratelimit: fix stress test thread lifecycle and leak

[Justin He]

> -----Original Message-----
> From: Petr Mladek <pmladek@suse.com>
> Sent: Thursday, May 21, 2026 8:51 PM
> To: Justin He <Justin.He@arm.com>
> Cc: Paul E. McKenney <paulmck@kernel.org>; Kees Cook <kees@kernel.org>;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCHv2 1/1] lib/tests: test_ratelimit: fix stress test thread
> lifecycle and leak
> 
> On Tue 2026-05-19 09:37:45, Jia He wrote:
> > The stress test checks the kthread pointer in the child thread, but
> > the thread was created with kthread_run(). Since kthread_run() wakes
> > the thread before returning, the child can run before sktp[i].tp is
> > assigned, triggering the WARN_ON_ONCE() check on an arm64 N2 server
> with 128 cores.
> 
> True.
> 
> > Use kthread_create() instead, assign sktp[i].tp after successful
> > creation, and then wake the thread explicitly. This guarantees that
> > the child observes an initialized task pointer.
> 
> IMHO, this does not guarantee that the task pointer has been assigned.
> AFAIK, there might be spurious wakeups, for example, by the suspend/resume
> or livepatching code.
> 
> I have another idea, see below.
> 
> > 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.
> 
> This part looks good to me.
> 
> > Signed-off-by: Jia He <justin.he@arm.com>
> > ---
> > v2: replace the module-static doneflag with kthread_should_stop() to
> > avoid UAF (sashiko-bot)  lib/tests/test_ratelimit.c | 37
> > ++++++++++++++++++++++++++-----------
> >  1 file changed, 26 insertions(+), 11 deletions(-)
> >
> > diff --git a/lib/tests/test_ratelimit.c b/lib/tests/test_ratelimit.c
> > index 33cea5f3d28b..d95c49e914c7 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 {
> > @@ -86,7 +85,7 @@ static int test_ratelimit_stress_child(void *arg)
> >  	set_user_nice(current, MAX_NICE);
> >  	WARN_ON_ONCE(!sktp->tp);
> 
> It seem that "sktp->tp" is not really used in this function.
> I am not sure why the WARN_ON_ONCE() is there.
> 
> It would simply remove this check and keep the original kthread_run().
> 
> >
> > -	while (!READ_ONCE(doneflag)) {
> > +	while (!kthread_should_stop()) {
> >  		sktp->nattempts++;
> >  		if (___ratelimit(&stressrl, __func__))
> >  			sktp->nunlimited++;
> 

Thanks Petr, makes sense.

Since the child does not really use sktp->tp, I’ll just remove the
WARN_ON_ONCE() and keep kthread_run().

I’ll still keep the kthread_should_stop() conversion and the cleanup path
for partial creation failures.


--- 
Cheers,
Justin He(Jia He)