[PATCH] kernel/sys.c: use RCU when accessing task_struct->real_parent

[PATCH] kernel/sys.c: use RCU when accessing task_struct->real_parent

[Alex Elder]

In the setpgid() syscall definition, a check is made to determine
whether the target process is in the same thread group as the
current process.  The check accesses the target process's real_parent
pointer directly, however that field is supposed to be accessed via
RCU.  Use rcu_dereference() to avoid this C=1 build warning:

  kernel/sys.c:1144:32: warning: incorrect type in argument 1 (different address spaces)

Signed-off-by: Alex Elder <elder@riscstar.com>
---
 kernel/sys.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index df69bd71de032..434fe6ad30e13 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1114,6 +1114,7 @@ COMPAT_SYSCALL_DEFINE1(times, struct compat_tms __user *, tbuf)
 SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
 {
 	struct task_struct *p;
+	struct task_struct *real_parent;
 	struct task_struct *group_leader = current->group_leader;
 	struct pid *pids[PIDTYPE_MAX] = { 0 };
 	struct pid *pgrp;
@@ -1141,7 +1142,8 @@ SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
 	if (!thread_group_leader(p))
 		goto out;
 
-	if (same_thread_group(p->real_parent, group_leader)) {
+	real_parent = rcu_dereference(p->real_parent);
+	if (same_thread_group(real_parent, group_leader)) {
 		err = -EPERM;
 		if (task_session(p) != task_session(group_leader))
 			goto out;

base-commit: 6b8c8af514d739d0335f5579b585e02babe8a727
-- 
2.53.0

Re: [PATCH] kernel/sys.c: use RCU when accessing task_struct->real_parent

[Andrew Morton]

On Tue, 11 Aug 2026 14:05:00 -0500 Alex Elder <elder@riscstar.com> wrote:

> In the setpgid() syscall definition, a check is made to determine
> whether the target process is in the same thread group as the
> current process.  The check accesses the target process's real_parent
> pointer directly, however that field is supposed to be accessed via
> RCU.  Use rcu_dereference() to avoid this C=1 build warning:
> 
>   kernel/sys.c:1144:32: warning: incorrect type in argument 1 (different address spaces)

Oh.

Why don't all those other uses of ->real_parent produce this warning?


> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -1114,6 +1114,7 @@ COMPAT_SYSCALL_DEFINE1(times, struct compat_tms __user *, tbuf)
>  SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
>  {
>  	struct task_struct *p;
> +	struct task_struct *real_parent;
>  	struct task_struct *group_leader = current->group_leader;
>  	struct pid *pids[PIDTYPE_MAX] = { 0 };
>  	struct pid *pgrp;
> @@ -1141,7 +1142,8 @@ SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
>  	if (!thread_group_leader(p))
>  		goto out;
>  
> -	if (same_thread_group(p->real_parent, group_leader)) {
> +	real_parent = rcu_dereference(p->real_parent);
> +	if (same_thread_group(real_parent, group_leader)) {
>  		err = -EPERM;
>  		if (task_session(p) != task_session(group_leader))
>  			goto out;
> 

Re: [PATCH] kernel/sys.c: use RCU when accessing task_struct->real_parent

[Alex Elder]

On 8/11/26 8:17 PM, Andrew Morton wrote:
> On Tue, 11 Aug 2026 14:05:00 -0500 Alex Elder <elder@riscstar.com> wrote:
> 
>> In the setpgid() syscall definition, a check is made to determine
>> whether the target process is in the same thread group as the
>> current process.  The check accesses the target process's real_parent
>> pointer directly, however that field is supposed to be accessed via
>> RCU.  Use rcu_dereference() to avoid this C=1 build warning:
>>
>>    kernel/sys.c:1144:32: warning: incorrect type in argument 1 (different address spaces)
> 
> Oh.
> 
> Why don't all those other uses of ->real_parent produce this warning?

Looks like there are others; this was just the one that kept
showing up in my builds.  Maybe some cases simply don't need
the rcu_dereference() (like initializing init_task.real_parent
in init/init_task.c).

It's inconsistent.  Some files use rcu_dereference() always
for task_struct->real_parent:
- drivers/connector/cn_proc.c
- fs/binfmt_elf.c
- fs/nfs/dir.c
- fs/binfmt_elf_fdpic.c
- kernel/sched/core.c
- kernel/acct.c
- security/keys/keyctl.c
- security/selinux/hooks.c
- security/yama/yama_lsm.c

But others do not.  And there are lots of other fields
annotated with __rcu that don't always use rcu_dereference():
- task_struct->parent
- task_struct->sighand
- mm_struct->owner
and so on.

I could go dig a little deeper and solve this more comprehensively.
It would probably have to be a long-running background effort.
Do you think it's worth it?

If not, I don't mind just retracting this suggestion for now.

					-Alex

> 
> 
>> --- a/kernel/sys.c
>> +++ b/kernel/sys.c
>> @@ -1114,6 +1114,7 @@ COMPAT_SYSCALL_DEFINE1(times, struct compat_tms __user *, tbuf)
>>   SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
>>   {
>>   	struct task_struct *p;
>> +	struct task_struct *real_parent;
>>   	struct task_struct *group_leader = current->group_leader;
>>   	struct pid *pids[PIDTYPE_MAX] = { 0 };
>>   	struct pid *pgrp;
>> @@ -1141,7 +1142,8 @@ SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
>>   	if (!thread_group_leader(p))
>>   		goto out;
>>   
>> -	if (same_thread_group(p->real_parent, group_leader)) {
>> +	real_parent = rcu_dereference(p->real_parent);
>> +	if (same_thread_group(real_parent, group_leader)) {
>>   		err = -EPERM;
>>   		if (task_session(p) != task_session(group_leader))
>>   			goto out;
>>
> 
> 

Re: [PATCH] kernel/sys.c: use RCU when accessing task_struct->real_parent

[Oleg Nesterov]

Just in case, I am travelling without my work laptop until Aug 19,
can't read the code and I rarely read emails...

On 08/11, Alex Elder wrote:
>
> In the setpgid() syscall definition, a check is made to determine
> whether the target process is in the same thread group as the
> current process.  The check accesses the target process's real_parent
> pointer directly, however that field is supposed to be accessed via
> RCU.  Use rcu_dereference() to avoid this C=1 build warning:
>
>   kernel/sys.c:1144:32: warning: incorrect type in argument 1 (different address spaces)

This is the sparse warning... Do we really care? We have a lot
more users of ->real_parent without rcu_dereference().

Note also that the "rcu" annotation of ->real_parent is misleading.
If the task exits, task->real_parent points to nowhere. Same for
->group_leader.

This reminds me... months ago I was going to introduce the helper
to access ->group_leader and move it to signal_struct. Then I was
going to do the same with ->real_parent. I sent some preparations,
but then I was distracted. I'll try to return to this after PTO.

>  SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
>  {
>  	struct task_struct *p;
> +	struct task_struct *real_parent;
>  	struct task_struct *group_leader = current->group_leader;
>  	struct pid *pids[PIDTYPE_MAX] = { 0 };
>  	struct pid *pgrp;
> @@ -1141,7 +1142,8 @@ SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
>  	if (!thread_group_leader(p))
>  		goto out;
>
> -	if (same_thread_group(p->real_parent, group_leader)) {
> +	real_parent = rcu_dereference(p->real_parent);
> +	if (same_thread_group(real_parent, group_leader)) {
>  		err = -EPERM;
>  		if (task_session(p) != task_session(group_leader))
>  			goto out;

IIRC, this code runs under tasklist_lock (at least it should ;)
->real_parent is stable.

I'd prefer to leave this code as is (see above), but even if we
really want to shut up sparse we don't need rcu_dereference()
anyway, we are not going to dereference this pointer.

We have other helpers to read the "rcu" pointers, but I can't
recall the names...

Oleg.

Re: [PATCH] kernel/sys.c: use RCU when accessing task_struct->real_parent

[Alex Elder]

On 8/12/26 2:55 AM, Oleg Nesterov wrote:
> Just in case, I am travelling without my work laptop until Aug 19,
> can't read the code and I rarely read emails...
> 
> On 08/11, Alex Elder wrote:
>>
>> In the setpgid() syscall definition, a check is made to determine
>> whether the target process is in the same thread group as the
>> current process.  The check accesses the target process's real_parent
>> pointer directly, however that field is supposed to be accessed via
>> RCU.  Use rcu_dereference() to avoid this C=1 build warning:
>>
>>    kernel/sys.c:1144:32: warning: incorrect type in argument 1 (different address spaces)
> 
> This is the sparse warning... Do we really care? We have a lot
> more users of ->real_parent without rcu_dereference().
> 
> Note also that the "rcu" annotation of ->real_parent is misleading.
> If the task exits, task->real_parent points to nowhere. Same for
> ->group_leader.

Right, there are some cases where there is other protection as
well (like the one you point out below).

> This reminds me... months ago I was going to introduce the helper
> to access ->group_leader and move it to signal_struct. Then I was
> going to do the same with ->real_parent. I sent some preparations,
> but then I was distracted. I'll try to return to this after PTO.

Yes, this is a better solution overall.  I sent this patch to
address one warning I kept seeing in my builds, but Andrew
pointed out there are lots of others that would also warn.

>>   SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
>>   {
>>   	struct task_struct *p;
>> +	struct task_struct *real_parent;
>>   	struct task_struct *group_leader = current->group_leader;
>>   	struct pid *pids[PIDTYPE_MAX] = { 0 };
>>   	struct pid *pgrp;
>> @@ -1141,7 +1142,8 @@ SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid)
>>   	if (!thread_group_leader(p))
>>   		goto out;
>>
>> -	if (same_thread_group(p->real_parent, group_leader)) {
>> +	real_parent = rcu_dereference(p->real_parent);
>> +	if (same_thread_group(real_parent, group_leader)) {
>>   		err = -EPERM;
>>   		if (task_session(p) != task_session(group_leader))
>>   			goto out;
> 
> IIRC, this code runs under tasklist_lock (at least it should ;)
> ->real_parent is stable.
> 
> I'd prefer to leave this code as is (see above), but even if we
> really want to shut up sparse we don't need rcu_dereference()
> anyway, we are not going to dereference this pointer.

OK that's perfectly fine with me.  I retract this patch,
and will look forward to you introducing helpers to address
this in a more general way.

Thanks a lot.

					-Alex
> We have other helpers to read the "rcu" pointers, but I can't
> recall the names...
> 
> Oleg.
>