[PATCH v2] futex: don't leak robust_list pointer on exec race

[PATCH v2] futex: don't leak robust_list pointer on exec race

[Pranav Tyagi]

sys_get_robust_list() and compat_get_robust_list() use
ptrace_may_access() to check if the calling task is allowed to access
another task's robust_list pointer. This check is racy against a
concurrent exec() in the target process.

During exec(), a task may transition from a non-privileged binary to a
privileged one (e.g., setuid binary) and its credentials/memory mappings
may change. If get_robust_list() performs ptrace_may_access() before
this transition, it may erroneously allow access to sensitive information
after the target becomes privileged.

A racy access allows an attacker to exploit a window
during which ptrace_may_access() passes before a target process
transitions to a privileged state via exec().

For example, consider a non-privileged task T that is about to execute a
setuid-root binary. An attacker task A calls get_robust_list(T) while T
is still unprivileged. Since ptrace_may_access() checks permissions
based on current credentials, it succeeds. However, if T begins exec
immediately afterwards, it becomes privileged and may change its memory
mappings. Because get_robust_list() proceeds to access T->robust_list
without synchronizing with exec() it may read user-space pointers from a
now-privileged process.

This violates the intended post-exec access restrictions and could
expose sensitive memory addresses or be used as a primitive in a larger
exploit chain. Consequently, the race can lead to unauthorized
disclosure of information across privilege boundaries and poses a
potential security risk.

Take a read lock on signal->exec_update_lock prior to invoking
ptrace_may_access() and accessing the robust_list/compat_robust_list.
This ensures that the target task's exec state remains stable during the
check, allowing for consistent and synchronized validation of
credentials.

changed in v2:
- improved changelog
- helper function for common part of the compat and native syscalls

Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
Suggested-by: Jann Horn <jann@thejh.net>
Link: https://lore.kernel.org/linux-fsdevel/1477863998-3298-5-git-send-email-jann@thejh.net/
Link: https://github.com/KSPP/linux/issues/119
---
 kernel/futex/syscalls.c | 110 ++++++++++++++++++++++------------------
 1 file changed, 62 insertions(+), 48 deletions(-)

diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
index 4b6da9116aa6..3278d91d95ce 100644
--- a/kernel/futex/syscalls.c
+++ b/kernel/futex/syscalls.c
@@ -39,46 +39,81 @@ SYSCALL_DEFINE2(set_robust_list, struct robust_list_head __user *, head,
 	return 0;
 }
 
-/**
- * sys_get_robust_list() - Get the robust-futex list head of a task
- * @pid:	pid of the process [zero for current task]
- * @head_ptr:	pointer to a list-head pointer, the kernel fills it in
- * @len_ptr:	pointer to a length field, the kernel fills in the header size
- */
-SYSCALL_DEFINE3(get_robust_list, int, pid,
-		struct robust_list_head __user * __user *, head_ptr,
-		size_t __user *, len_ptr)
+static void __user *get_robust_list_common(int pid,
+		bool compat)
 {
-	struct robust_list_head __user *head;
+	void __user *head;
 	unsigned long ret;
-	struct task_struct *p;
 
-	rcu_read_lock();
+	struct task_struct *p;
 
-	ret = -ESRCH;
-	if (!pid)
+	if (!pid) {
 		p = current;
-	else {
+		get_task_struct(p);
+	} else {
+		rcu_read_lock();
 		p = find_task_by_vpid(pid);
+		/*
+		 * pin the task to permit dropping the RCU read lock before
+		 * acquiring the semaphore
+		 */
+		if (p)
+			get_task_struct(p);
+		rcu_read_unlock();
 		if (!p)
-			goto err_unlock;
+			return ERR_PTR(-ESRCH);
 	}
 
+	/*
+	 * Hold exec_update_lock to serialize with concurrent exec()
+	 * so ptrace_may_access() is checked against stable credentials
+	 */
+
+	ret = down_read_killable(&p->signal->exec_update_lock);
+	if (ret)
+		goto err_put;
+
 	ret = -EPERM;
 	if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
 		goto err_unlock;
 
-	head = p->robust_list;
-	rcu_read_unlock();
+	if (compat)
+		head = p->compat_robust_list;
+	else
+		head = p->robust_list;
 
-	if (put_user(sizeof(*head), len_ptr))
-		return -EFAULT;
-	return put_user(head, head_ptr);
+	up_read(&p->signal->exec_update_lock);
+	put_task_struct(p);
+
+	return head;
 
 err_unlock:
-	rcu_read_unlock();
+	up_read(&p->signal->exec_update_lock);
+err_put:
+	put_task_struct(p);
+	return ERR_PTR(ret);
+}
 
-	return ret;
+
+/**
+ * sys_get_robust_list() - Get the robust-futex list head of a task
+ * @pid:	pid of the process [zero for current task]
+ * @head_ptr:	pointer to a list-head pointer, the kernel fills it in
+ * @len_ptr:	pointer to a length field, the kernel fills in the header size
+ */
+SYSCALL_DEFINE3(get_robust_list, int, pid,
+		struct robust_list_head __user * __user *, head_ptr,
+		size_t __user *, len_ptr)
+{
+	struct robust_list_head __user *head =
+		get_robust_list_common(pid, false);
+
+	if (IS_ERR(head))
+		return PTR_ERR(head);
+
+	if (put_user(sizeof(*head), len_ptr))
+		return -EFAULT;
+	return put_user(head, head_ptr);
 }
 
 long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
@@ -455,36 +490,15 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
 			compat_uptr_t __user *, head_ptr,
 			compat_size_t __user *, len_ptr)
 {
-	struct compat_robust_list_head __user *head;
-	unsigned long ret;
-	struct task_struct *p;
+	struct compat_robust_list_head __user *head =
+		get_robust_list_common(pid, true);
 
-	rcu_read_lock();
-
-	ret = -ESRCH;
-	if (!pid)
-		p = current;
-	else {
-		p = find_task_by_vpid(pid);
-		if (!p)
-			goto err_unlock;
-	}
-
-	ret = -EPERM;
-	if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
-		goto err_unlock;
-
-	head = p->compat_robust_list;
-	rcu_read_unlock();
+	if (IS_ERR(head))
+		return PTR_ERR(head);
 
 	if (put_user(sizeof(*head), len_ptr))
 		return -EFAULT;
 	return put_user(ptr_to_compat(head), head_ptr);
-
-err_unlock:
-	rcu_read_unlock();
-
-	return ret;
 }
 #endif /* CONFIG_COMPAT */
 
-- 
2.49.0

Re: [PATCH v2] futex: don't leak robust_list pointer on exec race

[Kees Cook]

On Mon, Aug 04, 2025 at 05:25:33PM +0530, Pranav Tyagi wrote:
> sys_get_robust_list() and compat_get_robust_list() use
> ptrace_may_access() to check if the calling task is allowed to access
> another task's robust_list pointer. This check is racy against a
> concurrent exec() in the target process.
> 
> During exec(), a task may transition from a non-privileged binary to a
> privileged one (e.g., setuid binary) and its credentials/memory mappings
> may change. If get_robust_list() performs ptrace_may_access() before
> this transition, it may erroneously allow access to sensitive information
> after the target becomes privileged.
> 
> A racy access allows an attacker to exploit a window
> during which ptrace_may_access() passes before a target process
> transitions to a privileged state via exec().
> 
> For example, consider a non-privileged task T that is about to execute a
> setuid-root binary. An attacker task A calls get_robust_list(T) while T
> is still unprivileged. Since ptrace_may_access() checks permissions
> based on current credentials, it succeeds. However, if T begins exec
> immediately afterwards, it becomes privileged and may change its memory
> mappings. Because get_robust_list() proceeds to access T->robust_list
> without synchronizing with exec() it may read user-space pointers from a
> now-privileged process.
> 
> This violates the intended post-exec access restrictions and could
> expose sensitive memory addresses or be used as a primitive in a larger
> exploit chain. Consequently, the race can lead to unauthorized
> disclosure of information across privilege boundaries and poses a
> potential security risk.
> 
> Take a read lock on signal->exec_update_lock prior to invoking
> ptrace_may_access() and accessing the robust_list/compat_robust_list.
> This ensures that the target task's exec state remains stable during the
> check, allowing for consistent and synchronized validation of
> credentials.

Thanks for this commit log; I think it captures the concern very well.

> 
> changed in v2:
> - improved changelog
> - helper function for common part of the compat and native syscalls
> 
> Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
> Suggested-by: Jann Horn <jann@thejh.net>
> Link: https://lore.kernel.org/linux-fsdevel/1477863998-3298-5-git-send-email-jann@thejh.net/
> Link: https://github.com/KSPP/linux/issues/119
> ---
>  kernel/futex/syscalls.c | 110 ++++++++++++++++++++++------------------
>  1 file changed, 62 insertions(+), 48 deletions(-)
> 
> diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
> index 4b6da9116aa6..3278d91d95ce 100644
> --- a/kernel/futex/syscalls.c
> +++ b/kernel/futex/syscalls.c
> @@ -39,46 +39,81 @@ SYSCALL_DEFINE2(set_robust_list, struct robust_list_head __user *, head,
>  	return 0;
>  }
>  
> -/**
> - * sys_get_robust_list() - Get the robust-futex list head of a task
> - * @pid:	pid of the process [zero for current task]
> - * @head_ptr:	pointer to a list-head pointer, the kernel fills it in
> - * @len_ptr:	pointer to a length field, the kernel fills in the header size
> - */
> -SYSCALL_DEFINE3(get_robust_list, int, pid,
> -		struct robust_list_head __user * __user *, head_ptr,
> -		size_t __user *, len_ptr)
> +static void __user *get_robust_list_common(int pid,
> +		bool compat)
>  {
> -	struct robust_list_head __user *head;
> +	void __user *head;
>  	unsigned long ret;
> -	struct task_struct *p;
>  
> -	rcu_read_lock();
> +	struct task_struct *p;
>  
> -	ret = -ESRCH;
> -	if (!pid)
> +	if (!pid) {
>  		p = current;
> -	else {
> +		get_task_struct(p);
> +	} else {
> +		rcu_read_lock();
>  		p = find_task_by_vpid(pid);
> +		/*
> +		 * pin the task to permit dropping the RCU read lock before
> +		 * acquiring the semaphore
> +		 */
> +		if (p)
> +			get_task_struct(p);
> +		rcu_read_unlock();
>  		if (!p)
> -			goto err_unlock;
> +			return ERR_PTR(-ESRCH);
>  	}
>  
> +	/*
> +	 * Hold exec_update_lock to serialize with concurrent exec()
> +	 * so ptrace_may_access() is checked against stable credentials
> +	 */
> +
> +	ret = down_read_killable(&p->signal->exec_update_lock);
> +	if (ret)
> +		goto err_put;
> +
>  	ret = -EPERM;
>  	if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
>  		goto err_unlock;
>  
> -	head = p->robust_list;
> -	rcu_read_unlock();
> +	if (compat)
> +		head = p->compat_robust_list;
> +	else
> +		head = p->robust_list;
>  
> -	if (put_user(sizeof(*head), len_ptr))
> -		return -EFAULT;
> -	return put_user(head, head_ptr);
> +	up_read(&p->signal->exec_update_lock);
> +	put_task_struct(p);
> +
> +	return head;
>  
>  err_unlock:
> -	rcu_read_unlock();
> +	up_read(&p->signal->exec_update_lock);
> +err_put:
> +	put_task_struct(p);
> +	return ERR_PTR(ret);
> +}
>  
> -	return ret;
> +
> +/**
> + * sys_get_robust_list() - Get the robust-futex list head of a task
> + * @pid:	pid of the process [zero for current task]
> + * @head_ptr:	pointer to a list-head pointer, the kernel fills it in
> + * @len_ptr:	pointer to a length field, the kernel fills in the header size
> + */
> +SYSCALL_DEFINE3(get_robust_list, int, pid,
> +		struct robust_list_head __user * __user *, head_ptr,
> +		size_t __user *, len_ptr)
> +{
> +	struct robust_list_head __user *head =
> +		get_robust_list_common(pid, false);
> +
> +	if (IS_ERR(head))
> +		return PTR_ERR(head);
> +
> +	if (put_user(sizeof(*head), len_ptr))
> +		return -EFAULT;
> +	return put_user(head, head_ptr);
>  }
>  
>  long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
> @@ -455,36 +490,15 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
>  			compat_uptr_t __user *, head_ptr,
>  			compat_size_t __user *, len_ptr)
>  {
> -	struct compat_robust_list_head __user *head;
> -	unsigned long ret;
> -	struct task_struct *p;
> +	struct compat_robust_list_head __user *head =
> +		get_robust_list_common(pid, true);
>  
> -	rcu_read_lock();
> -
> -	ret = -ESRCH;
> -	if (!pid)
> -		p = current;
> -	else {
> -		p = find_task_by_vpid(pid);
> -		if (!p)
> -			goto err_unlock;
> -	}
> -
> -	ret = -EPERM;
> -	if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
> -		goto err_unlock;
> -
> -	head = p->compat_robust_list;
> -	rcu_read_unlock();
> +	if (IS_ERR(head))
> +		return PTR_ERR(head);
>  
>  	if (put_user(sizeof(*head), len_ptr))
>  		return -EFAULT;
>  	return put_user(ptr_to_compat(head), head_ptr);
> -
> -err_unlock:
> -	rcu_read_unlock();
> -
> -	return ret;
>  }
>  #endif /* CONFIG_COMPAT */
>  
> -- 
> 2.49.0
> 

This looks good to me; it needs to use a void * for the common helper,
but it's nice to have it all in one place.

Reviewed-by: Kees Cook <kees@kernel.org>

-Kees

-- 
Kees Cook

Re: [PATCH v2] futex: don't leak robust_list pointer on exec race

[Thomas Gleixner]

On Mon, Aug 04 2025 at 17:25, Pranav Tyagi wrote:
> Take a read lock on signal->exec_update_lock prior to invoking
> ptrace_may_access() and accessing the robust_list/compat_robust_list.
> This ensures that the target task's exec state remains stable during the
> check, allowing for consistent and synchronized validation of
> credentials.
>
> changed in v2:
> - improved changelog
> - helper function for common part of the compat and native syscalls

Please put version log below the --- line. That's not part of the change log.

> Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
> Suggested-by: Jann Horn <jann@thejh.net>
> Link: https://lore.kernel.org/linux-fsdevel/1477863998-3298-5-git-send-email-jann@thejh.net/
> Link: https://github.com/KSPP/linux/issues/119
> ---
>  kernel/futex/syscalls.c | 110 ++++++++++++++++++++++------------------
>  1 file changed, 62 insertions(+), 48 deletions(-)
>
> diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
> index 4b6da9116aa6..3278d91d95ce 100644
> --- a/kernel/futex/syscalls.c
> +++ b/kernel/futex/syscalls.c
> @@ -39,46 +39,81 @@ SYSCALL_DEFINE2(set_robust_list, struct robust_list_head __user *, head,
>  	return 0;
>  }
>  
> -/**
> - * sys_get_robust_list() - Get the robust-futex list head of a task
> - * @pid:	pid of the process [zero for current task]
> - * @head_ptr:	pointer to a list-head pointer, the kernel fills it in
> - * @len_ptr:	pointer to a length field, the kernel fills in the header size
> - */
> -SYSCALL_DEFINE3(get_robust_list, int, pid,
> -		struct robust_list_head __user * __user *, head_ptr,
> -		size_t __user *, len_ptr)
> +static void __user *get_robust_list_common(int pid,
> +		bool compat)

What is this random line break for?

>  {
> -	struct robust_list_head __user *head;
> +	void __user *head;
>  	unsigned long ret;
> -	struct task_struct *p;
>  

Stray new line and please use reverse fir tree ordering of variables:

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations

> -	rcu_read_lock();
> +	struct task_struct *p;
>  
> -	ret = -ESRCH;
> -	if (!pid)
> +	if (!pid) {
>  		p = current;
> -	else {
> +		get_task_struct(p);
> +	} else {
> +		rcu_read_lock();
>  		p = find_task_by_vpid(pid);
> +		/*
> +		 * pin the task to permit dropping the RCU read lock before
> +		 * acquiring the semaphore
> +		 */
> +		if (p)
> +			get_task_struct(p);
> +		rcu_read_unlock();
>  		if (!p)
> -			goto err_unlock;
> +			return ERR_PTR(-ESRCH);

                scoped_guard(rcu) {
                     p = find_task_by_vpid(pid);
                     if (!p)
			return (void __user *)ERR_PTR(-ESRCH);
                     get_task_struct(p);
                }

No need for a comment about pinning the task. This is obvious and a
common pattern all over the place. And note the type case on the error
return.

But you can simplify this whole thing even further:

	struct task_struct *p = current;

	scoped_guard(rcu) {
        	if (pid) {
                     p = find_task_by_vpid(pid);
                     if (!p)
			return (void __user *)ERR_PTR(-ESRCH);
		}
        	get_task_struct(p);
        }

Yes, RCU is not required for the !pid case, but this is not a hot path.

>  
> +	/*
> +	 * Hold exec_update_lock to serialize with concurrent exec()
> +	 * so ptrace_may_access() is checked against stable credentials
> +	 */
> +

Stray newline.

> +	ret = down_read_killable(&p->signal->exec_update_lock);
> +	if (ret)
> +		goto err_put;
> +
>  	ret = -EPERM;
>  	if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
>  		goto err_unlock;
>  
> -	head = p->robust_list;
> -	rcu_read_unlock();
> +	if (compat)
> +		head = p->compat_robust_list;
> +	else
> +		head = p->robust_list;

Brain compiler complains about a build fail with CONFIG_COMPAT=n

static inline void __user *task_robust_list(struct task_struct *p, bool compat)
{
#ifdef COMPAT
	if (compat)
		return p->compat_robust_list;
#endif
	return p->robust_list;
}

So you don't have the #ifdef ugly in this function..

> -	if (put_user(sizeof(*head), len_ptr))
> -		return -EFAULT;
> -	return put_user(head, head_ptr);
> +	up_read(&p->signal->exec_update_lock);
> +	put_task_struct(p);
> +
> +	return head;
>  
>  err_unlock:
> -	rcu_read_unlock();
> +	up_read(&p->signal->exec_update_lock);
> +err_put:
> +	put_task_struct(p);
> +	return ERR_PTR(ret);
> +}
>  
> -	return ret;
> +
> +/**
> + * sys_get_robust_list() - Get the robust-futex list head of a task
> + * @pid:	pid of the process [zero for current task]
> + * @head_ptr:	pointer to a list-head pointer, the kernel fills it in
> + * @len_ptr:	pointer to a length field, the kernel fills in the header size
> + */
> +SYSCALL_DEFINE3(get_robust_list, int, pid,
> +		struct robust_list_head __user * __user *, head_ptr,
> +		size_t __user *, len_ptr)
> +{
> +	struct robust_list_head __user *head =
> +		get_robust_list_common(pid, false);

No line break required.

> +	if (IS_ERR(head))
> +		return PTR_ERR(head);
> +
> +	if (put_user(sizeof(*head), len_ptr))
> +		return -EFAULT;
> +	return put_user(head, head_ptr);
>  }
>  
>  long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
> @@ -455,36 +490,15 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
>  			compat_uptr_t __user *, head_ptr,
>  			compat_size_t __user *, len_ptr)
>  {
> -	struct compat_robust_list_head __user *head;
> -	unsigned long ret;
> -	struct task_struct *p;
> +	struct compat_robust_list_head __user *head =
> +		get_robust_list_common(pid, true);

Ditto

Thanks,

        tglx

Re: [PATCH v2] futex: don't leak robust_list pointer on exec race

[kernel test robot]

Hi Pranav,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/locking/core]
[also build test ERROR on linus/master v6.16 next-20250805]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Pranav-Tyagi/futex-don-t-leak-robust_list-pointer-on-exec-race/20250804-195739
base:   tip/locking/core
patch link:    https://lore.kernel.org/r/20250804115533.14186-1-pranav.tyagi03%40gmail.com
patch subject: [PATCH v2] futex: don't leak robust_list pointer on exec race
config: riscv-randconfig-001-20250805 (https://download.01.org/0day-ci/archive/20250805/202508051607.VJkmdtGV-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250805/202508051607.VJkmdtGV-lkp@intel.com/reproduce)

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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508051607.VJkmdtGV-lkp@intel.com/

All errors (new ones prefixed by >>):

>> kernel/futex/syscalls.c:81:13: error: no member named 'compat_robust_list' in 'struct task_struct'
      81 |                 head = p->compat_robust_list;
         |                        ~  ^
   1 error generated.


vim +81 kernel/futex/syscalls.c

    41	
    42	static void __user *get_robust_list_common(int pid,
    43			bool compat)
    44	{
    45		void __user *head;
    46		unsigned long ret;
    47	
    48		struct task_struct *p;
    49	
    50		if (!pid) {
    51			p = current;
    52			get_task_struct(p);
    53		} else {
    54			rcu_read_lock();
    55			p = find_task_by_vpid(pid);
    56			/*
    57			 * pin the task to permit dropping the RCU read lock before
    58			 * acquiring the semaphore
    59			 */
    60			if (p)
    61				get_task_struct(p);
    62			rcu_read_unlock();
    63			if (!p)
    64				return ERR_PTR(-ESRCH);
    65		}
    66	
    67		/*
    68		 * Hold exec_update_lock to serialize with concurrent exec()
    69		 * so ptrace_may_access() is checked against stable credentials
    70		 */
    71	
    72		ret = down_read_killable(&p->signal->exec_update_lock);
    73		if (ret)
    74			goto err_put;
    75	
    76		ret = -EPERM;
    77		if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
    78			goto err_unlock;
    79	
    80		if (compat)
  > 81			head = p->compat_robust_list;
    82		else
    83			head = p->robust_list;
    84	
    85		up_read(&p->signal->exec_update_lock);
    86		put_task_struct(p);
    87	
    88		return head;
    89	
    90	err_unlock:
    91		up_read(&p->signal->exec_update_lock);
    92	err_put:
    93		put_task_struct(p);
    94		return ERR_PTR(ret);
    95	}
    96	

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

Re: [PATCH v2] futex: don't leak robust_list pointer on exec race

[Pranav Tyagi]

On Tue, Aug 5, 2025 at 1:49 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Mon, Aug 04 2025 at 17:25, Pranav Tyagi wrote:
> > Take a read lock on signal->exec_update_lock prior to invoking
> > ptrace_may_access() and accessing the robust_list/compat_robust_list.
> > This ensures that the target task's exec state remains stable during the
> > check, allowing for consistent and synchronized validation of
> > credentials.
> >
> > changed in v2:
> > - improved changelog
> > - helper function for common part of the compat and native syscalls
>
> Please put version log below the --- line. That's not part of the change log.
>
> > Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
> > Suggested-by: Jann Horn <jann@thejh.net>
> > Link: https://lore.kernel.org/linux-fsdevel/1477863998-3298-5-git-send-email-jann@thejh.net/
> > Link: https://github.com/KSPP/linux/issues/119
> > ---
> >  kernel/futex/syscalls.c | 110 ++++++++++++++++++++++------------------
> >  1 file changed, 62 insertions(+), 48 deletions(-)
> >
> > diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
> > index 4b6da9116aa6..3278d91d95ce 100644
> > --- a/kernel/futex/syscalls.c
> > +++ b/kernel/futex/syscalls.c
> > @@ -39,46 +39,81 @@ SYSCALL_DEFINE2(set_robust_list, struct robust_list_head __user *, head,
> >       return 0;
> >  }
> >
> > -/**
> > - * sys_get_robust_list() - Get the robust-futex list head of a task
> > - * @pid:     pid of the process [zero for current task]
> > - * @head_ptr:        pointer to a list-head pointer, the kernel fills it in
> > - * @len_ptr: pointer to a length field, the kernel fills in the header size
> > - */
> > -SYSCALL_DEFINE3(get_robust_list, int, pid,
> > -             struct robust_list_head __user * __user *, head_ptr,
> > -             size_t __user *, len_ptr)
> > +static void __user *get_robust_list_common(int pid,
> > +             bool compat)
>
> What is this random line break for?
>
> >  {
> > -     struct robust_list_head __user *head;
> > +     void __user *head;
> >       unsigned long ret;
> > -     struct task_struct *p;
> >
>
> Stray new line and please use reverse fir tree ordering of variables:
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations
>
> > -     rcu_read_lock();
> > +     struct task_struct *p;
> >
> > -     ret = -ESRCH;
> > -     if (!pid)
> > +     if (!pid) {
> >               p = current;
> > -     else {
> > +             get_task_struct(p);
> > +     } else {
> > +             rcu_read_lock();
> >               p = find_task_by_vpid(pid);
> > +             /*
> > +              * pin the task to permit dropping the RCU read lock before
> > +              * acquiring the semaphore
> > +              */
> > +             if (p)
> > +                     get_task_struct(p);
> > +             rcu_read_unlock();
> >               if (!p)
> > -                     goto err_unlock;
> > +                     return ERR_PTR(-ESRCH);
>
>                 scoped_guard(rcu) {
>                      p = find_task_by_vpid(pid);
>                      if (!p)
>                         return (void __user *)ERR_PTR(-ESRCH);
>                      get_task_struct(p);
>                 }
>
> No need for a comment about pinning the task. This is obvious and a
> common pattern all over the place. And note the type case on the error
> return.
>
> But you can simplify this whole thing even further:
>
>         struct task_struct *p = current;
>
>         scoped_guard(rcu) {
>                 if (pid) {
>                      p = find_task_by_vpid(pid);
>                      if (!p)
>                         return (void __user *)ERR_PTR(-ESRCH);
>                 }
>                 get_task_struct(p);
>         }
>
> Yes, RCU is not required for the !pid case, but this is not a hot path.
>
> >
> > +     /*
> > +      * Hold exec_update_lock to serialize with concurrent exec()
> > +      * so ptrace_may_access() is checked against stable credentials
> > +      */
> > +
>
> Stray newline.
>
> > +     ret = down_read_killable(&p->signal->exec_update_lock);
> > +     if (ret)
> > +             goto err_put;
> > +
> >       ret = -EPERM;
> >       if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
> >               goto err_unlock;
> >
> > -     head = p->robust_list;
> > -     rcu_read_unlock();
> > +     if (compat)
> > +             head = p->compat_robust_list;
> > +     else
> > +             head = p->robust_list;
>
> Brain compiler complains about a build fail with CONFIG_COMPAT=n
>
> static inline void __user *task_robust_list(struct task_struct *p, bool compat)
> {
> #ifdef COMPAT
>         if (compat)
>                 return p->compat_robust_list;
> #endif
>         return p->robust_list;
> }
>
> So you don't have the #ifdef ugly in this function..
>
> > -     if (put_user(sizeof(*head), len_ptr))
> > -             return -EFAULT;
> > -     return put_user(head, head_ptr);
> > +     up_read(&p->signal->exec_update_lock);
> > +     put_task_struct(p);
> > +
> > +     return head;
> >
> >  err_unlock:
> > -     rcu_read_unlock();
> > +     up_read(&p->signal->exec_update_lock);
> > +err_put:
> > +     put_task_struct(p);
> > +     return ERR_PTR(ret);
> > +}
> >
> > -     return ret;
> > +
> > +/**
> > + * sys_get_robust_list() - Get the robust-futex list head of a task
> > + * @pid:     pid of the process [zero for current task]
> > + * @head_ptr:        pointer to a list-head pointer, the kernel fills it in
> > + * @len_ptr: pointer to a length field, the kernel fills in the header size
> > + */
> > +SYSCALL_DEFINE3(get_robust_list, int, pid,
> > +             struct robust_list_head __user * __user *, head_ptr,
> > +             size_t __user *, len_ptr)
> > +{
> > +     struct robust_list_head __user *head =
> > +             get_robust_list_common(pid, false);
>
> No line break required.
>
> > +     if (IS_ERR(head))
> > +             return PTR_ERR(head);
> > +
> > +     if (put_user(sizeof(*head), len_ptr))
> > +             return -EFAULT;
> > +     return put_user(head, head_ptr);
> >  }
> >
> >  long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
> > @@ -455,36 +490,15 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
> >                       compat_uptr_t __user *, head_ptr,
> >                       compat_size_t __user *, len_ptr)
> >  {
> > -     struct compat_robust_list_head __user *head;
> > -     unsigned long ret;
> > -     struct task_struct *p;
> > +     struct compat_robust_list_head __user *head =
> > +             get_robust_list_common(pid, true);
>
> Ditto
>
> Thanks,
>
>         tglx

Hi,

Thanks for pointing out the shortcomings in my patch. I will
resend a v3 for the same shortly.

Regards
Pranav Tyagi

Re: [PATCH v2] futex: don't leak robust_list pointer on exec race

[kernel test robot]

Hi Pranav,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/locking/core]
[also build test WARNING on linus/master v6.16 next-20250805]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Pranav-Tyagi/futex-don-t-leak-robust_list-pointer-on-exec-race/20250804-195739
base:   tip/locking/core
patch link:    https://lore.kernel.org/r/20250804115533.14186-1-pranav.tyagi03%40gmail.com
patch subject: [PATCH v2] futex: don't leak robust_list pointer on exec race
config: x86_64-randconfig-r132-20250805 (https://download.01.org/0day-ci/archive/20250806/202508060225.urZa7B2V-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250806/202508060225.urZa7B2V-lkp@intel.com/reproduce)

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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508060225.urZa7B2V-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> kernel/futex/syscalls.c:64:39: sparse: sparse: incorrect type in return expression (different address spaces) @@     expected void [noderef] __user * @@     got void * @@
   kernel/futex/syscalls.c:64:39: sparse:     expected void [noderef] __user *
   kernel/futex/syscalls.c:64:39: sparse:     got void *
   kernel/futex/syscalls.c:94:23: sparse: sparse: incorrect type in return expression (different address spaces) @@     expected void [noderef] __user * @@     got void * @@
   kernel/futex/syscalls.c:94:23: sparse:     expected void [noderef] __user *
   kernel/futex/syscalls.c:94:23: sparse:     got void *

vim +64 kernel/futex/syscalls.c

    41	
    42	static void __user *get_robust_list_common(int pid,
    43			bool compat)
    44	{
    45		void __user *head;
    46		unsigned long ret;
    47	
    48		struct task_struct *p;
    49	
    50		if (!pid) {
    51			p = current;
    52			get_task_struct(p);
    53		} else {
    54			rcu_read_lock();
    55			p = find_task_by_vpid(pid);
    56			/*
    57			 * pin the task to permit dropping the RCU read lock before
    58			 * acquiring the semaphore
    59			 */
    60			if (p)
    61				get_task_struct(p);
    62			rcu_read_unlock();
    63			if (!p)
  > 64				return ERR_PTR(-ESRCH);
    65		}
    66	
    67		/*
    68		 * Hold exec_update_lock to serialize with concurrent exec()
    69		 * so ptrace_may_access() is checked against stable credentials
    70		 */
    71	
    72		ret = down_read_killable(&p->signal->exec_update_lock);
    73		if (ret)
    74			goto err_put;
    75	
    76		ret = -EPERM;
    77		if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
    78			goto err_unlock;
    79	
    80		if (compat)
    81			head = p->compat_robust_list;
    82		else
    83			head = p->robust_list;
    84	
    85		up_read(&p->signal->exec_update_lock);
    86		put_task_struct(p);
    87	
    88		return head;
    89	
    90	err_unlock:
    91		up_read(&p->signal->exec_update_lock);
    92	err_put:
    93		put_task_struct(p);
    94		return ERR_PTR(ret);
    95	}
    96	

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