2023-07-07 18:23:45

by Waiman Long

[permalink] [raw]
Subject: [PATCH v2] refscale: Fix use of uninitalized wait_queue_head_t

It was found that running the refscale test might crash the kernel once
in a while with the following error:

[ 8569.952896] BUG: unable to handle page fault for address: ffffffffffffffe8
[ 8569.952900] #PF: supervisor read access in kernel mode
[ 8569.952902] #PF: error_code(0x0000) - not-present page
[ 8569.952904] PGD c4b048067 P4D c4b049067 PUD c4b04b067 PMD 0
[ 8569.952910] Oops: 0000 [#1] PREEMPT_RT SMP NOPTI
[ 8569.952916] Hardware name: Dell Inc. PowerEdge R750/0WMWCR, BIOS 1.2.4 05/28/2021
[ 8569.952917] RIP: 0010:prepare_to_wait_event+0x101/0x190
:
[ 8569.952940] Call Trace:
[ 8569.952941] <TASK>
[ 8569.952944] ref_scale_reader+0x380/0x4a0 [refscale]
[ 8569.952959] kthread+0x10e/0x130
[ 8569.952966] ret_from_fork+0x1f/0x30
[ 8569.952973] </TASK>

This is likely caused by the fact that init_waitqueue_head() is
called after the ref_scale_reader kthread is created. The kthread
can potentially try to use the waitqueue head before it is properly
initialized. The crash happened at

static inline void __add_wait_queue(...)
{
:
if (!(wq->flags & WQ_FLAG_PRIORITY)) <=== Crash here

The offset of flags from list_head entry in wait_queue_entry is -0x18. If
reader_tasks[i].wq.head.next is NULL as allocated reader_task structure
is zero initialized, the instruction will try to access address
0xffffffffffffffe8 which is the fault address listed above.

Fix this by initializing the waitqueue head first before kthread
creation.

Fixes: 653ed64b01dc ("refperf: Add a test to measure performance of read-side synchronization")
Signed-off-by: Waiman Long <[email protected]>
Reviewed-by: Qiuxu Zhuo <[email protected]>
Reviewed-by: Davidlohr Bueso <[email protected]>
---
kernel/rcu/refscale.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
index 1970ce5f22d4..71d138573856 100644
--- a/kernel/rcu/refscale.c
+++ b/kernel/rcu/refscale.c
@@ -1107,12 +1107,11 @@ ref_scale_init(void)
VERBOSE_SCALEOUT("Starting %d reader threads", nreaders);

for (i = 0; i < nreaders; i++) {
+ init_waitqueue_head(&reader_tasks[i].wq);
firsterr = torture_create_kthread(ref_scale_reader, (void *)i,
reader_tasks[i].task);
if (torture_init_error(firsterr))
goto unwind;
-
- init_waitqueue_head(&(reader_tasks[i].wq));
}

// Main Task
--
2.31.1



2023-07-07 20:13:19

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH v2] refscale: Fix use of uninitalized wait_queue_head_t

On Fri, Jul 07, 2023 at 01:53:55PM -0400, Waiman Long wrote:
> It was found that running the refscale test might crash the kernel once
> in a while with the following error:
>
> [ 8569.952896] BUG: unable to handle page fault for address: ffffffffffffffe8
> [ 8569.952900] #PF: supervisor read access in kernel mode
> [ 8569.952902] #PF: error_code(0x0000) - not-present page
> [ 8569.952904] PGD c4b048067 P4D c4b049067 PUD c4b04b067 PMD 0
> [ 8569.952910] Oops: 0000 [#1] PREEMPT_RT SMP NOPTI
> [ 8569.952916] Hardware name: Dell Inc. PowerEdge R750/0WMWCR, BIOS 1.2.4 05/28/2021
> [ 8569.952917] RIP: 0010:prepare_to_wait_event+0x101/0x190
> :
> [ 8569.952940] Call Trace:
> [ 8569.952941] <TASK>
> [ 8569.952944] ref_scale_reader+0x380/0x4a0 [refscale]
> [ 8569.952959] kthread+0x10e/0x130
> [ 8569.952966] ret_from_fork+0x1f/0x30
> [ 8569.952973] </TASK>
>
> This is likely caused by the fact that init_waitqueue_head() is
> called after the ref_scale_reader kthread is created. The kthread
> can potentially try to use the waitqueue head before it is properly
> initialized. The crash happened at
>
> static inline void __add_wait_queue(...)
> {
> :
> if (!(wq->flags & WQ_FLAG_PRIORITY)) <=== Crash here
>
> The offset of flags from list_head entry in wait_queue_entry is -0x18. If
> reader_tasks[i].wq.head.next is NULL as allocated reader_task structure
> is zero initialized, the instruction will try to access address
> 0xffffffffffffffe8 which is the fault address listed above.
>
> Fix this by initializing the waitqueue head first before kthread
> creation.
>
> Fixes: 653ed64b01dc ("refperf: Add a test to measure performance of read-side synchronization")
> Signed-off-by: Waiman Long <[email protected]>
> Reviewed-by: Qiuxu Zhuo <[email protected]>
> Reviewed-by: Davidlohr Bueso <[email protected]>

Queued and pushed, thank you all!

As always, I could not resist wordsmithing the commit log, please see
below.

Thanx, Paul

------------------------------------------------------------------------

commit 933d3bf8f96d7cedf78081030e004d23aee2b56c
Author: Waiman Long <[email protected]>
Date: Fri Jul 7 13:53:55 2023 -0400

refscale: Fix uninitalized use of wait_queue_head_t

Running the refscale test occasionally crashes the kernel with the
following error:

[ 8569.952896] BUG: unable to handle page fault for address: ffffffffffffffe8
[ 8569.952900] #PF: supervisor read access in kernel mode
[ 8569.952902] #PF: error_code(0x0000) - not-present page
[ 8569.952904] PGD c4b048067 P4D c4b049067 PUD c4b04b067 PMD 0
[ 8569.952910] Oops: 0000 [#1] PREEMPT_RT SMP NOPTI
[ 8569.952916] Hardware name: Dell Inc. PowerEdge R750/0WMWCR, BIOS 1.2.4 05/28/2021
[ 8569.952917] RIP: 0010:prepare_to_wait_event+0x101/0x190
:
[ 8569.952940] Call Trace:
[ 8569.952941] <TASK>
[ 8569.952944] ref_scale_reader+0x380/0x4a0 [refscale]
[ 8569.952959] kthread+0x10e/0x130
[ 8569.952966] ret_from_fork+0x1f/0x30
[ 8569.952973] </TASK>

The likely cause is that init_waitqueue_head() is called after the call to
the torture_create_kthread() function that creates the ref_scale_reader
kthread. Although this init_waitqueue_head() call will very likely
complete before this kthread is created and starts running, it is
possible that the calling kthread will be delayed between the calls to
torture_create_kthread() and init_waitqueue_head(). In this case, the
new kthread will use the waitqueue head before it is properly initialized,
which is not good for the kernel's health and well-being.

The above crash happened here:

static inline void __add_wait_queue(...)
{
:
if (!(wq->flags & WQ_FLAG_PRIORITY)) <=== Crash here

The offset of flags from list_head entry in wait_queue_entry is
-0x18. If reader_tasks[i].wq.head.next is NULL as allocated reader_task
structure is zero initialized, the instruction will try to access address
0xffffffffffffffe8, which is exactly the fault address listed above.

This commit therefore invokes init_waitqueue_head() before creating
the kthread.

Fixes: 653ed64b01dc ("refperf: Add a test to measure performance of read-side synchronization")
Signed-off-by: Waiman Long <[email protected]>
Reviewed-by: Qiuxu Zhuo <[email protected]>
Reviewed-by: Davidlohr Bueso <[email protected]>
Acked-by: Joel Fernandes (Google) <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>

diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
index 1970ce5f22d4..71d138573856 100644
--- a/kernel/rcu/refscale.c
+++ b/kernel/rcu/refscale.c
@@ -1107,12 +1107,11 @@ ref_scale_init(void)
VERBOSE_SCALEOUT("Starting %d reader threads", nreaders);

for (i = 0; i < nreaders; i++) {
+ init_waitqueue_head(&reader_tasks[i].wq);
firsterr = torture_create_kthread(ref_scale_reader, (void *)i,
reader_tasks[i].task);
if (torture_init_error(firsterr))
goto unwind;
-
- init_waitqueue_head(&(reader_tasks[i].wq));
}

// Main Task

2023-07-07 20:33:07

by Waiman Long

[permalink] [raw]
Subject: Re: [PATCH v2] refscale: Fix use of uninitalized wait_queue_head_t

On 7/7/23 15:54, Paul E. McKenney wrote:
> On Fri, Jul 07, 2023 at 01:53:55PM -0400, Waiman Long wrote:
>> It was found that running the refscale test might crash the kernel once
>> in a while with the following error:
>>
>> [ 8569.952896] BUG: unable to handle page fault for address: ffffffffffffffe8
>> [ 8569.952900] #PF: supervisor read access in kernel mode
>> [ 8569.952902] #PF: error_code(0x0000) - not-present page
>> [ 8569.952904] PGD c4b048067 P4D c4b049067 PUD c4b04b067 PMD 0
>> [ 8569.952910] Oops: 0000 [#1] PREEMPT_RT SMP NOPTI
>> [ 8569.952916] Hardware name: Dell Inc. PowerEdge R750/0WMWCR, BIOS 1.2.4 05/28/2021
>> [ 8569.952917] RIP: 0010:prepare_to_wait_event+0x101/0x190
>> :
>> [ 8569.952940] Call Trace:
>> [ 8569.952941] <TASK>
>> [ 8569.952944] ref_scale_reader+0x380/0x4a0 [refscale]
>> [ 8569.952959] kthread+0x10e/0x130
>> [ 8569.952966] ret_from_fork+0x1f/0x30
>> [ 8569.952973] </TASK>
>>
>> This is likely caused by the fact that init_waitqueue_head() is
>> called after the ref_scale_reader kthread is created. The kthread
>> can potentially try to use the waitqueue head before it is properly
>> initialized. The crash happened at
>>
>> static inline void __add_wait_queue(...)
>> {
>> :
>> if (!(wq->flags & WQ_FLAG_PRIORITY)) <=== Crash here
>>
>> The offset of flags from list_head entry in wait_queue_entry is -0x18. If
>> reader_tasks[i].wq.head.next is NULL as allocated reader_task structure
>> is zero initialized, the instruction will try to access address
>> 0xffffffffffffffe8 which is the fault address listed above.
>>
>> Fix this by initializing the waitqueue head first before kthread
>> creation.
>>
>> Fixes: 653ed64b01dc ("refperf: Add a test to measure performance of read-side synchronization")
>> Signed-off-by: Waiman Long <[email protected]>
>> Reviewed-by: Qiuxu Zhuo <[email protected]>
>> Reviewed-by: Davidlohr Bueso <[email protected]>
> Queued and pushed, thank you all!
>
> As always, I could not resist wordsmithing the commit log, please see
> below.

Thanks for the wordsmithing. I don't mind at all.

Cheers,
Longman

> Thanx, Paul
>
> ------------------------------------------------------------------------
>
> commit 933d3bf8f96d7cedf78081030e004d23aee2b56c
> Author: Waiman Long <[email protected]>
> Date: Fri Jul 7 13:53:55 2023 -0400
>
> refscale: Fix uninitalized use of wait_queue_head_t
>
> Running the refscale test occasionally crashes the kernel with the
> following error:
>
> [ 8569.952896] BUG: unable to handle page fault for address: ffffffffffffffe8
> [ 8569.952900] #PF: supervisor read access in kernel mode
> [ 8569.952902] #PF: error_code(0x0000) - not-present page
> [ 8569.952904] PGD c4b048067 P4D c4b049067 PUD c4b04b067 PMD 0
> [ 8569.952910] Oops: 0000 [#1] PREEMPT_RT SMP NOPTI
> [ 8569.952916] Hardware name: Dell Inc. PowerEdge R750/0WMWCR, BIOS 1.2.4 05/28/2021
> [ 8569.952917] RIP: 0010:prepare_to_wait_event+0x101/0x190
> :
> [ 8569.952940] Call Trace:
> [ 8569.952941] <TASK>
> [ 8569.952944] ref_scale_reader+0x380/0x4a0 [refscale]
> [ 8569.952959] kthread+0x10e/0x130
> [ 8569.952966] ret_from_fork+0x1f/0x30
> [ 8569.952973] </TASK>
>
> The likely cause is that init_waitqueue_head() is called after the call to
> the torture_create_kthread() function that creates the ref_scale_reader
> kthread. Although this init_waitqueue_head() call will very likely
> complete before this kthread is created and starts running, it is
> possible that the calling kthread will be delayed between the calls to
> torture_create_kthread() and init_waitqueue_head(). In this case, the
> new kthread will use the waitqueue head before it is properly initialized,
> which is not good for the kernel's health and well-being.
>
> The above crash happened here:
>
> static inline void __add_wait_queue(...)
> {
> :
> if (!(wq->flags & WQ_FLAG_PRIORITY)) <=== Crash here
>
> The offset of flags from list_head entry in wait_queue_entry is
> -0x18. If reader_tasks[i].wq.head.next is NULL as allocated reader_task
> structure is zero initialized, the instruction will try to access address
> 0xffffffffffffffe8, which is exactly the fault address listed above.
>
> This commit therefore invokes init_waitqueue_head() before creating
> the kthread.
>
> Fixes: 653ed64b01dc ("refperf: Add a test to measure performance of read-side synchronization")
> Signed-off-by: Waiman Long <[email protected]>
> Reviewed-by: Qiuxu Zhuo <[email protected]>
> Reviewed-by: Davidlohr Bueso <[email protected]>
> Acked-by: Joel Fernandes (Google) <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
>
> diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
> index 1970ce5f22d4..71d138573856 100644
> --- a/kernel/rcu/refscale.c
> +++ b/kernel/rcu/refscale.c
> @@ -1107,12 +1107,11 @@ ref_scale_init(void)
> VERBOSE_SCALEOUT("Starting %d reader threads", nreaders);
>
> for (i = 0; i < nreaders; i++) {
> + init_waitqueue_head(&reader_tasks[i].wq);
> firsterr = torture_create_kthread(ref_scale_reader, (void *)i,
> reader_tasks[i].task);
> if (torture_init_error(firsterr))
> goto unwind;
> -
> - init_waitqueue_head(&(reader_tasks[i].wq));
> }
>
> // Main Task
>