[PATCH] Fixing warning of directly dereferencing __rcu tagged

[PATCH] Fixing warning of directly dereferencing __rcu tagged

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should 
always be avoided according to the docs. There is a rcu helper 
functions rcu_dereference(...) to use when dereferencing a __rcu 
pointer. This functions returns the non __rcu tagged pointer. 
Like normal pointer there should be a check for null case when 
further dereferencing the returned dereferenced __rcu pointer.


Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
 kernel/fork.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..5afb1b389a66 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,9 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		const struct cred *real_cred = rcu_dereference(p->real_cred);
+
+		if (real_cred && real_cred->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
-- 
2.39.2

[PATCH] Fixing warning of directly dereferencing __rcu tagged

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should 
always be avoided according to the docs. There is a rcu helper 
functions rcu_dereference(...) to use when dereferencing a __rcu 
pointer. This functions returns the non __rcu tagged pointer. 
Like normal pointer there should be a check for null case when 
further dereferencing the returned dereferenced __rcu pointer.


Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
 kernel/fork.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..5afb1b389a66 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,9 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		const struct cred *real_cred = rcu_dereference(p->real_cred);
+
+		if (real_cred && real_cred->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
-- 
2.39.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH] Fixing warning of directly dereferencing __rcu tagged

[Andrew Morton]

On Thu, 26 Oct 2023 03:58:11 +0530 Abhinav Singh <singhabhinav9051571833@gmail.com> wrote:

> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should 
> always be avoided according to the docs. There is a rcu helper 
> functions rcu_dereference(...) to use when dereferencing a __rcu 
> pointer. This functions returns the non __rcu tagged pointer. 

Seems sensible.

> Like normal pointer there should be a check for null case when 
> further dereferencing the returned dereferenced __rcu pointer.

Why is this?

> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,9 @@ __latent_entropy struct task_struct *copy_process(
>  
>  	retval = -EAGAIN;
>  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		const struct cred *real_cred = rcu_dereference(p->real_cred);
> +
> +		if (real_cred && real_cred->user != INIT_USER &&
>  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>  			goto bad_fork_cleanup_count;

The old code assumes that p->read_cred cannot be NULL and the new code
does nothing to make it possible that `real_cred' can be NULL?

In other words, I see no reason to add this new check for NULL?

Re: [PATCH] Fixing warning of directly dereferencing __rcu tagged

[Andrew Morton]

On Thu, 26 Oct 2023 03:58:11 +0530 Abhinav Singh <singhabhinav9051571833@gmail.com> wrote:

> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should 
> always be avoided according to the docs. There is a rcu helper 
> functions rcu_dereference(...) to use when dereferencing a __rcu 
> pointer. This functions returns the non __rcu tagged pointer. 

Seems sensible.

> Like normal pointer there should be a check for null case when 
> further dereferencing the returned dereferenced __rcu pointer.

Why is this?

> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,9 @@ __latent_entropy struct task_struct *copy_process(
>  
>  	retval = -EAGAIN;
>  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		const struct cred *real_cred = rcu_dereference(p->real_cred);
> +
> +		if (real_cred && real_cred->user != INIT_USER &&
>  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>  			goto bad_fork_cleanup_count;

The old code assumes that p->read_cred cannot be NULL and the new code
does nothing to make it possible that `real_cred' can be NULL?

In other words, I see no reason to add this new check for NULL?
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH] Fixing warning of directly dereferencing __rcu tagged

[Abhinav Singh]

On 10/26/23 04:08, Andrew Morton wrote:
> On Thu, 26 Oct 2023 03:58:11 +0530 Abhinav Singh <singhabhinav9051571833@gmail.com> wrote:
> 
>> This patch fixes the warning about directly dereferencing a pointer
>> tagged with __rcu annotation.
>>
>> Dereferencing the pointers tagged with __rcu directly should
>> always be avoided according to the docs. There is a rcu helper
>> functions rcu_dereference(...) to use when dereferencing a __rcu
>> pointer. This functions returns the non __rcu tagged pointer.
> 
> Seems sensible.
> 
>> Like normal pointer there should be a check for null case when
>> further dereferencing the returned dereferenced __rcu pointer.
> 
> Why is this?
> 
>> --- a/kernel/fork.c
>> +++ b/kernel/fork.c
>> @@ -2369,7 +2369,9 @@ __latent_entropy struct task_struct *copy_process(
>>   
>>   	retval = -EAGAIN;
>>   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
>> -		if (p->real_cred->user != INIT_USER &&
>> +		const struct cred *real_cred = rcu_dereference(p->real_cred);
>> +
>> +		if (real_cred && real_cred->user != INIT_USER &&
>>   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>>   			goto bad_fork_cleanup_count;
> 
> The old code assumes that p->read_cred cannot be NULL and the new code
> does nothing to make it possible that `real_cred' can be NULL?
> 
> In other words, I see no reason to add this new check for NULL?

Thank you for the response!

I thought it will be better to have check before accessing it, just so 
we dont have any segmentation fault in future.

Also I just noticed there are two more places where direct dereferencing 
of __rcu pointer is done in this same file. Should I do those changes in 
this patch ?

Re: [PATCH] Fixing warning of directly dereferencing __rcu tagged

[Abhinav Singh]

On 10/26/23 04:08, Andrew Morton wrote:
> On Thu, 26 Oct 2023 03:58:11 +0530 Abhinav Singh <singhabhinav9051571833@gmail.com> wrote:
> 
>> This patch fixes the warning about directly dereferencing a pointer
>> tagged with __rcu annotation.
>>
>> Dereferencing the pointers tagged with __rcu directly should
>> always be avoided according to the docs. There is a rcu helper
>> functions rcu_dereference(...) to use when dereferencing a __rcu
>> pointer. This functions returns the non __rcu tagged pointer.
> 
> Seems sensible.
> 
>> Like normal pointer there should be a check for null case when
>> further dereferencing the returned dereferenced __rcu pointer.
> 
> Why is this?
> 
>> --- a/kernel/fork.c
>> +++ b/kernel/fork.c
>> @@ -2369,7 +2369,9 @@ __latent_entropy struct task_struct *copy_process(
>>   
>>   	retval = -EAGAIN;
>>   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
>> -		if (p->real_cred->user != INIT_USER &&
>> +		const struct cred *real_cred = rcu_dereference(p->real_cred);
>> +
>> +		if (real_cred && real_cred->user != INIT_USER &&
>>   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>>   			goto bad_fork_cleanup_count;
> 
> The old code assumes that p->read_cred cannot be NULL and the new code
> does nothing to make it possible that `real_cred' can be NULL?
> 
> In other words, I see no reason to add this new check for NULL?

Thank you for the response!

I thought it will be better to have check before accessing it, just so 
we dont have any segmentation fault in future.

Also I just noticed there are two more places where direct dereferencing 
of __rcu pointer is done in this same file. Should I do those changes in 
this patch ?

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH] Fixing warning of directly dereferencing __rcu tagged

[Andrew Morton]

On Thu, 26 Oct 2023 04:57:42 +0530 Abhinav Singh <singhabhinav9051571833@gmail.com> wrote:

> On 10/26/23 04:08, Andrew Morton wrote:
> >> +++ b/kernel/fork.c
> >> @@ -2369,7 +2369,9 @@ __latent_entropy struct task_struct *copy_process(
> >>   
> >>   	retval = -EAGAIN;
> >>   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> >> -		if (p->real_cred->user != INIT_USER &&
> >> +		const struct cred *real_cred = rcu_dereference(p->real_cred);
> >> +
> >> +		if (real_cred && real_cred->user != INIT_USER &&
> >>   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
> >>   			goto bad_fork_cleanup_count;
> > 
> > The old code assumes that p->read_cred cannot be NULL and the new code
> > does nothing to make it possible that `real_cred' can be NULL?
> > 
> > In other words, I see no reason to add this new check for NULL?
> 
> Thank you for the response!
> 
> I thought it will be better to have check before accessing it, just so 
> we dont have any segmentation fault in future.

That would be adding code which has no effect?

> Also I just noticed there are two more places where direct dereferencing 
> of __rcu pointer is done in this same file. Should I do those changes in 
> this patch ?

I don't see why.  rcu_dereference(p) cannot return NULL if `p' is non-NULL?
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH] Fixing warning of directly dereferencing __rcu tagged

[Andrew Morton]

On Thu, 26 Oct 2023 04:57:42 +0530 Abhinav Singh <singhabhinav9051571833@gmail.com> wrote:

> On 10/26/23 04:08, Andrew Morton wrote:
> >> +++ b/kernel/fork.c
> >> @@ -2369,7 +2369,9 @@ __latent_entropy struct task_struct *copy_process(
> >>   
> >>   	retval = -EAGAIN;
> >>   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> >> -		if (p->real_cred->user != INIT_USER &&
> >> +		const struct cred *real_cred = rcu_dereference(p->real_cred);
> >> +
> >> +		if (real_cred && real_cred->user != INIT_USER &&
> >>   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
> >>   			goto bad_fork_cleanup_count;
> > 
> > The old code assumes that p->read_cred cannot be NULL and the new code
> > does nothing to make it possible that `real_cred' can be NULL?
> > 
> > In other words, I see no reason to add this new check for NULL?
> 
> Thank you for the response!
> 
> I thought it will be better to have check before accessing it, just so 
> we dont have any segmentation fault in future.

That would be adding code which has no effect?

> Also I just noticed there are two more places where direct dereferencing 
> of __rcu pointer is done in this same file. Should I do those changes in 
> this patch ?

I don't see why.  rcu_dereference(p) cannot return NULL if `p' is non-NULL?

[PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should
always be avoided according to the docs. There is a rcu helper
functions rcu_dereference(...) to use when dereferencing a __rcu
pointer. This functions returns the non __rcu tagged pointer which
can be dereferenced just like a normal pointers.

Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
 kernel/fork.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..802b7bbe3d92 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
@@ -2692,7 +2692,7 @@ __latent_entropy struct task_struct *copy_process(
 			 */
 			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
 							 p->real_parent->signal->is_child_subreaper;
-			list_add_tail(&p->sibling, &p->real_parent->children);
+			list_add_tail(&p->sibling, &(rcu_dereference(p->real_parent)->children));
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			attach_pid(p, PIDTYPE_TGID);
 			attach_pid(p, PIDTYPE_PGID);
-- 
2.39.2

[PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should
always be avoided according to the docs. There is a rcu helper
functions rcu_dereference(...) to use when dereferencing a __rcu
pointer. This functions returns the non __rcu tagged pointer which
can be dereferenced just like a normal pointers.

Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
 kernel/fork.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..802b7bbe3d92 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
@@ -2692,7 +2692,7 @@ __latent_entropy struct task_struct *copy_process(
 			 */
 			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
 							 p->real_parent->signal->is_child_subreaper;
-			list_add_tail(&p->sibling, &p->real_parent->children);
+			list_add_tail(&p->sibling, &(rcu_dereference(p->real_parent)->children));
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			attach_pid(p, PIDTYPE_TGID);
 			attach_pid(p, PIDTYPE_PGID);
-- 
2.39.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> functions rcu_dereference(...) to use when dereferencing a __rcu
> pointer. This functions returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointers.
> 
> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>

Well yes but these need to be called under rcu_read_lock.
Who does it here?
If no one then maybe you found an actual bug and we need to
fix it not paper over it.


> ---
>  kernel/fork.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..802b7bbe3d92 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>  
>  	retval = -EAGAIN;
>  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>  			goto bad_fork_cleanup_count;
>  	}
> @@ -2692,7 +2692,7 @@ __latent_entropy struct task_struct *copy_process(
>  			 */
>  			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
>  							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			list_add_tail(&p->sibling, &(rcu_dereference(p->real_parent)->children));
>  			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>  			attach_pid(p, PIDTYPE_TGID);
>  			attach_pid(p, PIDTYPE_PGID);
> -- 
> 2.39.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> functions rcu_dereference(...) to use when dereferencing a __rcu
> pointer. This functions returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointers.
> 
> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>

Well yes but these need to be called under rcu_read_lock.
Who does it here?
If no one then maybe you found an actual bug and we need to
fix it not paper over it.


> ---
>  kernel/fork.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..802b7bbe3d92 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>  
>  	retval = -EAGAIN;
>  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>  			goto bad_fork_cleanup_count;
>  	}
> @@ -2692,7 +2692,7 @@ __latent_entropy struct task_struct *copy_process(
>  			 */
>  			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
>  							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			list_add_tail(&p->sibling, &(rcu_dereference(p->real_parent)->children));
>  			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>  			attach_pid(p, PIDTYPE_TGID);
>  			attach_pid(p, PIDTYPE_PGID);
> -- 
> 2.39.2

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Mateusz Guzik]

On 10/26/23, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
>> This patch fixes the warning about directly dereferencing a pointer
>> tagged with __rcu annotation.
>>
>> Dereferencing the pointers tagged with __rcu directly should
>> always be avoided according to the docs. There is a rcu helper
>> functions rcu_dereference(...) to use when dereferencing a __rcu
>> pointer. This functions returns the non __rcu tagged pointer which
>> can be dereferenced just like a normal pointers.
>>
>> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
>
> Well yes but these need to be called under rcu_read_lock.
> Who does it here?
> If no one then maybe you found an actual bug and we need to
> fix it not paper over it.
>

There is no bug here.

p is the newly created thread, ->real_cred was initialized just prior
to this code and there is nobody to whack the creds from under it.

Second bit in the patch changes one real_parent deref, but leaves 2
others just above it. Once more no bug since the entire thing happens
under tasklist_lock, but the patch should either sort all these cases
or none.

I think it would help if the submitter had shown warnings they see.

-- 
Mateusz Guzik <mjguzik gmail.com>

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Mateusz Guzik]

On 10/26/23, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
>> This patch fixes the warning about directly dereferencing a pointer
>> tagged with __rcu annotation.
>>
>> Dereferencing the pointers tagged with __rcu directly should
>> always be avoided according to the docs. There is a rcu helper
>> functions rcu_dereference(...) to use when dereferencing a __rcu
>> pointer. This functions returns the non __rcu tagged pointer which
>> can be dereferenced just like a normal pointers.
>>
>> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
>
> Well yes but these need to be called under rcu_read_lock.
> Who does it here?
> If no one then maybe you found an actual bug and we need to
> fix it not paper over it.
>

There is no bug here.

p is the newly created thread, ->real_cred was initialized just prior
to this code and there is nobody to whack the creds from under it.

Second bit in the patch changes one real_parent deref, but leaves 2
others just above it. Once more no bug since the entire thing happens
under tasklist_lock, but the patch should either sort all these cases
or none.

I think it would help if the submitter had shown warnings they see.

-- 
Mateusz Guzik <mjguzik gmail.com>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Mathieu Desnoyers]

On 2023-10-26 10:06, Mateusz Guzik wrote:
> On 10/26/23, Michael S. Tsirkin <mst@redhat.com> wrote:
>> On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
>>> This patch fixes the warning about directly dereferencing a pointer
>>> tagged with __rcu annotation.
>>>
>>> Dereferencing the pointers tagged with __rcu directly should
>>> always be avoided according to the docs. There is a rcu helper
>>> functions rcu_dereference(...) to use when dereferencing a __rcu
>>> pointer. This functions returns the non __rcu tagged pointer which
>>> can be dereferenced just like a normal pointers.
>>>
>>> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
>>
>> Well yes but these need to be called under rcu_read_lock.
>> Who does it here?
>> If no one then maybe you found an actual bug and we need to
>> fix it not paper over it.
>>
> 
> There is no bug here.
> 
> p is the newly created thread, ->real_cred was initialized just prior
> to this code and there is nobody to whack the creds from under it.
> 
> Second bit in the patch changes one real_parent deref, but leaves 2
> others just above it. Once more no bug since the entire thing happens
> under tasklist_lock, but the patch should either sort all these cases
> or none.

Drive-by comment: perhaps use rcu_dereference_protected() ?

Thanks,

Mathieu

> 
> I think it would help if the submitter had shown warnings they see.
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Mathieu Desnoyers]

On 2023-10-26 10:06, Mateusz Guzik wrote:
> On 10/26/23, Michael S. Tsirkin <mst@redhat.com> wrote:
>> On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
>>> This patch fixes the warning about directly dereferencing a pointer
>>> tagged with __rcu annotation.
>>>
>>> Dereferencing the pointers tagged with __rcu directly should
>>> always be avoided according to the docs. There is a rcu helper
>>> functions rcu_dereference(...) to use when dereferencing a __rcu
>>> pointer. This functions returns the non __rcu tagged pointer which
>>> can be dereferenced just like a normal pointers.
>>>
>>> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
>>
>> Well yes but these need to be called under rcu_read_lock.
>> Who does it here?
>> If no one then maybe you found an actual bug and we need to
>> fix it not paper over it.
>>
> 
> There is no bug here.
> 
> p is the newly created thread, ->real_cred was initialized just prior
> to this code and there is nobody to whack the creds from under it.
> 
> Second bit in the patch changes one real_parent deref, but leaves 2
> others just above it. Once more no bug since the entire thing happens
> under tasklist_lock, but the patch should either sort all these cases
> or none.

Drive-by comment: perhaps use rcu_dereference_protected() ?

Thanks,

Mathieu

> 
> I think it would help if the submitter had shown warnings they see.
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Mateusz Guzik]

On 10/26/23, Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> Drive-by comment: perhaps use rcu_dereference_protected() ?
>

Definitely.

But as I mentioned even after applying the patch there are uses which
should have been reported (and consequently sorted out). If one is to
bother with any of this at least the entire file should be covered.

-- 
Mateusz Guzik <mjguzik gmail.com>

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Mateusz Guzik]

On 10/26/23, Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> Drive-by comment: perhaps use rcu_dereference_protected() ?
>

Definitely.

But as I mentioned even after applying the patch there are uses which
should have been reported (and consequently sorted out). If one is to
bother with any of this at least the entire file should be covered.

-- 
Mateusz Guzik <mjguzik gmail.com>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 04:06:24PM +0200, Mateusz Guzik wrote:
> On 10/26/23, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
> >> This patch fixes the warning about directly dereferencing a pointer
> >> tagged with __rcu annotation.
> >>
> >> Dereferencing the pointers tagged with __rcu directly should
> >> always be avoided according to the docs. There is a rcu helper
> >> functions rcu_dereference(...) to use when dereferencing a __rcu
> >> pointer. This functions returns the non __rcu tagged pointer which
> >> can be dereferenced just like a normal pointers.
> >>
> >> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> >
> > Well yes but these need to be called under rcu_read_lock.
> > Who does it here?
> > If no one then maybe you found an actual bug and we need to
> > fix it not paper over it.
> >
> 
> There is no bug here.
> 
> p is the newly created thread, ->real_cred was initialized just prior
> to this code and there is nobody to whack the creds from under it.
> 
> Second bit in the patch changes one real_parent deref, but leaves 2
> others just above it. Once more no bug since the entire thing happens
> under tasklist_lock, but the patch should either sort all these cases
> or none.
> 
> I think it would help if the submitter had shown warnings they see.

Yes, and this must be tested under lockdep, which I think would
spit out warnings for this patch.

What should be used here I'm not sure. IIUC rcu_dereference_protected(p, 1)
is discouraged now?

-- 
MST

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 04:06:24PM +0200, Mateusz Guzik wrote:
> On 10/26/23, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
> >> This patch fixes the warning about directly dereferencing a pointer
> >> tagged with __rcu annotation.
> >>
> >> Dereferencing the pointers tagged with __rcu directly should
> >> always be avoided according to the docs. There is a rcu helper
> >> functions rcu_dereference(...) to use when dereferencing a __rcu
> >> pointer. This functions returns the non __rcu tagged pointer which
> >> can be dereferenced just like a normal pointers.
> >>
> >> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> >
> > Well yes but these need to be called under rcu_read_lock.
> > Who does it here?
> > If no one then maybe you found an actual bug and we need to
> > fix it not paper over it.
> >
> 
> There is no bug here.
> 
> p is the newly created thread, ->real_cred was initialized just prior
> to this code and there is nobody to whack the creds from under it.
> 
> Second bit in the patch changes one real_parent deref, but leaves 2
> others just above it. Once more no bug since the entire thing happens
> under tasklist_lock, but the patch should either sort all these cases
> or none.
> 
> I think it would help if the submitter had shown warnings they see.

Yes, and this must be tested under lockdep, which I think would
spit out warnings for this patch.

What should be used here I'm not sure. IIUC rcu_dereference_protected(p, 1)
is discouraged now?

-- 
MST

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

On 10/26/23 20:47, Michael S. Tsirkin wrote:
> On Thu, Oct 26, 2023 at 04:06:24PM +0200, Mateusz Guzik wrote:
>> On 10/26/23, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
>>>> This patch fixes the warning about directly dereferencing a pointer
>>>> tagged with __rcu annotation.
>>>>
>>>> Dereferencing the pointers tagged with __rcu directly should
>>>> always be avoided according to the docs. There is a rcu helper
>>>> functions rcu_dereference(...) to use when dereferencing a __rcu
>>>> pointer. This functions returns the non __rcu tagged pointer which
>>>> can be dereferenced just like a normal pointers.
>>>>
>>>> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
>>>
>>> Well yes but these need to be called under rcu_read_lock.
>>> Who does it here?
>>> If no one then maybe you found an actual bug and we need to
>>> fix it not paper over it.
>>>
>>
>> There is no bug here.
>>
>> p is the newly created thread, ->real_cred was initialized just prior
>> to this code and there is nobody to whack the creds from under it.
>>
>> Second bit in the patch changes one real_parent deref, but leaves 2
>> others just above it. Once more no bug since the entire thing happens
>> under tasklist_lock, but the patch should either sort all these cases
>> or none.
Sparse reported 3 similar dereferencing warning this patch contains 2 
fixes for 2, but yeah I should fixed all 3 of them.
>>
>> I think it would help if the submitter had shown warnings they see.
The warning message :- warning: dereference of noderef expression
> 
> Yes, and this must be tested under lockdep, which I think would
> spit out warnings for this patch.
Not sure, but I tested this with sparse (make C=2) and after the above 
changes I dont get the warning.
> 
> What should be used here I'm not sure. IIUC rcu_dereference_protected(p, 1)
> is discouraged now?
> 
Not sure but I read that, rcu_dereference should be prefered when 
reading and rcu_dereference_protected should when writing.

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

On 10/26/23 20:47, Michael S. Tsirkin wrote:
> On Thu, Oct 26, 2023 at 04:06:24PM +0200, Mateusz Guzik wrote:
>> On 10/26/23, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
>>>> This patch fixes the warning about directly dereferencing a pointer
>>>> tagged with __rcu annotation.
>>>>
>>>> Dereferencing the pointers tagged with __rcu directly should
>>>> always be avoided according to the docs. There is a rcu helper
>>>> functions rcu_dereference(...) to use when dereferencing a __rcu
>>>> pointer. This functions returns the non __rcu tagged pointer which
>>>> can be dereferenced just like a normal pointers.
>>>>
>>>> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
>>>
>>> Well yes but these need to be called under rcu_read_lock.
>>> Who does it here?
>>> If no one then maybe you found an actual bug and we need to
>>> fix it not paper over it.
>>>
>>
>> There is no bug here.
>>
>> p is the newly created thread, ->real_cred was initialized just prior
>> to this code and there is nobody to whack the creds from under it.
>>
>> Second bit in the patch changes one real_parent deref, but leaves 2
>> others just above it. Once more no bug since the entire thing happens
>> under tasklist_lock, but the patch should either sort all these cases
>> or none.
Sparse reported 3 similar dereferencing warning this patch contains 2 
fixes for 2, but yeah I should fixed all 3 of them.
>>
>> I think it would help if the submitter had shown warnings they see.
The warning message :- warning: dereference of noderef expression
> 
> Yes, and this must be tested under lockdep, which I think would
> spit out warnings for this patch.
Not sure, but I tested this with sparse (make C=2) and after the above 
changes I dont get the warning.
> 
> What should be used here I'm not sure. IIUC rcu_dereference_protected(p, 1)
> is discouraged now?
> 
Not sure but I read that, rcu_dereference should be prefered when 
reading and rcu_dereference_protected should when writing.

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 09:07:46PM +0530, Abhinav Singh wrote:
> On 10/26/23 20:47, Michael S. Tsirkin wrote:
> > On Thu, Oct 26, 2023 at 04:06:24PM +0200, Mateusz Guzik wrote:
> > > On 10/26/23, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
> > > > > This patch fixes the warning about directly dereferencing a pointer
> > > > > tagged with __rcu annotation.
> > > > > 
> > > > > Dereferencing the pointers tagged with __rcu directly should
> > > > > always be avoided according to the docs. There is a rcu helper
> > > > > functions rcu_dereference(...) to use when dereferencing a __rcu
> > > > > pointer. This functions returns the non __rcu tagged pointer which
> > > > > can be dereferenced just like a normal pointers.
> > > > > 
> > > > > Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> > > > 
> > > > Well yes but these need to be called under rcu_read_lock.
> > > > Who does it here?
> > > > If no one then maybe you found an actual bug and we need to
> > > > fix it not paper over it.
> > > > 
> > > 
> > > There is no bug here.
> > > 
> > > p is the newly created thread, ->real_cred was initialized just prior
> > > to this code and there is nobody to whack the creds from under it.
> > > 
> > > Second bit in the patch changes one real_parent deref, but leaves 2
> > > others just above it. Once more no bug since the entire thing happens
> > > under tasklist_lock, but the patch should either sort all these cases
> > > or none.
> Sparse reported 3 similar dereferencing warning this patch contains 2 fixes
> for 2, but yeah I should fixed all 3 of them.
> > > 
> > > I think it would help if the submitter had shown warnings they see.
> The warning message :- warning: dereference of noderef expression
> > 
> > Yes, and this must be tested under lockdep, which I think would
> > spit out warnings for this patch.
> Not sure, but I tested this with sparse (make C=2) and after the above
> changes I dont get the warning.

sparse is a static analysis tool. You should also actually
test your patch.

> > 
> > What should be used here I'm not sure. IIUC rcu_dereference_protected(p, 1)
> > is discouraged now?
> > 
> Not sure but I read that, rcu_dereference should be prefered when reading
> and rcu_dereference_protected should when writing.

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 09:07:46PM +0530, Abhinav Singh wrote:
> On 10/26/23 20:47, Michael S. Tsirkin wrote:
> > On Thu, Oct 26, 2023 at 04:06:24PM +0200, Mateusz Guzik wrote:
> > > On 10/26/23, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > On Thu, Oct 26, 2023 at 05:46:21PM +0530, Abhinav Singh wrote:
> > > > > This patch fixes the warning about directly dereferencing a pointer
> > > > > tagged with __rcu annotation.
> > > > > 
> > > > > Dereferencing the pointers tagged with __rcu directly should
> > > > > always be avoided according to the docs. There is a rcu helper
> > > > > functions rcu_dereference(...) to use when dereferencing a __rcu
> > > > > pointer. This functions returns the non __rcu tagged pointer which
> > > > > can be dereferenced just like a normal pointers.
> > > > > 
> > > > > Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> > > > 
> > > > Well yes but these need to be called under rcu_read_lock.
> > > > Who does it here?
> > > > If no one then maybe you found an actual bug and we need to
> > > > fix it not paper over it.
> > > > 
> > > 
> > > There is no bug here.
> > > 
> > > p is the newly created thread, ->real_cred was initialized just prior
> > > to this code and there is nobody to whack the creds from under it.
> > > 
> > > Second bit in the patch changes one real_parent deref, but leaves 2
> > > others just above it. Once more no bug since the entire thing happens
> > > under tasklist_lock, but the patch should either sort all these cases
> > > or none.
> Sparse reported 3 similar dereferencing warning this patch contains 2 fixes
> for 2, but yeah I should fixed all 3 of them.
> > > 
> > > I think it would help if the submitter had shown warnings they see.
> The warning message :- warning: dereference of noderef expression
> > 
> > Yes, and this must be tested under lockdep, which I think would
> > spit out warnings for this patch.
> Not sure, but I tested this with sparse (make C=2) and after the above
> changes I dont get the warning.

sparse is a static analysis tool. You should also actually
test your patch.

> > 
> > What should be used here I'm not sure. IIUC rcu_dereference_protected(p, 1)
> > is discouraged now?
> > 
> Not sure but I read that, rcu_dereference should be prefered when reading
> and rcu_dereference_protected should when writing.

[PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should
always be avoided according to the docs. There is a rcu helper
functions rcu_dereference(...) to use when dereferencing a __rcu
pointer. This functions returns the non __rcu tagged pointer which
can be dereferenced just like a normal pointers.


Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
 kernel/fork.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..e78649974669 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
@@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
 			 * tasklist_lock with adding child to the process tree
 			 * for propagate_has_child_subreaper optimization.
 			 */
-			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
-							 p->real_parent->signal->is_child_subreaper;
-			list_add_tail(&p->sibling, &p->real_parent->children);
+			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
+							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
+			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			attach_pid(p, PIDTYPE_TGID);
 			attach_pid(p, PIDTYPE_PGID);
-- 
2.39.2

[PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should
always be avoided according to the docs. There is a rcu helper
functions rcu_dereference(...) to use when dereferencing a __rcu
pointer. This functions returns the non __rcu tagged pointer which
can be dereferenced just like a normal pointers.


Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
 kernel/fork.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..e78649974669 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
@@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
 			 * tasklist_lock with adding child to the process tree
 			 * for propagate_has_child_subreaper optimization.
 			 */
-			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
-							 p->real_parent->signal->is_child_subreaper;
-			list_add_tail(&p->sibling, &p->real_parent->children);
+			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
+							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
+			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			attach_pid(p, PIDTYPE_TGID);
 			attach_pid(p, PIDTYPE_PGID);
-- 
2.39.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

On 10/27/23 12:07, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> functions rcu_dereference(...) to use when dereferencing a __rcu
> pointer. This functions returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointers.
> 
> 
> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> ---
>   kernel/fork.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..e78649974669 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>   
>   	retval = -EAGAIN;
>   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>   			goto bad_fork_cleanup_count;
>   	}
> @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
>   			 * tasklist_lock with adding child to the process tree
>   			 * for propagate_has_child_subreaper optimization.
>   			 */
> -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> -							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
> +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
> +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
>   			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>   			attach_pid(p, PIDTYPE_TGID);
>   			attach_pid(p, PIDTYPE_PGID);

For this particular file I have resolved the rcu pointer dereferencing 
issue and I have tested the above by using qemu using this command
qemu-system-x86_64 \
	-m 2G \
	-smp 2 \
	-kernel /home/abhinav/linux_work/linux/arch/x86/boot/bzImage \
	-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
	-drive file=/home/abhinav/linux_work/boot_images/bullseye.img,format=raw \
	-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
	-net nic,model=e1000 \
	-enable-kvm \
	-nographic \
	-pidfile vm.pid \
	2>&1 | tee vm.log
it booted without any issues.

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

On 10/27/23 12:07, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> functions rcu_dereference(...) to use when dereferencing a __rcu
> pointer. This functions returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointers.
> 
> 
> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> ---
>   kernel/fork.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..e78649974669 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>   
>   	retval = -EAGAIN;
>   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>   			goto bad_fork_cleanup_count;
>   	}
> @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
>   			 * tasklist_lock with adding child to the process tree
>   			 * for propagate_has_child_subreaper optimization.
>   			 */
> -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> -							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
> +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
> +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
>   			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>   			attach_pid(p, PIDTYPE_TGID);
>   			attach_pid(p, PIDTYPE_PGID);

For this particular file I have resolved the rcu pointer dereferencing 
issue and I have tested the above by using qemu using this command
qemu-system-x86_64 \
	-m 2G \
	-smp 2 \
	-kernel /home/abhinav/linux_work/linux/arch/x86/boot/bzImage \
	-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
	-drive file=/home/abhinav/linux_work/boot_images/bullseye.img,format=raw \
	-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
	-net nic,model=e1000 \
	-enable-kvm \
	-nographic \
	-pidfile vm.pid \
	2>&1 | tee vm.log
it booted without any issues.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Fri, Oct 27, 2023 at 12:11:47PM +0530, Abhinav Singh wrote:
> On 10/27/23 12:07, Abhinav Singh wrote:
> > This patch fixes the warning about directly dereferencing a pointer
> > tagged with __rcu annotation.
> > 
> > Dereferencing the pointers tagged with __rcu directly should
> > always be avoided according to the docs.



> There is a rcu helper
> > functions rcu_dereference(...) to use when dereferencing a __rcu
> > pointer. This functions returns the non __rcu tagged pointer which
> > can be dereferenced just like a normal pointers.
> > 
> > 
> > Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> > ---
> >   kernel/fork.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index 10917c3e1f03..e78649974669 100644
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
> >   	retval = -EAGAIN;
> >   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> > -		if (p->real_cred->user != INIT_USER &&
> > +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
> >   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
> >   			goto bad_fork_cleanup_count;
> >   	}
> > @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
> >   			 * tasklist_lock with adding child to the process tree
> >   			 * for propagate_has_child_subreaper optimization.
> >   			 */
> > -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> > -							 p->real_parent->signal->is_child_subreaper;
> > -			list_add_tail(&p->sibling, &p->real_parent->children);
> > +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
> > +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
> > +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
> >   			list_add_tail_rcu(&p->tasks, &init_task.tasks);
> >   			attach_pid(p, PIDTYPE_TGID);
> >   			attach_pid(p, PIDTYPE_PGID);
> 
> For this particular file I have resolved the rcu pointer dereferencing issue
> and I have tested the above by using qemu using this command
> qemu-system-x86_64 \
> 	-m 2G \
> 	-smp 2 \
> 	-kernel /home/abhinav/linux_work/linux/arch/x86/boot/bzImage \
> 	-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
> 	-drive file=/home/abhinav/linux_work/boot_images/bullseye.img,format=raw \
> 	-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
> 	-net nic,model=e1000 \
> 	-enable-kvm \
> 	-nographic \
> 	-pidfile vm.pid \
> 	2>&1 | tee vm.log
> it booted without any issues.

Did you enable lockdep in your kernel?
CONFIG_PROVE_LOCKING and CONFIG_PROVE_RCU in particular.

-- 
MST

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Fri, Oct 27, 2023 at 12:11:47PM +0530, Abhinav Singh wrote:
> On 10/27/23 12:07, Abhinav Singh wrote:
> > This patch fixes the warning about directly dereferencing a pointer
> > tagged with __rcu annotation.
> > 
> > Dereferencing the pointers tagged with __rcu directly should
> > always be avoided according to the docs.



> There is a rcu helper
> > functions rcu_dereference(...) to use when dereferencing a __rcu
> > pointer. This functions returns the non __rcu tagged pointer which
> > can be dereferenced just like a normal pointers.
> > 
> > 
> > Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> > ---
> >   kernel/fork.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index 10917c3e1f03..e78649974669 100644
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
> >   	retval = -EAGAIN;
> >   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> > -		if (p->real_cred->user != INIT_USER &&
> > +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
> >   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
> >   			goto bad_fork_cleanup_count;
> >   	}
> > @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
> >   			 * tasklist_lock with adding child to the process tree
> >   			 * for propagate_has_child_subreaper optimization.
> >   			 */
> > -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> > -							 p->real_parent->signal->is_child_subreaper;
> > -			list_add_tail(&p->sibling, &p->real_parent->children);
> > +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
> > +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
> > +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
> >   			list_add_tail_rcu(&p->tasks, &init_task.tasks);
> >   			attach_pid(p, PIDTYPE_TGID);
> >   			attach_pid(p, PIDTYPE_PGID);
> 
> For this particular file I have resolved the rcu pointer dereferencing issue
> and I have tested the above by using qemu using this command
> qemu-system-x86_64 \
> 	-m 2G \
> 	-smp 2 \
> 	-kernel /home/abhinav/linux_work/linux/arch/x86/boot/bzImage \
> 	-append "console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
> 	-drive file=/home/abhinav/linux_work/boot_images/bullseye.img,format=raw \
> 	-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
> 	-net nic,model=e1000 \
> 	-enable-kvm \
> 	-nographic \
> 	-pidfile vm.pid \
> 	2>&1 | tee vm.log
> it booted without any issues.

Did you enable lockdep in your kernel?
CONFIG_PROVE_LOCKING and CONFIG_PROVE_RCU in particular.

-- 
MST

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Fri, Oct 27, 2023 at 12:07:13PM +0530, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> functions

function

> rcu_dereference(...) to use when dereferencing a __rcu
> pointer.


... inside rcu read side critical sections.

> This functions

function

> returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointers.

pointer

> 
>

Extra empty line here.
 

Did you test this with lockdep on or did you just build it?
Include info on how the patch was tested pls.

> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>

> ---

Changelog?


>  kernel/fork.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..e78649974669 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>  
>  	retval = -EAGAIN;
>  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>  			goto bad_fork_cleanup_count;
>  	}
> @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
>  			 * tasklist_lock with adding child to the process tree
>  			 * for propagate_has_child_subreaper optimization.
>  			 */
> -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> -							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
> +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
> +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
>  			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>  			attach_pid(p, PIDTYPE_TGID);
>  			attach_pid(p, PIDTYPE_PGID);

It looks like you are calling rcu_dereference outside of
read side critical section and that does not look right to me.
Test with lockdep on.

> -- 
> 2.39.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Fri, Oct 27, 2023 at 12:07:13PM +0530, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> functions

function

> rcu_dereference(...) to use when dereferencing a __rcu
> pointer.


... inside rcu read side critical sections.

> This functions

function

> returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointers.

pointer

> 
>

Extra empty line here.
 

Did you test this with lockdep on or did you just build it?
Include info on how the patch was tested pls.

> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>

> ---

Changelog?


>  kernel/fork.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..e78649974669 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>  
>  	retval = -EAGAIN;
>  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>  			goto bad_fork_cleanup_count;
>  	}
> @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
>  			 * tasklist_lock with adding child to the process tree
>  			 * for propagate_has_child_subreaper optimization.
>  			 */
> -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> -							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
> +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
> +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
>  			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>  			attach_pid(p, PIDTYPE_TGID);
>  			attach_pid(p, PIDTYPE_PGID);

It looks like you are calling rcu_dereference outside of
read side critical section and that does not look right to me.
Test with lockdep on.

> -- 
> 2.39.2

Re: [PATCH] Fixing warning of directly dereferencing __rcu tagged

[Abhinav Singh]

[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]

On 10/26/23 05:20, Andrew Morton wrote:
> On Thu, 26 Oct 2023 04:57:42 +0530 Abhinav Singh <singhabhinav9051571833@gmail.com> wrote:
> 
>> On 10/26/23 04:08, Andrew Morton wrote:
>>>> +++ b/kernel/fork.c
>>>> @@ -2369,7 +2369,9 @@ __latent_entropy struct task_struct *copy_process(
>>>>    
>>>>    	retval = -EAGAIN;
>>>>    	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
>>>> -		if (p->real_cred->user != INIT_USER &&
>>>> +		const struct cred *real_cred = rcu_dereference(p->real_cred);
>>>> +
>>>> +		if (real_cred && real_cred->user != INIT_USER &&
>>>>    		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>>>>    			goto bad_fork_cleanup_count;
>>>
>>> The old code assumes that p->read_cred cannot be NULL and the new code
>>> does nothing to make it possible that `real_cred' can be NULL?
>>>
>>> In other words, I see no reason to add this new check for NULL?
>>
>> Thank you for the response!
>>
>> I thought it will be better to have check before accessing it, just so
>> we dont have any segmentation fault in future.
> 
> That would be adding code which has no effect?
> 
>> Also I just noticed there are two more places where direct dereferencing
>> of __rcu pointer is done in this same file. Should I do those changes in
>> this patch ?
> 
> I don't see why.  rcu_dereference(p) cannot return NULL if `p' is non-NULL?


[-- Attachment #2: 0001-Fixing-sparse-warning-cast-removes-address-space-__i.patch --]
[-- Type: text/x-patch, Size: 1298 bytes --]

From 598bf82b611a9af96c5d412855957e378523f529 Mon Sep 17 00:00:00 2001
From: Abhinav Singh <singhabhinav9051571833@gmail.com>
Date: Tue, 24 Oct 2023 15:24:52 +0530
Subject: [PATCH] Fixing warning cast removes address space '__iomem' of
 expression

This patch fixes sparse complaining about the removal of __iomem address
space when casting the return value of this function ioremap_cache(...)
from `void __ioremap*` to `void*`.

I think there are two way of fixing it, first one is changing the
datatype of variable `ghcb_va` from `void*` to `void __iomem*` . 
Second way of fixing it is using the memremap(...) which is 
done in this patch.

Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
 arch/x86/hyperv/hv_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 21556ad87f4b..c14161add274 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -70,7 +70,7 @@ static int hyperv_init_ghcb(void)
 
 	/* Mask out vTOM bit. ioremap_cache() maps decrypted */
 	ghcb_gpa &= ~ms_hyperv.shared_gpa_boundary;
-	ghcb_va = (void *)ioremap_cache(ghcb_gpa, HV_HYP_PAGE_SIZE);
+	ghcb_va = memremap(ghcb_gpa, HV_HYP_PAGE_SIZE, MEMREMAP_WB);
 	if (!ghcb_va)
 		return -ENOMEM;
 
-- 
2.39.2

Re: [PATCH] Fixing warning of directly dereferencing __rcu tagged

[Abhinav Singh]

[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]

On 10/26/23 05:20, Andrew Morton wrote:
> On Thu, 26 Oct 2023 04:57:42 +0530 Abhinav Singh <singhabhinav9051571833@gmail.com> wrote:
> 
>> On 10/26/23 04:08, Andrew Morton wrote:
>>>> +++ b/kernel/fork.c
>>>> @@ -2369,7 +2369,9 @@ __latent_entropy struct task_struct *copy_process(
>>>>    
>>>>    	retval = -EAGAIN;
>>>>    	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
>>>> -		if (p->real_cred->user != INIT_USER &&
>>>> +		const struct cred *real_cred = rcu_dereference(p->real_cred);
>>>> +
>>>> +		if (real_cred && real_cred->user != INIT_USER &&
>>>>    		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>>>>    			goto bad_fork_cleanup_count;
>>>
>>> The old code assumes that p->read_cred cannot be NULL and the new code
>>> does nothing to make it possible that `real_cred' can be NULL?
>>>
>>> In other words, I see no reason to add this new check for NULL?
>>
>> Thank you for the response!
>>
>> I thought it will be better to have check before accessing it, just so
>> we dont have any segmentation fault in future.
> 
> That would be adding code which has no effect?
> 
>> Also I just noticed there are two more places where direct dereferencing
>> of __rcu pointer is done in this same file. Should I do those changes in
>> this patch ?
> 
> I don't see why.  rcu_dereference(p) cannot return NULL if `p' is non-NULL?


[-- Attachment #2: 0001-Fixing-sparse-warning-cast-removes-address-space-__i.patch --]
[-- Type: text/x-patch, Size: 1298 bytes --]

From 598bf82b611a9af96c5d412855957e378523f529 Mon Sep 17 00:00:00 2001
From: Abhinav Singh <singhabhinav9051571833@gmail.com>
Date: Tue, 24 Oct 2023 15:24:52 +0530
Subject: [PATCH] Fixing warning cast removes address space '__iomem' of
 expression

This patch fixes sparse complaining about the removal of __iomem address
space when casting the return value of this function ioremap_cache(...)
from `void __ioremap*` to `void*`.

I think there are two way of fixing it, first one is changing the
datatype of variable `ghcb_va` from `void*` to `void __iomem*` . 
Second way of fixing it is using the memremap(...) which is 
done in this patch.

Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
 arch/x86/hyperv/hv_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 21556ad87f4b..c14161add274 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -70,7 +70,7 @@ static int hyperv_init_ghcb(void)
 
 	/* Mask out vTOM bit. ioremap_cache() maps decrypted */
 	ghcb_gpa &= ~ms_hyperv.shared_gpa_boundary;
-	ghcb_va = (void *)ioremap_cache(ghcb_gpa, HV_HYP_PAGE_SIZE);
+	ghcb_va = memremap(ghcb_gpa, HV_HYP_PAGE_SIZE, MEMREMAP_WB);
 	if (!ghcb_va)
 		return -ENOMEM;
 
-- 
2.39.2


[-- Attachment #3: Type: text/plain, Size: 201 bytes --]

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[PATCH] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should
always be avoided according to the docs. There is a rcu helper
functions rcu_dereference(...) to use when dereferencing a __rcu
pointer. This functions returns the non __rcu tagged pointer which
can be dereferenced just like a normal pointers.

Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
 kernel/fork.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..802b7bbe3d92 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
@@ -2692,7 +2692,7 @@ __latent_entropy struct task_struct *copy_process(
 			 */
 			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
 							 p->real_parent->signal->is_child_subreaper;
-			list_add_tail(&p->sibling, &p->real_parent->children);
+			list_add_tail(&p->sibling, &(rcu_dereference(p->real_parent)->children));
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			attach_pid(p, PIDTYPE_TGID);
 			attach_pid(p, PIDTYPE_PGID);
-- 
2.39.2

[PATCH] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should
always be avoided according to the docs. There is a rcu helper
functions rcu_dereference(...) to use when dereferencing a __rcu
pointer. This functions returns the non __rcu tagged pointer which
can be dereferenced just like a normal pointers.

Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
 kernel/fork.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..802b7bbe3d92 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
@@ -2692,7 +2692,7 @@ __latent_entropy struct task_struct *copy_process(
 			 */
 			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
 							 p->real_parent->signal->is_child_subreaper;
-			list_add_tail(&p->sibling, &p->real_parent->children);
+			list_add_tail(&p->sibling, &(rcu_dereference(p->real_parent)->children));
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			attach_pid(p, PIDTYPE_TGID);
 			attach_pid(p, PIDTYPE_PGID);
-- 
2.39.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH] Fixing directly deferencing a __rcu pointer warning

[Peter Zijlstra]

$Subject should indicate a subsystem, also you seem to have a somewhat
random collection of Cc. It looks like dhowells is the cred guy and he's
not on.

On Thu, Oct 26, 2023 at 05:57:48PM +0530, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> functions rcu_dereference(...) to use when dereferencing a __rcu
> pointer. This functions returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointers.
> 
> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> ---
>  kernel/fork.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..802b7bbe3d92 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>  
>  	retval = -EAGAIN;
>  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>  			goto bad_fork_cleanup_count;
>  	}

This seems entirely misguided and only makes the code more confusing.

AFAICT at this point @p is not life, we're constructing the new task,
but it's not yet published, therefore no concurrency possible.
Additionally we're not actually in an RCU critical section afaict.

> @@ -2692,7 +2692,7 @@ __latent_entropy struct task_struct *copy_process(
>  			 */
>  			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
>  							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			list_add_tail(&p->sibling, &(rcu_dereference(p->real_parent)->children));
>  			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>  			attach_pid(p, PIDTYPE_TGID);
>  			attach_pid(p, PIDTYPE_PGID);

As to the real_parent, we hold the tasklist lock, which is the write
side lock for parent stuff, so rcu dereference is pointless here.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH] Fixing directly deferencing a __rcu pointer warning

[Peter Zijlstra]

$Subject should indicate a subsystem, also you seem to have a somewhat
random collection of Cc. It looks like dhowells is the cred guy and he's
not on.

On Thu, Oct 26, 2023 at 05:57:48PM +0530, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> functions rcu_dereference(...) to use when dereferencing a __rcu
> pointer. This functions returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointers.
> 
> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> ---
>  kernel/fork.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..802b7bbe3d92 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>  
>  	retval = -EAGAIN;
>  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>  			goto bad_fork_cleanup_count;
>  	}

This seems entirely misguided and only makes the code more confusing.

AFAICT at this point @p is not life, we're constructing the new task,
but it's not yet published, therefore no concurrency possible.
Additionally we're not actually in an RCU critical section afaict.

> @@ -2692,7 +2692,7 @@ __latent_entropy struct task_struct *copy_process(
>  			 */
>  			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
>  							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			list_add_tail(&p->sibling, &(rcu_dereference(p->real_parent)->children));
>  			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>  			attach_pid(p, PIDTYPE_TGID);
>  			attach_pid(p, PIDTYPE_PGID);

As to the real_parent, we hold the tasklist lock, which is the write
side lock for parent stuff, so rcu dereference is pointless here.

Re: [PATCH] Fixing directly deferencing a __rcu pointer warning

[Oleg Nesterov]

On 10/26, Peter Zijlstra wrote:
>
> >  kernel/fork.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index 10917c3e1f03..802b7bbe3d92 100644
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
> >
> >  	retval = -EAGAIN;
> >  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> > -		if (p->real_cred->user != INIT_USER &&
> > +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
> >  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
> >  			goto bad_fork_cleanup_count;
> >  	}
>
> This seems entirely misguided and only makes the code more confusing.
>
> AFAICT at this point @p is not life, we're constructing the new task,
> but it's not yet published, therefore no concurrency possible.
> Additionally we're not actually in an RCU critical section afaict.
>
> > @@ -2692,7 +2692,7 @@ __latent_entropy struct task_struct *copy_process(
> >  			 */
> >  			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> >  							 p->real_parent->signal->is_child_subreaper;
> > -			list_add_tail(&p->sibling, &p->real_parent->children);
> > +			list_add_tail(&p->sibling, &(rcu_dereference(p->real_parent)->children));
> >  			list_add_tail_rcu(&p->tasks, &init_task.tasks);
> >  			attach_pid(p, PIDTYPE_TGID);
> >  			attach_pid(p, PIDTYPE_PGID);
>
> As to the real_parent, we hold the tasklist lock, which is the write
> side lock for parent stuff, so rcu dereference is pointless here.

Agreed.

Plus I don't think this change is correct, iiuc rcu_dereference() will trigger
the run-time "suspicious rcu_dereference_check() usage" warning, it is called
without rcu_read_lock().

Oleg.

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH] Fixing directly deferencing a __rcu pointer warning

[Oleg Nesterov]

On 10/26, Peter Zijlstra wrote:
>
> >  kernel/fork.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index 10917c3e1f03..802b7bbe3d92 100644
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
> >
> >  	retval = -EAGAIN;
> >  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> > -		if (p->real_cred->user != INIT_USER &&
> > +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
> >  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
> >  			goto bad_fork_cleanup_count;
> >  	}
>
> This seems entirely misguided and only makes the code more confusing.
>
> AFAICT at this point @p is not life, we're constructing the new task,
> but it's not yet published, therefore no concurrency possible.
> Additionally we're not actually in an RCU critical section afaict.
>
> > @@ -2692,7 +2692,7 @@ __latent_entropy struct task_struct *copy_process(
> >  			 */
> >  			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> >  							 p->real_parent->signal->is_child_subreaper;
> > -			list_add_tail(&p->sibling, &p->real_parent->children);
> > +			list_add_tail(&p->sibling, &(rcu_dereference(p->real_parent)->children));
> >  			list_add_tail_rcu(&p->tasks, &init_task.tasks);
> >  			attach_pid(p, PIDTYPE_TGID);
> >  			attach_pid(p, PIDTYPE_PGID);
>
> As to the real_parent, we hold the tasklist lock, which is the write
> side lock for parent stuff, so rcu dereference is pointless here.

Agreed.

Plus I don't think this change is correct, iiuc rcu_dereference() will trigger
the run-time "suspicious rcu_dereference_check() usage" warning, it is called
without rcu_read_lock().

Oleg.

[PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should
always be avoided according to the docs. There is a rcu helper
function rcu_dereference(...) to use when dereferencing a __rcu
pointer. This function returns the non __rcu tagged pointer which
can be dereferenced just like a normal pointer.

Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
v1 -> v2 : added rcu_dereference(...) at line 2694
v2 -> v3 : added rcu_dereference(...) at line 2695

 kernel/fork.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..e78649974669 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
@@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
 			 * tasklist_lock with adding child to the process tree
 			 * for propagate_has_child_subreaper optimization.
 			 */
-			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
-							 p->real_parent->signal->is_child_subreaper;
-			list_add_tail(&p->sibling, &p->real_parent->children);
+			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
+							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
+			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			attach_pid(p, PIDTYPE_TGID);
 			attach_pid(p, PIDTYPE_PGID);
-- 
2.39.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should
always be avoided according to the docs. There is a rcu helper
function rcu_dereference(...) to use when dereferencing a __rcu
pointer. This function returns the non __rcu tagged pointer which
can be dereferenced just like a normal pointer.

Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---
v1 -> v2 : added rcu_dereference(...) at line 2694
v2 -> v3 : added rcu_dereference(...) at line 2695

 kernel/fork.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..e78649974669 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
@@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
 			 * tasklist_lock with adding child to the process tree
 			 * for propagate_has_child_subreaper optimization.
 			 */
-			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
-							 p->real_parent->signal->is_child_subreaper;
-			list_add_tail(&p->sibling, &p->real_parent->children);
+			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
+							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
+			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			attach_pid(p, PIDTYPE_TGID);
 			attach_pid(p, PIDTYPE_PGID);
-- 
2.39.2

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

On 10/28/23 15:52, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> function rcu_dereference(...) to use when dereferencing a __rcu
> pointer. This function returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointer.
> 
> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> ---
> v1 -> v2 : added rcu_dereference(...) at line 2694
> v2 -> v3 : added rcu_dereference(...) at line 2695
> 
>   kernel/fork.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..e78649974669 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>   
>   	retval = -EAGAIN;
>   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>   			goto bad_fork_cleanup_count;
>   	}
> @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
>   			 * tasklist_lock with adding child to the process tree
>   			 * for propagate_has_child_subreaper optimization.
>   			 */
> -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> -							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
> +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
> +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
>   			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>   			attach_pid(p, PIDTYPE_TGID);
>   			attach_pid(p, PIDTYPE_PGID);

I tested the above with these two config enabled, CONFIG_PROVE_LOCKING 
and CONFIG_PROVE_RCU. In qemu it booted fine without any issues. I then 
checked dmesg log (inside booted qemu envirnoment) for any issues with 
rcu, but didnt get any error or warning.

I didnt receive any warning message "suspicious rcu_dereference_check() 
usage" atleast from sparse tool or in the dmesg log as suggested by Oleg 
Nesterov.

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

On 10/28/23 15:52, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> function rcu_dereference(...) to use when dereferencing a __rcu
> pointer. This function returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointer.
> 
> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> ---
> v1 -> v2 : added rcu_dereference(...) at line 2694
> v2 -> v3 : added rcu_dereference(...) at line 2695
> 
>   kernel/fork.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..e78649974669 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>   
>   	retval = -EAGAIN;
>   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>   			goto bad_fork_cleanup_count;
>   	}
> @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
>   			 * tasklist_lock with adding child to the process tree
>   			 * for propagate_has_child_subreaper optimization.
>   			 */
> -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> -							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
> +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
> +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
>   			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>   			attach_pid(p, PIDTYPE_TGID);
>   			attach_pid(p, PIDTYPE_PGID);

I tested the above with these two config enabled, CONFIG_PROVE_LOCKING 
and CONFIG_PROVE_RCU. In qemu it booted fine without any issues. I then 
checked dmesg log (inside booted qemu envirnoment) for any issues with 
rcu, but didnt get any error or warning.

I didnt receive any warning message "suspicious rcu_dereference_check() 
usage" atleast from sparse tool or in the dmesg log as suggested by Oleg 
Nesterov.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Sat, Oct 28, 2023 at 03:52:47PM +0530, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> function rcu_dereference(...) to use when dereferencing a __rcu
> pointer. This function returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointer.
> 
> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> ---
> v1 -> v2 : added rcu_dereference(...) at line 2694
> v2 -> v3 : added rcu_dereference(...) at line 2695
> 
>  kernel/fork.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..e78649974669 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>  
>  	retval = -EAGAIN;
>  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>  			goto bad_fork_cleanup_count;
>  	}
> @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
>  			 * tasklist_lock with adding child to the process tree
>  			 * for propagate_has_child_subreaper optimization.
>  			 */
> -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> -							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
> +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
> +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
>  			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>  			attach_pid(p, PIDTYPE_TGID);
>  			attach_pid(p, PIDTYPE_PGID);
> -- 
> 2.39.2


You seem to just ignore review comments. NAK. I'm not going to review this anymore.


-- 
MST

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Michael S. Tsirkin]

On Sat, Oct 28, 2023 at 03:52:47PM +0530, Abhinav Singh wrote:
> This patch fixes the warning about directly dereferencing a pointer
> tagged with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> function rcu_dereference(...) to use when dereferencing a __rcu
> pointer. This function returns the non __rcu tagged pointer which
> can be dereferenced just like a normal pointer.
> 
> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
> ---
> v1 -> v2 : added rcu_dereference(...) at line 2694
> v2 -> v3 : added rcu_dereference(...) at line 2695
> 
>  kernel/fork.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10917c3e1f03..e78649974669 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>  
>  	retval = -EAGAIN;
>  	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
> -		if (p->real_cred->user != INIT_USER &&
> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>  		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>  			goto bad_fork_cleanup_count;
>  	}
> @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
>  			 * tasklist_lock with adding child to the process tree
>  			 * for propagate_has_child_subreaper optimization.
>  			 */
> -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
> -							 p->real_parent->signal->is_child_subreaper;
> -			list_add_tail(&p->sibling, &p->real_parent->children);
> +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
> +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
> +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
>  			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>  			attach_pid(p, PIDTYPE_TGID);
>  			attach_pid(p, PIDTYPE_PGID);
> -- 
> 2.39.2


You seem to just ignore review comments. NAK. I'm not going to review this anymore.


-- 
MST

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

On 10/28/23 17:50, Michael S. Tsirkin wrote:
> On Sat, Oct 28, 2023 at 03:52:47PM +0530, Abhinav Singh wrote:
>> This patch fixes the warning about directly dereferencing a pointer
>> tagged with __rcu annotation.
>>
>> Dereferencing the pointers tagged with __rcu directly should
>> always be avoided according to the docs. There is a rcu helper
>> function rcu_dereference(...) to use when dereferencing a __rcu
>> pointer. This function returns the non __rcu tagged pointer which
>> can be dereferenced just like a normal pointer.
>>
>> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
>> ---
>> v1 -> v2 : added rcu_dereference(...) at line 2694
>> v2 -> v3 : added rcu_dereference(...) at line 2695
>>
>>   kernel/fork.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/fork.c b/kernel/fork.c
>> index 10917c3e1f03..e78649974669 100644
>> --- a/kernel/fork.c
>> +++ b/kernel/fork.c
>> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>>   
>>   	retval = -EAGAIN;
>>   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
>> -		if (p->real_cred->user != INIT_USER &&
>> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>>   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>>   			goto bad_fork_cleanup_count;
>>   	}
>> @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
>>   			 * tasklist_lock with adding child to the process tree
>>   			 * for propagate_has_child_subreaper optimization.
>>   			 */
>> -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
>> -							 p->real_parent->signal->is_child_subreaper;
>> -			list_add_tail(&p->sibling, &p->real_parent->children);
>> +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
>> +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
>> +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
>>   			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>>   			attach_pid(p, PIDTYPE_TGID);
>>   			attach_pid(p, PIDTYPE_PGID);
>> -- 
>> 2.39.2
> 
> 
> You seem to just ignore review comments. NAK. I'm not going to review this anymore.
> 
> 
I m really sorry for ignorance and careless behaviour. This is 
completely my fault, a maintainer has to do a lot of work and he cant 
correct me all the times. On my defense I will only say that I was 
really confused about a thing, instead of asking you question, I thought 
of sending in another patch with some more information is a better 
choice then a clearing my confusion by sending in a extra mail. You were 
very concise and clear about your comments but MY stupidity was on 
another level today. I m sorry for ignorant behaviour. And also thanks 
and appreciate a lot for reviewing this patch till now to all the 
maintainers.

Not sure if this patch will be reviewed again or not, but I think I 
should answer the queries.

The last patch I sent, I tested with `lockdep` on (I hope "test with 
`lockdep on`" means booting the kernel with lockdep enabled),
with these config options `CONFIG_PROVE_RCU` and `CONFIG_PROVE_LOCKING` 
enabled and it booted just fine. To confirm if lockdep was really 
enabled I found these paths inside the qemu virtual envirnoment
/proc/lockdep
/proc/lockdep_chains
/proc/lockdep_stat
/proc/locks
/proc/lock_stats

I tested the above kernel using qemu with this command
qemu-system-x86_64 \
	-m 2G \
	-smp 2 \
	-kernel /home/abhinav/linux_work/linux/arch/x86/boot/bzImage \
	-append "console=ttyS0 root=/dev/sda earlyprintk=serial   net.ifnames=0" \
	-drive file=/home/abhinav/linux_work/boot_images/bullseye.img,format=raw \
	-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
	-net nic,model=e1000 \
	-enable-kvm \
	-nographic \
	-pidfile vm.pid \
	2>&1 | tee vm.log

I did not get warning `the run-time "suspicious rcu_dereference_check() 
usage"` as mentioned by Oleg Nesterov, which mean rcu_dereference(...) 
it called inside of rcu read side critical sections.

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

On 10/28/23 17:50, Michael S. Tsirkin wrote:
> On Sat, Oct 28, 2023 at 03:52:47PM +0530, Abhinav Singh wrote:
>> This patch fixes the warning about directly dereferencing a pointer
>> tagged with __rcu annotation.
>>
>> Dereferencing the pointers tagged with __rcu directly should
>> always be avoided according to the docs. There is a rcu helper
>> function rcu_dereference(...) to use when dereferencing a __rcu
>> pointer. This function returns the non __rcu tagged pointer which
>> can be dereferenced just like a normal pointer.
>>
>> Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
>> ---
>> v1 -> v2 : added rcu_dereference(...) at line 2694
>> v2 -> v3 : added rcu_dereference(...) at line 2695
>>
>>   kernel/fork.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/fork.c b/kernel/fork.c
>> index 10917c3e1f03..e78649974669 100644
>> --- a/kernel/fork.c
>> +++ b/kernel/fork.c
>> @@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
>>   
>>   	retval = -EAGAIN;
>>   	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
>> -		if (p->real_cred->user != INIT_USER &&
>> +		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
>>   		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
>>   			goto bad_fork_cleanup_count;
>>   	}
>> @@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
>>   			 * tasklist_lock with adding child to the process tree
>>   			 * for propagate_has_child_subreaper optimization.
>>   			 */
>> -			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
>> -							 p->real_parent->signal->is_child_subreaper;
>> -			list_add_tail(&p->sibling, &p->real_parent->children);
>> +			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
>> +							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
>> +			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
>>   			list_add_tail_rcu(&p->tasks, &init_task.tasks);
>>   			attach_pid(p, PIDTYPE_TGID);
>>   			attach_pid(p, PIDTYPE_PGID);
>> -- 
>> 2.39.2
> 
> 
> You seem to just ignore review comments. NAK. I'm not going to review this anymore.
> 
> 
I m really sorry for ignorance and careless behaviour. This is 
completely my fault, a maintainer has to do a lot of work and he cant 
correct me all the times. On my defense I will only say that I was 
really confused about a thing, instead of asking you question, I thought 
of sending in another patch with some more information is a better 
choice then a clearing my confusion by sending in a extra mail. You were 
very concise and clear about your comments but MY stupidity was on 
another level today. I m sorry for ignorant behaviour. And also thanks 
and appreciate a lot for reviewing this patch till now to all the 
maintainers.

Not sure if this patch will be reviewed again or not, but I think I 
should answer the queries.

The last patch I sent, I tested with `lockdep` on (I hope "test with 
`lockdep on`" means booting the kernel with lockdep enabled),
with these config options `CONFIG_PROVE_RCU` and `CONFIG_PROVE_LOCKING` 
enabled and it booted just fine. To confirm if lockdep was really 
enabled I found these paths inside the qemu virtual envirnoment
/proc/lockdep
/proc/lockdep_chains
/proc/lockdep_stat
/proc/locks
/proc/lock_stats

I tested the above kernel using qemu with this command
qemu-system-x86_64 \
	-m 2G \
	-smp 2 \
	-kernel /home/abhinav/linux_work/linux/arch/x86/boot/bzImage \
	-append "console=ttyS0 root=/dev/sda earlyprintk=serial   net.ifnames=0" \
	-drive file=/home/abhinav/linux_work/boot_images/bullseye.img,format=raw \
	-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
	-net nic,model=e1000 \
	-enable-kvm \
	-nographic \
	-pidfile vm.pid \
	2>&1 | tee vm.log

I did not get warning `the run-time "suspicious rcu_dereference_check() 
usage"` as mentioned by Oleg Nesterov, which mean rcu_dereference(...) 
it called inside of rcu read side critical sections.




_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should
always be avoided according to the docs. There is a rcu helper
function rcu_dereference(...) to use when dereferencing a __rcu
pointer inside rcu read side critical sections. This function 
returns the non __rcu tagged pointer which can be dereferenced 
just like a normal pointer.

Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---

Link to original patch 
 https://lore.kernel.org/all/20231025222811.855336-1-singhabhinav9051571833@gmail.com/

Change from original -> v2 : 
 1. removed the null check before dereferencing the dereferenced rcu
    pointer at line 2372.
 2. added rcu_dereference(...) at line 2694
    
Changes from v2 -> v3 
 1. added rcu_dereference(...) at line 2693

 kernel/fork.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..e78649974669 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
@@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
 			 * tasklist_lock with adding child to the process tree
 			 * for propagate_has_child_subreaper optimization.
 			 */
-			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
-							 p->real_parent->signal->is_child_subreaper;
-			list_add_tail(&p->sibling, &p->real_parent->children);
+			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
+							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
+			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			attach_pid(p, PIDTYPE_TGID);
 			attach_pid(p, PIDTYPE_PGID);
-- 
2.39.2

[PATCH v3] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

This patch fixes the warning about directly dereferencing a pointer
tagged with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should
always be avoided according to the docs. There is a rcu helper
function rcu_dereference(...) to use when dereferencing a __rcu
pointer inside rcu read side critical sections. This function 
returns the non __rcu tagged pointer which can be dereferenced 
just like a normal pointer.

Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---

Link to original patch 
 https://lore.kernel.org/all/20231025222811.855336-1-singhabhinav9051571833@gmail.com/

Change from original -> v2 : 
 1. removed the null check before dereferencing the dereferenced rcu
    pointer at line 2372.
 2. added rcu_dereference(...) at line 2694
    
Changes from v2 -> v3 
 1. added rcu_dereference(...) at line 2693

 kernel/fork.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..e78649974669 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,7 +2369,7 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
@@ -2690,9 +2690,9 @@ __latent_entropy struct task_struct *copy_process(
 			 * tasklist_lock with adding child to the process tree
 			 * for propagate_has_child_subreaper optimization.
 			 */
-			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
-							 p->real_parent->signal->is_child_subreaper;
-			list_add_tail(&p->sibling, &p->real_parent->children);
+			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
+							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
+			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			attach_pid(p, PIDTYPE_TGID);
 			attach_pid(p, PIDTYPE_PGID);
-- 
2.39.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[kernel test robot]

Hello,

kernel test robot noticed "WARNING:suspicious_RCU_usage" on:

commit: 0c940f3d0d4c41567b1957a4e09ad68bdeee2111 ("[PATCH v3] Fixing directly deferencing a __rcu pointer warning")
url: https://github.com/intel-lab-lkp/linux/commits/Abhinav-Singh/Fixing-directly-deferencing-a-__rcu-pointer-warning/20231029-044918
base: https://git.kernel.org/cgit/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/all/20231028204609.426841-1-singhabhinav9051571833@gmail.com/
patch subject: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

in testcase: boot

compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)


+--------------------------------------------------------+------------+------------+
|                                                        | 44c9217272 | 0c940f3d0d |
+--------------------------------------------------------+------------+------------+
| WARNING:suspicious_RCU_usage                           | 0          | 8          |
| kernel/fork.c:#suspicious_rcu_dereference_check()usage | 0          | 8          |
+--------------------------------------------------------+------------+------------+


If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202311031320.c3ebc782-oliver.sang@intel.com


[    2.386253][    T0] WARNING: suspicious RCU usage
[    2.386253][    T0] 6.6.0-rc4-00506-g0c940f3d0d4c #7 Not tainted
[    2.386253][    T0] -----------------------------
[    2.386253][    T0] kernel/fork.c:2688 suspicious rcu_dereference_check() usage!
[    2.386253][    T0]
[    2.386253][    T0] other info that might help us debug this:
[    2.386253][    T0]
[    2.386253][    T0] rcu_scheduler_active = 2, debug_locks = 1
[    2.386253][    T0] 3 locks held by swapper/0:
[ 2.386253][ T0] #0: ffffffff84dbdc50 (cgroup_threadgroup_rwsem){.+.+}-{0:0}, at: cgroup_can_fork (kbuild/src/rand-x86_64-3/kernel/cgroup/cgroup.c:6538) 
[ 2.386253][ T0] #1: ffffffff8482f010 (tasklist_lock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2624) 
[ 2.386253][ T0] #2: ffffffff84846178 (init_sighand.siglock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2226 kbuild/src/rand-x86_64-3/kernel/fork.c:2643) 
[    2.386253][    T0]
[    2.386253][    T0] stack backtrace:
[    2.386253][    T0] CPU: 0 PID: 0 Comm: swapper Not tainted 6.6.0-rc4-00506-g0c940f3d0d4c #7 88a30a6c68427eeed926405592f52ff30544ebdd
[    2.386253][    T0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[    2.386253][    T0] Call Trace:
[    2.386253][    T0]  <TASK>
[ 2.386253][ T0] copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2688 (discriminator 9)) 
[ 2.386253][ T0] ? pidfd_prepare (kbuild/src/rand-x86_64-3/kernel/fork.c:2245) 
[ 2.386253][ T0] ? acpi_hw_validate_register (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:196) 
[ 2.386253][ T0] ? proc_register (kbuild/src/rand-x86_64-3/fs/proc/generic.c:383) 
[ 2.386253][ T0] kernel_clone (kbuild/src/rand-x86_64-3/include/linux/random.h:26 kbuild/src/rand-x86_64-3/kernel/fork.c:2903) 
[ 2.386253][ T0] ? acpi_hw_register_read (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:489) 
[ 2.386253][ T0] ? create_io_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2862) 
[ 2.386253][ T0] user_mode_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? kernel_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? acpi_hw_validate_io_request (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwvalid.c:128) 
[ 2.386253][ T0] ? rest_init (kbuild/src/rand-x86_64-3/init/main.c:1429) 
[ 2.386253][ T0] rest_init (kbuild/src/rand-x86_64-3/init/main.c:691) 
[ 2.386253][ T0] arch_call_rest_init+0x13/0x80 
[ 2.386253][ T0] start_kernel (kbuild/src/rand-x86_64-3/init/main.c:992 (discriminator 1)) 
[ 2.386253][ T0] x86_64_start_reservations (kbuild/src/rand-x86_64-3/arch/x86/kernel/head64.c:544) 
[ 2.386253][ T0] x86_64_start_kernel (??:?) 
[ 2.386253][ T0] secondary_startup_64_no_verify (kbuild/src/rand-x86_64-3/arch/x86/kernel/head_64.S:433) 
[    2.386253][    T0]  </TASK>
[    2.386253][    T0]
[    2.386253][    T0] =============================
[    2.386253][    T0] WARNING: suspicious RCU usage
[    2.386253][    T0] 6.6.0-rc4-00506-g0c940f3d0d4c #7 Not tainted
[    2.386253][    T0] -----------------------------
[    2.386253][    T0] kernel/fork.c:2689 suspicious rcu_dereference_check() usage!
[    2.386253][    T0]
[    2.386253][    T0] other info that might help us debug this:
[    2.386253][    T0]
[    2.386253][    T0] rcu_scheduler_active = 2, debug_locks = 1
[    2.386253][    T0] 3 locks held by swapper/0:
[ 2.386253][ T0] #0: ffffffff84dbdc50 (cgroup_threadgroup_rwsem){.+.+}-{0:0}, at: cgroup_can_fork (kbuild/src/rand-x86_64-3/kernel/cgroup/cgroup.c:6538) 
[ 2.386253][ T0] #1: ffffffff8482f010 (tasklist_lock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2624) 
[ 2.386253][ T0] #2: ffffffff84846178 (init_sighand.siglock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2226 kbuild/src/rand-x86_64-3/kernel/fork.c:2643) 
[    2.386253][    T0]
[    2.386253][    T0] stack backtrace:
[    2.386253][    T0] CPU: 0 PID: 0 Comm: swapper Not tainted 6.6.0-rc4-00506-g0c940f3d0d4c #7 88a30a6c68427eeed926405592f52ff30544ebdd
[    2.386253][    T0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[    2.386253][    T0] Call Trace:
[    2.386253][    T0]  <TASK>
[ 2.386253][ T0] copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2689 (discriminator 9)) 
[ 2.386253][ T0] ? pidfd_prepare (kbuild/src/rand-x86_64-3/kernel/fork.c:2245) 
[ 2.386253][ T0] ? acpi_hw_validate_register (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:196) 
[ 2.386253][ T0] ? proc_register (kbuild/src/rand-x86_64-3/fs/proc/generic.c:383) 
[ 2.386253][ T0] kernel_clone (kbuild/src/rand-x86_64-3/include/linux/random.h:26 kbuild/src/rand-x86_64-3/kernel/fork.c:2903) 
[ 2.386253][ T0] ? acpi_hw_register_read (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:489) 
[ 2.386253][ T0] ? create_io_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2862) 
[ 2.386253][ T0] user_mode_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? kernel_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? acpi_hw_validate_io_request (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwvalid.c:128) 
[ 2.386253][ T0] ? rest_init (kbuild/src/rand-x86_64-3/init/main.c:1429) 
[ 2.386253][ T0] rest_init (kbuild/src/rand-x86_64-3/init/main.c:691) 
[ 2.386253][ T0] arch_call_rest_init+0x13/0x80 
[ 2.386253][ T0] start_kernel (kbuild/src/rand-x86_64-3/init/main.c:992 (discriminator 1)) 
[ 2.386253][ T0] x86_64_start_reservations (kbuild/src/rand-x86_64-3/arch/x86/kernel/head64.c:544) 
[ 2.386253][ T0] x86_64_start_kernel (??:?) 
[ 2.386253][ T0] secondary_startup_64_no_verify (kbuild/src/rand-x86_64-3/arch/x86/kernel/head_64.S:433) 
[    2.386253][    T0]  </TASK>
[    2.386253][    T0]
[    2.386253][    T0] =============================
[    2.386253][    T0] WARNING: suspicious RCU usage
[    2.386253][    T0] 6.6.0-rc4-00506-g0c940f3d0d4c #7 Not tainted
[    2.386253][    T0] -----------------------------
[    2.386253][    T0] kernel/fork.c:2690 suspicious rcu_dereference_check() usage!
[    2.386253][    T0]
[    2.386253][    T0] other info that might help us debug this:
[    2.386253][    T0]
[    2.386253][    T0] rcu_scheduler_active = 2, debug_locks = 1
[    2.386253][    T0] 3 locks held by swapper/0:
[ 2.386253][ T0] #0: ffffffff84dbdc50 (cgroup_threadgroup_rwsem){.+.+}-{0:0}, at: cgroup_can_fork (kbuild/src/rand-x86_64-3/kernel/cgroup/cgroup.c:6538) 
[ 2.386253][ T0] #1: ffffffff8482f010 (tasklist_lock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2624) 
[ 2.386253][ T0] #2: ffffffff84846178 (init_sighand.siglock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2226 kbuild/src/rand-x86_64-3/kernel/fork.c:2643) 
[    2.386253][    T0]
[    2.386253][    T0] stack backtrace:
[    2.386253][    T0] CPU: 0 PID: 0 Comm: swapper Not tainted 6.6.0-rc4-00506-g0c940f3d0d4c #7 88a30a6c68427eeed926405592f52ff30544ebdd
[    2.386253][    T0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[    2.386253][    T0] Call Trace:
[    2.386253][    T0]  <TASK>
[ 2.386253][ T0] copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2690 (discriminator 9)) 
[ 2.386253][ T0] ? pidfd_prepare (kbuild/src/rand-x86_64-3/kernel/fork.c:2245) 
[ 2.386253][ T0] ? acpi_hw_validate_register (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:196) 
[ 2.386253][ T0] ? proc_register (kbuild/src/rand-x86_64-3/fs/proc/generic.c:383) 
[ 2.386253][ T0] kernel_clone (kbuild/src/rand-x86_64-3/include/linux/random.h:26 kbuild/src/rand-x86_64-3/kernel/fork.c:2903) 
[ 2.386253][ T0] ? acpi_hw_register_read (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:489) 
[ 2.386253][ T0] ? create_io_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2862) 
[ 2.386253][ T0] user_mode_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? kernel_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? acpi_hw_validate_io_request (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwvalid.c:128) 
[ 2.386253][ T0] ? rest_init (kbuild/src/rand-x86_64-3/init/main.c:1429) 
[ 2.386253][ T0] rest_init (kbuild/src/rand-x86_64-3/init/main.c:691) 
[ 2.386253][ T0] arch_call_rest_init+0x13/0x80 
[ 2.386253][ T0] start_kernel (kbuild/src/rand-x86_64-3/init/main.c:992 (discriminator 1)) 
[ 2.386253][ T0] x86_64_start_reservations (kbuild/src/rand-x86_64-3/arch/x86/kernel/head64.c:544) 
[ 2.386253][ T0] x86_64_start_kernel (??:?) 
[ 2.386253][ T0] secondary_startup_64_no_verify (kbuild/src/rand-x86_64-3/arch/x86/kernel/head_64.S:433) 
[    2.386253][    T0]  </TASK>
[    2.387851][    T1] RCU Tasks: Setting shift to 0 and lim to 1 rcu_task_cb_adjust=1.
[    2.389031][    T1] RCU Tasks Rude: Setting shift to 0 and lim to 1 rcu_task_cb_adjust=1.
[    2.390637][    T1] RCU Tasks Trace: Setting shift to 0 and lim to 1 rcu_task_cb_adjust=1.
[    2.391901][    T1] Running RCU-tasks wait API self tests


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20231103/202311031320.c3ebc782-oliver.sang@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

[kernel test robot]

Hello,

kernel test robot noticed "WARNING:suspicious_RCU_usage" on:

commit: 0c940f3d0d4c41567b1957a4e09ad68bdeee2111 ("[PATCH v3] Fixing directly deferencing a __rcu pointer warning")
url: https://github.com/intel-lab-lkp/linux/commits/Abhinav-Singh/Fixing-directly-deferencing-a-__rcu-pointer-warning/20231029-044918
base: https://git.kernel.org/cgit/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/all/20231028204609.426841-1-singhabhinav9051571833@gmail.com/
patch subject: [PATCH v3] Fixing directly deferencing a __rcu pointer warning

in testcase: boot

compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)


+--------------------------------------------------------+------------+------------+
|                                                        | 44c9217272 | 0c940f3d0d |
+--------------------------------------------------------+------------+------------+
| WARNING:suspicious_RCU_usage                           | 0          | 8          |
| kernel/fork.c:#suspicious_rcu_dereference_check()usage | 0          | 8          |
+--------------------------------------------------------+------------+------------+


If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202311031320.c3ebc782-oliver.sang@intel.com


[    2.386253][    T0] WARNING: suspicious RCU usage
[    2.386253][    T0] 6.6.0-rc4-00506-g0c940f3d0d4c #7 Not tainted
[    2.386253][    T0] -----------------------------
[    2.386253][    T0] kernel/fork.c:2688 suspicious rcu_dereference_check() usage!
[    2.386253][    T0]
[    2.386253][    T0] other info that might help us debug this:
[    2.386253][    T0]
[    2.386253][    T0] rcu_scheduler_active = 2, debug_locks = 1
[    2.386253][    T0] 3 locks held by swapper/0:
[ 2.386253][ T0] #0: ffffffff84dbdc50 (cgroup_threadgroup_rwsem){.+.+}-{0:0}, at: cgroup_can_fork (kbuild/src/rand-x86_64-3/kernel/cgroup/cgroup.c:6538) 
[ 2.386253][ T0] #1: ffffffff8482f010 (tasklist_lock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2624) 
[ 2.386253][ T0] #2: ffffffff84846178 (init_sighand.siglock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2226 kbuild/src/rand-x86_64-3/kernel/fork.c:2643) 
[    2.386253][    T0]
[    2.386253][    T0] stack backtrace:
[    2.386253][    T0] CPU: 0 PID: 0 Comm: swapper Not tainted 6.6.0-rc4-00506-g0c940f3d0d4c #7 88a30a6c68427eeed926405592f52ff30544ebdd
[    2.386253][    T0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[    2.386253][    T0] Call Trace:
[    2.386253][    T0]  <TASK>
[ 2.386253][ T0] copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2688 (discriminator 9)) 
[ 2.386253][ T0] ? pidfd_prepare (kbuild/src/rand-x86_64-3/kernel/fork.c:2245) 
[ 2.386253][ T0] ? acpi_hw_validate_register (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:196) 
[ 2.386253][ T0] ? proc_register (kbuild/src/rand-x86_64-3/fs/proc/generic.c:383) 
[ 2.386253][ T0] kernel_clone (kbuild/src/rand-x86_64-3/include/linux/random.h:26 kbuild/src/rand-x86_64-3/kernel/fork.c:2903) 
[ 2.386253][ T0] ? acpi_hw_register_read (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:489) 
[ 2.386253][ T0] ? create_io_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2862) 
[ 2.386253][ T0] user_mode_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? kernel_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? acpi_hw_validate_io_request (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwvalid.c:128) 
[ 2.386253][ T0] ? rest_init (kbuild/src/rand-x86_64-3/init/main.c:1429) 
[ 2.386253][ T0] rest_init (kbuild/src/rand-x86_64-3/init/main.c:691) 
[ 2.386253][ T0] arch_call_rest_init+0x13/0x80 
[ 2.386253][ T0] start_kernel (kbuild/src/rand-x86_64-3/init/main.c:992 (discriminator 1)) 
[ 2.386253][ T0] x86_64_start_reservations (kbuild/src/rand-x86_64-3/arch/x86/kernel/head64.c:544) 
[ 2.386253][ T0] x86_64_start_kernel (??:?) 
[ 2.386253][ T0] secondary_startup_64_no_verify (kbuild/src/rand-x86_64-3/arch/x86/kernel/head_64.S:433) 
[    2.386253][    T0]  </TASK>
[    2.386253][    T0]
[    2.386253][    T0] =============================
[    2.386253][    T0] WARNING: suspicious RCU usage
[    2.386253][    T0] 6.6.0-rc4-00506-g0c940f3d0d4c #7 Not tainted
[    2.386253][    T0] -----------------------------
[    2.386253][    T0] kernel/fork.c:2689 suspicious rcu_dereference_check() usage!
[    2.386253][    T0]
[    2.386253][    T0] other info that might help us debug this:
[    2.386253][    T0]
[    2.386253][    T0] rcu_scheduler_active = 2, debug_locks = 1
[    2.386253][    T0] 3 locks held by swapper/0:
[ 2.386253][ T0] #0: ffffffff84dbdc50 (cgroup_threadgroup_rwsem){.+.+}-{0:0}, at: cgroup_can_fork (kbuild/src/rand-x86_64-3/kernel/cgroup/cgroup.c:6538) 
[ 2.386253][ T0] #1: ffffffff8482f010 (tasklist_lock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2624) 
[ 2.386253][ T0] #2: ffffffff84846178 (init_sighand.siglock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2226 kbuild/src/rand-x86_64-3/kernel/fork.c:2643) 
[    2.386253][    T0]
[    2.386253][    T0] stack backtrace:
[    2.386253][    T0] CPU: 0 PID: 0 Comm: swapper Not tainted 6.6.0-rc4-00506-g0c940f3d0d4c #7 88a30a6c68427eeed926405592f52ff30544ebdd
[    2.386253][    T0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[    2.386253][    T0] Call Trace:
[    2.386253][    T0]  <TASK>
[ 2.386253][ T0] copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2689 (discriminator 9)) 
[ 2.386253][ T0] ? pidfd_prepare (kbuild/src/rand-x86_64-3/kernel/fork.c:2245) 
[ 2.386253][ T0] ? acpi_hw_validate_register (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:196) 
[ 2.386253][ T0] ? proc_register (kbuild/src/rand-x86_64-3/fs/proc/generic.c:383) 
[ 2.386253][ T0] kernel_clone (kbuild/src/rand-x86_64-3/include/linux/random.h:26 kbuild/src/rand-x86_64-3/kernel/fork.c:2903) 
[ 2.386253][ T0] ? acpi_hw_register_read (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:489) 
[ 2.386253][ T0] ? create_io_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2862) 
[ 2.386253][ T0] user_mode_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? kernel_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? acpi_hw_validate_io_request (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwvalid.c:128) 
[ 2.386253][ T0] ? rest_init (kbuild/src/rand-x86_64-3/init/main.c:1429) 
[ 2.386253][ T0] rest_init (kbuild/src/rand-x86_64-3/init/main.c:691) 
[ 2.386253][ T0] arch_call_rest_init+0x13/0x80 
[ 2.386253][ T0] start_kernel (kbuild/src/rand-x86_64-3/init/main.c:992 (discriminator 1)) 
[ 2.386253][ T0] x86_64_start_reservations (kbuild/src/rand-x86_64-3/arch/x86/kernel/head64.c:544) 
[ 2.386253][ T0] x86_64_start_kernel (??:?) 
[ 2.386253][ T0] secondary_startup_64_no_verify (kbuild/src/rand-x86_64-3/arch/x86/kernel/head_64.S:433) 
[    2.386253][    T0]  </TASK>
[    2.386253][    T0]
[    2.386253][    T0] =============================
[    2.386253][    T0] WARNING: suspicious RCU usage
[    2.386253][    T0] 6.6.0-rc4-00506-g0c940f3d0d4c #7 Not tainted
[    2.386253][    T0] -----------------------------
[    2.386253][    T0] kernel/fork.c:2690 suspicious rcu_dereference_check() usage!
[    2.386253][    T0]
[    2.386253][    T0] other info that might help us debug this:
[    2.386253][    T0]
[    2.386253][    T0] rcu_scheduler_active = 2, debug_locks = 1
[    2.386253][    T0] 3 locks held by swapper/0:
[ 2.386253][ T0] #0: ffffffff84dbdc50 (cgroup_threadgroup_rwsem){.+.+}-{0:0}, at: cgroup_can_fork (kbuild/src/rand-x86_64-3/kernel/cgroup/cgroup.c:6538) 
[ 2.386253][ T0] #1: ffffffff8482f010 (tasklist_lock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2624) 
[ 2.386253][ T0] #2: ffffffff84846178 (init_sighand.siglock){....}-{2:2}, at: copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2226 kbuild/src/rand-x86_64-3/kernel/fork.c:2643) 
[    2.386253][    T0]
[    2.386253][    T0] stack backtrace:
[    2.386253][    T0] CPU: 0 PID: 0 Comm: swapper Not tainted 6.6.0-rc4-00506-g0c940f3d0d4c #7 88a30a6c68427eeed926405592f52ff30544ebdd
[    2.386253][    T0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[    2.386253][    T0] Call Trace:
[    2.386253][    T0]  <TASK>
[ 2.386253][ T0] copy_process (kbuild/src/rand-x86_64-3/kernel/fork.c:2690 (discriminator 9)) 
[ 2.386253][ T0] ? pidfd_prepare (kbuild/src/rand-x86_64-3/kernel/fork.c:2245) 
[ 2.386253][ T0] ? acpi_hw_validate_register (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:196) 
[ 2.386253][ T0] ? proc_register (kbuild/src/rand-x86_64-3/fs/proc/generic.c:383) 
[ 2.386253][ T0] kernel_clone (kbuild/src/rand-x86_64-3/include/linux/random.h:26 kbuild/src/rand-x86_64-3/kernel/fork.c:2903) 
[ 2.386253][ T0] ? acpi_hw_register_read (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwregs.c:489) 
[ 2.386253][ T0] ? create_io_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2862) 
[ 2.386253][ T0] user_mode_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? kernel_thread (kbuild/src/rand-x86_64-3/kernel/fork.c:2971) 
[ 2.386253][ T0] ? acpi_hw_validate_io_request (kbuild/src/rand-x86_64-3/drivers/acpi/acpica/hwvalid.c:128) 
[ 2.386253][ T0] ? rest_init (kbuild/src/rand-x86_64-3/init/main.c:1429) 
[ 2.386253][ T0] rest_init (kbuild/src/rand-x86_64-3/init/main.c:691) 
[ 2.386253][ T0] arch_call_rest_init+0x13/0x80 
[ 2.386253][ T0] start_kernel (kbuild/src/rand-x86_64-3/init/main.c:992 (discriminator 1)) 
[ 2.386253][ T0] x86_64_start_reservations (kbuild/src/rand-x86_64-3/arch/x86/kernel/head64.c:544) 
[ 2.386253][ T0] x86_64_start_kernel (??:?) 
[ 2.386253][ T0] secondary_startup_64_no_verify (kbuild/src/rand-x86_64-3/arch/x86/kernel/head_64.S:433) 
[    2.386253][    T0]  </TASK>
[    2.387851][    T1] RCU Tasks: Setting shift to 0 and lim to 1 rcu_task_cb_adjust=1.
[    2.389031][    T1] RCU Tasks Rude: Setting shift to 0 and lim to 1 rcu_task_cb_adjust=1.
[    2.390637][    T1] RCU Tasks Trace: Setting shift to 0 and lim to 1 rcu_task_cb_adjust=1.
[    2.391901][    T1] Running RCU-tasks wait API self tests


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20231103/202311031320.c3ebc782-oliver.sang@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[PATCH v4] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

This patch fixes the sparse warning with this message 
"dereference of noderef expression" , in this context 
it means about directly dereferencing a pointer tagged 
with __rcu annotation.

Dereferencing the pointers tagged with __rcu directly should
always be avoided according to the docs. There is a rcu helper
function rcu_dereference(...) to use when dereferencing a __rcu
pointer inside rcu read side critical sections. This function
returns the non __rcu tagged pointer which can be dereferenced
just like a normal pointer.

I tested with `lockdep` enabled, with these config options
`CONFIG_PROVE_RCU` and `CONFIG_PROVE_LOCKING` enabled and it 
booted just fine. To confirm if lockdep was really enabled 
I found these paths inside the qemu virtual envirnoment.
/proc/lockdep
/proc/lockdep_chains
/proc/lockdep_stat
/proc/locks
/proc/lock_stats

I tested the above kernel using qemu with this command
qemu-system-x86_64 \
        -m 2G \
        -smp 2 \
        -kernel /home/abhinav/linux_work/linux/arch/x86/boot/bzImage \
        -append "console=ttyS0 root=/dev/sda earlyprintk=serial   net.ifnames=0" \
        -drive file=/home/abhinav/linux_work/boot_images/bullseye.img,format=raw \
        -net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
        -net nic,model=e1000 \
        -enable-kvm \
        -nographic \
        -pidfile vm.pid \
        2>&1 | tee vm.log

Signed-off-by: Abhinav Singh <singhabhinav9051571833@gmail.com>
---

Link to original patch
 https://lore.kernel.org/all/20231025222811.855336-1-singhabhinav9051571833@gmail.com/

Change from original -> v2 :
 1. removed the null check before dereferencing the dereferenced rcu
    pointer at line 2372.
 2. added rcu_dereference(...) at line 2694

Changes from v2 -> v3
 1. added rcu_dereference(...) at line 2693

Changes from v3 -> v4
 1. added rcu_read_lock() and rcu_read_unlock() before and after
rcu_defereference() function to avoid race condition.

 kernel/fork.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10917c3e1f03..bb049b611015 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2369,9 +2369,14 @@ __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_rlimit_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
-		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
+		rcu_read_lock();
+		if (rcu_dereference(p->real_cred)->user != INIT_USER &&
+		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) {
+			rcu_read_unlock();
 			goto bad_fork_cleanup_count;
+		} else {
+			rcu_read_unlock();
+		}
 	}
 	current->flags &= ~PF_NPROC_EXCEEDED;
 
@@ -2690,9 +2695,11 @@ __latent_entropy struct task_struct *copy_process(
 			 * tasklist_lock with adding child to the process tree
 			 * for propagate_has_child_subreaper optimization.
 			 */
-			p->signal->has_child_subreaper = p->real_parent->signal->has_child_subreaper ||
-							 p->real_parent->signal->is_child_subreaper;
-			list_add_tail(&p->sibling, &p->real_parent->children);
+			rcu_read_lock();
+			p->signal->has_child_subreaper = rcu_dereference(p->real_parent)->signal->has_child_subreaper ||
+							rcu_dereference(p->real_parent)->signal->is_child_subreaper;
+			list_add_tail(&p->sibling, &rcu_dereference(p->real_parent)->children);
+			rcu_read_unlock();
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			attach_pid(p, PIDTYPE_TGID);
 			attach_pid(p, PIDTYPE_PGID);
-- 
2.39.2

Re: [PATCH v4] Fixing directly deferencing a __rcu pointer warning

[Mathieu Desnoyers]

On 2023-11-12 14:30, Abhinav Singh wrote:
> This patch fixes the sparse warning with this message
> "dereference of noderef expression" , in this context
> it means about directly dereferencing a pointer tagged
> with __rcu annotation.
> 
> Dereferencing the pointers tagged with __rcu directly should
> always be avoided according to the docs. There is a rcu helper
> function rcu_dereference(...) to use when dereferencing a __rcu
> pointer inside rcu read side critical sections. This function
> returns the non __rcu tagged pointer which can be dereferenced
> just like a normal pointer.
> 
> I tested with `lockdep` enabled, with these config options
> `CONFIG_PROVE_RCU` and `CONFIG_PROVE_LOCKING` enabled and it
> booted just fine. To confirm if lockdep was really enabled
> I found these paths inside the qemu virtual envirnoment.

I did not see actions taken nor answer on those comments:

https://lore.kernel.org/all/CAGudoHEfjSAim6Hh-qYPY+qi8nbLx7J3YdpGgFwSvD7xbeYR3w@mail.gmail.com/

Basically, the missing annotation here can be either:

- A missing rcu_dereference, if the intent is to use the pointer while protecting
   with with a read-side critical section,
- A missing rcu_dereference_protected(), if the use of the pointer is protected by
   a lock.

I don't really care if rcu_dereference happens to work in testing or not. _If_ the
intended design is that this rcu pointer is protected by a lock, or being used before
becoming published elsewhere, then using rcu_dereference to silence the warning is
wrong. (note: I did not do a review of the affected code, but I would expect the
commit message to take care of going through this level of detail)

And the fact that this aspect of the feedback has been hidden under the rug worries
me.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Re: [PATCH v4] Fixing directly deferencing a __rcu pointer warning

[Peter Zijlstra]

On Mon, Nov 13, 2023 at 01:00:55AM +0530, Abhinav Singh wrote:
> This patch fixes the sparse warning with this message 
> "dereference of noderef expression" , in this context 
> it means about directly dereferencing a pointer tagged 
> with __rcu annotation.

Please, stop this madness. Just accept that sparse is a stupid tool and
the code is actually fine.

Re: [PATCH v4] Fixing directly deferencing a __rcu pointer warning

[Abhinav Singh]

On 11/13/23 01:28, Peter Zijlstra wrote:
> On Mon, Nov 13, 2023 at 01:00:55AM +0530, Abhinav Singh wrote:
>> This patch fixes the sparse warning with this message
>> "dereference of noderef expression" , in this context
>> it means about directly dereferencing a pointer tagged
>> with __rcu annotation.
> 
> Please, stop this madness. Just accept that sparse is a stupid tool and
> the code is actually fine.

Thank you for your tine maintainers. Okay I understood that this extra 
patch, is not required. Just one follow up question, in some driver code 
as well I see these warning, might be possible those are real issue.
Can you suggest me like how do I know if a rcu related warning is really 
a issue or just a false positive warning.

Thanks,
Abhinav.