ring-buffer: Fix crash passing ERR_PTR to kthread_stop()

[PATCH] ring-buffer: Fix crash passing ERR_PTR to kthread_stop()

Posted by Hui Su 1 month, 3 weeks ago

In test_ringbuffer()'s out_free cleanup loop, the check
`!rb_threads[cpu]` only catches NULL entries and misses entries that
hold an ERR_PTR.

rb_threads[] is static, so unassigned slots are NULL. But when
kthread_run_on_cpu() fails for a cpu, it stores ERR_PTR(-ENOMEM) (or
-EINTR) in rb_threads[cpu] before the creation loop jumps to out_free.
That entry is non-NULL, so the old `!ptr` check does not break, and the
cleanup proceeds to call kthread_stop() on the ERR_PTR. kthread_stop()
then dereferences the bogus pointer, crashing the kernel during the
late_initcall self-test.

crash logs:
  BUG: kernel NULL pointer dereference, address: 000000000000001c
  Oops: 0002 [#1] SMP NOPTI
  CPU: 1 PID: 1 Comm: swapper/0 Not tainted 7.2.0-rc6-dirty #7 PREEMPT(lazy)
  RIP: 0010:kthread_stop+0x2e/0x220
  RBX: fffffffffffffff4
  CR2: 000000000000001c
  Call Trace:
   <TASK>
   test_ringbuffer+0x1ec/0x650
   do_one_initcall+0x6c/0x2c0
   kernel_init_freeable+0x21d/0x420
   kernel_init+0x15/0x1c0
   ret_from_fork+0x21b/0x320
   </TASK>
  Kernel panic - not syncing: Fatal exception

Fixes: 64ed3a049e3e ("ring-buffer: make use of the helper function kthread_run_on_cpu()")
Signed-off-by: Hui Su <sh_def@163.com>
---
 kernel/trace/ring_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 8e2485bb3aa8..6af7e36ca526 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -8214,7 +8214,7 @@ static __init int test_ringbuffer(void)
 
  out_free:
 	for_each_online_cpu(cpu) {
-		if (!rb_threads[cpu])
+		if (IS_ERR_OR_NULL(rb_threads[cpu]))
 			break;
 		kthread_stop(rb_threads[cpu]);
 	}
-- 
2.43.0

Re: [PATCH] ring-buffer: Fix crash passing ERR_PTR to kthread_stop()

Posted by Masami Hiramatsu (Google) 1 month, 3 weeks ago

On Fri,  7 Aug 2026 23:41:46 +0800
Hui Su <sh_def@163.com> wrote:

> In test_ringbuffer()'s out_free cleanup loop, the check
> `!rb_threads[cpu]` only catches NULL entries and misses entries that
> hold an ERR_PTR.
> 
> rb_threads[] is static, so unassigned slots are NULL. But when
> kthread_run_on_cpu() fails for a cpu, it stores ERR_PTR(-ENOMEM) (or
> -EINTR) in rb_threads[cpu] before the creation loop jumps to out_free.
> That entry is non-NULL, so the old `!ptr` check does not break, and the
> cleanup proceeds to call kthread_stop() on the ERR_PTR. kthread_stop()
> then dereferences the bogus pointer, crashing the kernel during the
> late_initcall self-test.
> 
> crash logs:
>   BUG: kernel NULL pointer dereference, address: 000000000000001c
>   Oops: 0002 [#1] SMP NOPTI
>   CPU: 1 PID: 1 Comm: swapper/0 Not tainted 7.2.0-rc6-dirty #7 PREEMPT(lazy)
>   RIP: 0010:kthread_stop+0x2e/0x220
>   RBX: fffffffffffffff4
>   CR2: 000000000000001c
>   Call Trace:
>    <TASK>
>    test_ringbuffer+0x1ec/0x650
>    do_one_initcall+0x6c/0x2c0
>    kernel_init_freeable+0x21d/0x420
>    kernel_init+0x15/0x1c0
>    ret_from_fork+0x21b/0x320
>    </TASK>
>   Kernel panic - not syncing: Fatal exception
> 
> Fixes: 64ed3a049e3e ("ring-buffer: make use of the helper function kthread_run_on_cpu()")
> Signed-off-by: Hui Su <sh_def@163.com>

Look good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you!

> ---
>  kernel/trace/ring_buffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 8e2485bb3aa8..6af7e36ca526 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -8214,7 +8214,7 @@ static __init int test_ringbuffer(void)
>  
>   out_free:
>  	for_each_online_cpu(cpu) {
> -		if (!rb_threads[cpu])
> +		if (IS_ERR_OR_NULL(rb_threads[cpu]))
>  			break;
>  		kthread_stop(rb_threads[cpu]);
>  	}
> -- 
> 2.43.0
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH] ring-buffer: Fix crash passing ERR_PTR to kthread_stop()

Posted by Vincent Donnefort 1 month, 3 weeks ago

On Fri, Aug 07, 2026 at 11:41:46PM +0800, Hui Su wrote:
> In test_ringbuffer()'s out_free cleanup loop, the check
> `!rb_threads[cpu]` only catches NULL entries and misses entries that
> hold an ERR_PTR.
> 
> rb_threads[] is static, so unassigned slots are NULL. But when
> kthread_run_on_cpu() fails for a cpu, it stores ERR_PTR(-ENOMEM) (or
> -EINTR) in rb_threads[cpu] before the creation loop jumps to out_free.
> That entry is non-NULL, so the old `!ptr` check does not break, and the
> cleanup proceeds to call kthread_stop() on the ERR_PTR. kthread_stop()
> then dereferences the bogus pointer, crashing the kernel during the
> late_initcall self-test.
> 
> crash logs:
>   BUG: kernel NULL pointer dereference, address: 000000000000001c
>   Oops: 0002 [#1] SMP NOPTI
>   CPU: 1 PID: 1 Comm: swapper/0 Not tainted 7.2.0-rc6-dirty #7 PREEMPT(lazy)
>   RIP: 0010:kthread_stop+0x2e/0x220
>   RBX: fffffffffffffff4
>   CR2: 000000000000001c
>   Call Trace:
>    <TASK>
>    test_ringbuffer+0x1ec/0x650
>    do_one_initcall+0x6c/0x2c0
>    kernel_init_freeable+0x21d/0x420
>    kernel_init+0x15/0x1c0
>    ret_from_fork+0x21b/0x320
>    </TASK>
>   Kernel panic - not syncing: Fatal exception
> 
> Fixes: 64ed3a049e3e ("ring-buffer: make use of the helper function kthread_run_on_cpu()")
> Signed-off-by: Hui Su <sh_def@163.com>
> ---
>  kernel/trace/ring_buffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 8e2485bb3aa8..6af7e36ca526 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -8214,7 +8214,7 @@ static __init int test_ringbuffer(void)
>  
>   out_free:
>  	for_each_online_cpu(cpu) {
> -		if (!rb_threads[cpu])
> +		if (IS_ERR_OR_NULL(rb_threads[cpu]))
>  			break;
>  		kthread_stop(rb_threads[cpu]);
>  	}
> -- 
> 2.43.0
> 
>

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>