[PATCH] fs: hidepid: Fixes hidepid non dumpable behavior

[PATCH] fs: hidepid: Fixes hidepid non dumpable behavior

[nicolas.bouchinet]

From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>

The hidepid mount option documentation defines the following modes:

- "noaccess": user may not access any `/proc/<pid>/ directories but
  their own.
- "invisible": all `/proc/<pid>/` will be fully invisible to other users.
- "ptraceable": means that procfs should only contain `/proc/<pid>/`
  directories that the caller can ptrace.

We thus expect that with "noaccess" and "invisible" users would be able to
see their own processes in `/proc/<pid>/`.

The implementation of hidepid however control accesses using the
`ptrace_may_access()` function in any cases. Thus, if a process set
itself as non-dumpable using the `prctl(PR_SET_DUMPABLE,
SUID_DUMP_DISABLE)` it becomes invisible to the user.

This patch fixes the `has_pid_permissions()` function in order to make
its behavior to match the documentation.

Note that since `ptrace_may_access()` is not called anymore with
"noaccess" and "invisible", the `security_ptrace_access_check()` will no
longer be called either.

Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
---
 fs/proc/base.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index c667702dc69b8ca2531e88e12ed7a18533f294dd..fb128cb5f95fe65016fce96c75aee18c762a30f2 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -746,9 +746,12 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
 				 struct task_struct *task,
 				 enum proc_hidepid hide_pid_min)
 {
+	const struct cred *cred = current_cred(), *tcred;
+	kuid_t caller_uid;
+	kgid_t caller_gid;
 	/*
-	 * If 'hidpid' mount option is set force a ptrace check,
-	 * we indicate that we are using a filesystem syscall
+	 * If 'hidepid=ptraceable' mount option is set, force a ptrace check.
+	 * We indicate that we are using a filesystem syscall
 	 * by passing PTRACE_MODE_READ_FSCREDS
 	 */
 	if (fs_info->hide_pid == HIDEPID_NOT_PTRACEABLE)
@@ -758,7 +761,25 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
 		return true;
 	if (in_group_p(fs_info->pid_gid))
 		return true;
-	return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
+
+	task_lock(task);
+	rcu_read_lock();
+	caller_uid = cred->fsuid;
+	caller_gid = cred->fsgid;
+	tcred = __task_cred(task);
+	if (uid_eq(caller_uid, tcred->euid) &&
+	    uid_eq(caller_uid, tcred->suid) &&
+	    uid_eq(caller_uid, tcred->uid)  &&
+	    gid_eq(caller_gid, tcred->egid) &&
+	    gid_eq(caller_gid, tcred->sgid) &&
+	    gid_eq(caller_gid, tcred->gid)) {
+		rcu_read_unlock();
+		task_unlock(task);
+		return true;
+	}
+	rcu_read_unlock();
+	task_unlock(task);
+	return false;
 }
 
 

---
base-commit: 884a80cc9208ce75831b2376f2b0464018d7f2c4
change-id: 20250718-hidepid_fix-d0743d0540e7

Best regards,
-- 
Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>

Re: [PATCH] fs: hidepid: Fixes hidepid non dumpable behavior

[Nicolas Bouchinet]

Note that a yama patch has also been sent [1].

[1]: 
https://lore.kernel.org/all/20250718-yama_fix-v1-1-a51455359e67@ssi.gouv.fr/

Best regards,

Nicolas

On 7/18/25 10:47, nicolas.bouchinet@oss.cyber.gouv.fr wrote:
> From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
>
> The hidepid mount option documentation defines the following modes:
>
> - "noaccess": user may not access any `/proc/<pid>/ directories but
>    their own.
> - "invisible": all `/proc/<pid>/` will be fully invisible to other users.
> - "ptraceable": means that procfs should only contain `/proc/<pid>/`
>    directories that the caller can ptrace.
>
> We thus expect that with "noaccess" and "invisible" users would be able to
> see their own processes in `/proc/<pid>/`.
>
> The implementation of hidepid however control accesses using the
> `ptrace_may_access()` function in any cases. Thus, if a process set
> itself as non-dumpable using the `prctl(PR_SET_DUMPABLE,
> SUID_DUMP_DISABLE)` it becomes invisible to the user.
>
> This patch fixes the `has_pid_permissions()` function in order to make
> its behavior to match the documentation.
>
> Note that since `ptrace_may_access()` is not called anymore with
> "noaccess" and "invisible", the `security_ptrace_access_check()` will no
> longer be called either.
>
> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> ---
>   fs/proc/base.c | 27 ++++++++++++++++++++++++---
>   1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index c667702dc69b8ca2531e88e12ed7a18533f294dd..fb128cb5f95fe65016fce96c75aee18c762a30f2 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -746,9 +746,12 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
>   				 struct task_struct *task,
>   				 enum proc_hidepid hide_pid_min)
>   {
> +	const struct cred *cred = current_cred(), *tcred;
> +	kuid_t caller_uid;
> +	kgid_t caller_gid;
>   	/*
> -	 * If 'hidpid' mount option is set force a ptrace check,
> -	 * we indicate that we are using a filesystem syscall
> +	 * If 'hidepid=ptraceable' mount option is set, force a ptrace check.
> +	 * We indicate that we are using a filesystem syscall
>   	 * by passing PTRACE_MODE_READ_FSCREDS
>   	 */
>   	if (fs_info->hide_pid == HIDEPID_NOT_PTRACEABLE)
> @@ -758,7 +761,25 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
>   		return true;
>   	if (in_group_p(fs_info->pid_gid))
>   		return true;
> -	return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
> +
> +	task_lock(task);
> +	rcu_read_lock();
> +	caller_uid = cred->fsuid;
> +	caller_gid = cred->fsgid;
> +	tcred = __task_cred(task);
> +	if (uid_eq(caller_uid, tcred->euid) &&
> +	    uid_eq(caller_uid, tcred->suid) &&
> +	    uid_eq(caller_uid, tcred->uid)  &&
> +	    gid_eq(caller_gid, tcred->egid) &&
> +	    gid_eq(caller_gid, tcred->sgid) &&
> +	    gid_eq(caller_gid, tcred->gid)) {
> +		rcu_read_unlock();
> +		task_unlock(task);
> +		return true;
> +	}
> +	rcu_read_unlock();
> +	task_unlock(task);
> +	return false;
>   }
>   
>   
>
> ---
> base-commit: 884a80cc9208ce75831b2376f2b0464018d7f2c4
> change-id: 20250718-hidepid_fix-d0743d0540e7
>
> Best regards,

Re: [PATCH] fs: hidepid: Fixes hidepid non dumpable behavior

[Aleksa Sarai]

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

On 2025-07-18, nicolas.bouchinet@oss.cyber.gouv.fr <nicolas.bouchinet@oss.cyber.gouv.fr> wrote:
> From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> 
> The hidepid mount option documentation defines the following modes:
> 
> - "noaccess": user may not access any `/proc/<pid>/ directories but
>   their own.
> - "invisible": all `/proc/<pid>/` will be fully invisible to other users.
> - "ptraceable": means that procfs should only contain `/proc/<pid>/`
>   directories that the caller can ptrace.
> 
> We thus expect that with "noaccess" and "invisible" users would be able to
> see their own processes in `/proc/<pid>/`.
> 
> The implementation of hidepid however control accesses using the
> `ptrace_may_access()` function in any cases. Thus, if a process set
> itself as non-dumpable using the `prctl(PR_SET_DUMPABLE,
> SUID_DUMP_DISABLE)` it becomes invisible to the user.

In my view, the documentation is wrong here. This behaviour has remained
effectively unchanged since it was introduced in 0499680a4214 ("procfs:
add hidepid= and gid= mount options"), and the documentation was written
by the same author (added to Cc, though they appear to be inactive since
2013). hidepid=ptraceable was added many years later, and so the current
documentation seeming somewhat contradictory is probably more a result
of a new feature being documented without rewriting the old
documentation, rather than an incorrect implementation.

A process marking itself with SUID_DUMP_DISABLE is a *very* strong
signal that other processes (even processes owned by the same user) must
have very restricted access to it. Given how many times they have been
instrumental for protecting against attacks, I am quite hesitant about
making changes to loosen these restrictions.

For instance, container runtimes need to set SUID_DUMP_DISABLE to avoid
all sorts of breakout attacks (CVE-2016-9962 and CVE-2019-5736 being the
most famous examples, but there are plenty of others). If a container
has been configured with a restrictive hidepid, I would expect the
kernel to block most attempts to interact with such a process to
non-privileged users. But this patch would loosen said restrictions.

Now, many of the bits in /proc/self/* are additionally gated behind
ptrace_may_access() checks, so this kind of change might be less
catastrophic than at first glance, but the original concerns that
motivated hidepid= were about /proc/self/cmdline and the uid/euid of
processes being discoverable, and AFAICS this patch still undoes those
protections for the cases we care about with SUID_DUMP_DISABLE.

What motivated you to want to change this behaviour?

> This patch fixes the `has_pid_permissions()` function in order to make
> its behavior to match the documentation.
> 
> Note that since `ptrace_may_access()` is not called anymore with
> "noaccess" and "invisible", the `security_ptrace_access_check()` will no
> longer be called either.
> 
> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> ---
>  fs/proc/base.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index c667702dc69b8ca2531e88e12ed7a18533f294dd..fb128cb5f95fe65016fce96c75aee18c762a30f2 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -746,9 +746,12 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
>  				 struct task_struct *task,
>  				 enum proc_hidepid hide_pid_min)
>  {
> +	const struct cred *cred = current_cred(), *tcred;
> +	kuid_t caller_uid;
> +	kgid_t caller_gid;
>  	/*
> -	 * If 'hidpid' mount option is set force a ptrace check,
> -	 * we indicate that we are using a filesystem syscall
> +	 * If 'hidepid=ptraceable' mount option is set, force a ptrace check.
> +	 * We indicate that we are using a filesystem syscall
>  	 * by passing PTRACE_MODE_READ_FSCREDS
>  	 */
>  	if (fs_info->hide_pid == HIDEPID_NOT_PTRACEABLE)
> @@ -758,7 +761,25 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
>  		return true;
>  	if (in_group_p(fs_info->pid_gid))
>  		return true;
> -	return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
> +
> +	task_lock(task);
> +	rcu_read_lock();
> +	caller_uid = cred->fsuid;
> +	caller_gid = cred->fsgid;
> +	tcred = __task_cred(task);
> +	if (uid_eq(caller_uid, tcred->euid) &&
> +	    uid_eq(caller_uid, tcred->suid) &&
> +	    uid_eq(caller_uid, tcred->uid)  &&
> +	    gid_eq(caller_gid, tcred->egid) &&
> +	    gid_eq(caller_gid, tcred->sgid) &&
> +	    gid_eq(caller_gid, tcred->gid)) {
> +		rcu_read_unlock();
> +		task_unlock(task);
> +		return true;
> +	}
> +	rcu_read_unlock();
> +	task_unlock(task);
> +	return false;

At the very least, this check needs to be gated based on
ns_capable(get_task_mm(task)->user_ns, CAP_SYS_PTRACE), to avoid
containers from being able to introspect SUID_DUMP_DISABLE processes
(such as container runtimes) in the process of joining a user namespaced
container.

>  }
>  
>  
> 
> ---
> base-commit: 884a80cc9208ce75831b2376f2b0464018d7f2c4
> change-id: 20250718-hidepid_fix-d0743d0540e7
> 
> Best regards,
> -- 
> Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> 
> 

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] fs: hidepid: Fixes hidepid non dumpable behavior

[Aleksa Sarai]

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

On 2025-07-18, Aleksa Sarai <cyphar@cyphar.com> wrote:
> On 2025-07-18, nicolas.bouchinet@oss.cyber.gouv.fr <nicolas.bouchinet@oss.cyber.gouv.fr> wrote:
> > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > 
> > The hidepid mount option documentation defines the following modes:
> > 
> > - "noaccess": user may not access any `/proc/<pid>/ directories but
> >   their own.
> > - "invisible": all `/proc/<pid>/` will be fully invisible to other users.
> > - "ptraceable": means that procfs should only contain `/proc/<pid>/`
> >   directories that the caller can ptrace.
> > 
> > We thus expect that with "noaccess" and "invisible" users would be able to
> > see their own processes in `/proc/<pid>/`.
> > 
> > The implementation of hidepid however control accesses using the
> > `ptrace_may_access()` function in any cases. Thus, if a process set
> > itself as non-dumpable using the `prctl(PR_SET_DUMPABLE,
> > SUID_DUMP_DISABLE)` it becomes invisible to the user.
> 
> In my view, the documentation is wrong here. This behaviour has remained
> effectively unchanged since it was introduced in 0499680a4214 ("procfs:
> add hidepid= and gid= mount options"), and the documentation was written
> by the same author (added to Cc, though they appear to be inactive since
> 2013). hidepid=ptraceable was added many years later, and so the current
> documentation seeming somewhat contradictory is probably more a result
> of a new feature being documented without rewriting the old
> documentation, rather than an incorrect implementation.
> 
> A process marking itself with SUID_DUMP_DISABLE is a *very* strong
> signal that other processes (even processes owned by the same user) must
> have very restricted access to it. Given how many times they have been
> instrumental for protecting against attacks, I am quite hesitant about
> making changes to loosen these restrictions.
> 
> For instance, container runtimes need to set SUID_DUMP_DISABLE to avoid
> all sorts of breakout attacks (CVE-2016-9962 and CVE-2019-5736 being the
> most famous examples, but there are plenty of others). If a container
> has been configured with a restrictive hidepid, I would expect the
> kernel to block most attempts to interact with such a process to
> non-privileged users. But this patch would loosen said restrictions.
> 
> Now, many of the bits in /proc/self/* are additionally gated behind
> ptrace_may_access() checks, so this kind of change might be less
> catastrophic than at first glance, but the original concerns that
> motivated hidepid= were about /proc/self/cmdline and the uid/euid of
> processes being discoverable, and AFAICS this patch still undoes those
> protections for the cases we care about with SUID_DUMP_DISABLE.
> 
> What motivated you to want to change this behaviour?
> 
> > This patch fixes the `has_pid_permissions()` function in order to make
> > its behavior to match the documentation.
> > 
> > Note that since `ptrace_may_access()` is not called anymore with
> > "noaccess" and "invisible", the `security_ptrace_access_check()` will no
> > longer be called either.
> > 
> > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > ---
> >  fs/proc/base.c | 27 ++++++++++++++++++++++++---
> >  1 file changed, 24 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index c667702dc69b8ca2531e88e12ed7a18533f294dd..fb128cb5f95fe65016fce96c75aee18c762a30f2 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -746,9 +746,12 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
> >  				 struct task_struct *task,
> >  				 enum proc_hidepid hide_pid_min)
> >  {
> > +	const struct cred *cred = current_cred(), *tcred;
> > +	kuid_t caller_uid;
> > +	kgid_t caller_gid;
> >  	/*
> > -	 * If 'hidpid' mount option is set force a ptrace check,
> > -	 * we indicate that we are using a filesystem syscall
> > +	 * If 'hidepid=ptraceable' mount option is set, force a ptrace check.
> > +	 * We indicate that we are using a filesystem syscall
> >  	 * by passing PTRACE_MODE_READ_FSCREDS
> >  	 */
> >  	if (fs_info->hide_pid == HIDEPID_NOT_PTRACEABLE)
> > @@ -758,7 +761,25 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
> >  		return true;
> >  	if (in_group_p(fs_info->pid_gid))
> >  		return true;
> > -	return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
> > +
> > +	task_lock(task);
> > +	rcu_read_lock();
> > +	caller_uid = cred->fsuid;
> > +	caller_gid = cred->fsgid;
> > +	tcred = __task_cred(task);
> > +	if (uid_eq(caller_uid, tcred->euid) &&
> > +	    uid_eq(caller_uid, tcred->suid) &&
> > +	    uid_eq(caller_uid, tcred->uid)  &&
> > +	    gid_eq(caller_gid, tcred->egid) &&
> > +	    gid_eq(caller_gid, tcred->sgid) &&
> > +	    gid_eq(caller_gid, tcred->gid)) {
> > +		rcu_read_unlock();
> > +		task_unlock(task);
> > +		return true;
> > +	}
> > +	rcu_read_unlock();
> > +	task_unlock(task);
> > +	return false;
> 
> At the very least, this check needs to be gated based on
> ns_capable(get_task_mm(task)->user_ns, CAP_SYS_PTRACE), to avoid
> containers from being able to introspect SUID_DUMP_DISABLE processes
> (such as container runtimes) in the process of joining a user namespaced
> container.

Actually get_task_mm(test)->user_ns == current_user_ns() is probably
want you want? ns_capable(..., CAP_SYS_PTRACE) is basically an
equivalent of ptrace_may_access() here. But we very rarely do permission
checks based just on the userns -- we almost always use ns_capable()
since that actually handles the hierarchical relationship between user
namespaces regarding privileges.

Either way, I'm not a fan of this change, to be honest.

> 
> >  }
> >  
> >  
> > 
> > ---
> > base-commit: 884a80cc9208ce75831b2376f2b0464018d7f2c4
> > change-id: 20250718-hidepid_fix-d0743d0540e7
> > 
> > Best regards,
> > -- 
> > Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > 
> > 

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] fs: hidepid: Fixes hidepid non dumpable behavior

[Nicolas Bouchinet]

Hi Aleksa,

Thanks for your reply !

On Fri, Jul 18, 2025 at 07:36:37PM +1000, Aleksa Sarai wrote:
> On 2025-07-18, nicolas.bouchinet@oss.cyber.gouv.fr <nicolas.bouchinet@oss.cyber.gouv.fr> wrote:
> > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > 
> > The hidepid mount option documentation defines the following modes:
> > 
> > - "noaccess": user may not access any `/proc/<pid>/ directories but
> >   their own.
> > - "invisible": all `/proc/<pid>/` will be fully invisible to other users.
> > - "ptraceable": means that procfs should only contain `/proc/<pid>/`
> >   directories that the caller can ptrace.
> > 
> > We thus expect that with "noaccess" and "invisible" users would be able to
> > see their own processes in `/proc/<pid>/`.
> > 
> > The implementation of hidepid however control accesses using the
> > `ptrace_may_access()` function in any cases. Thus, if a process set
> > itself as non-dumpable using the `prctl(PR_SET_DUMPABLE,
> > SUID_DUMP_DISABLE)` it becomes invisible to the user.
> 
> In my view, the documentation is wrong here. This behaviour has remained
> effectively unchanged since it was introduced in 0499680a4214 ("procfs:
> add hidepid= and gid= mount options"), and the documentation was written
> by the same author (added to Cc, though they appear to be inactive since
> 2013). hidepid=ptraceable was added many years later, and so the current
> documentation seeming somewhat contradictory is probably more a result
> of a new feature being documented without rewriting the old
> documentation, rather than an incorrect implementation.

I'll change the documentation to match what it really does.
> 
> A process marking itself with SUID_DUMP_DISABLE is a *very* strong
> signal that other processes (even processes owned by the same user) must
> have very restricted access to it. Given how many times they have been
> instrumental for protecting against attacks, I am quite hesitant about
> making changes to loosen these restrictions.
> 
> For instance, container runtimes need to set SUID_DUMP_DISABLE to avoid
> all sorts of breakout attacks (CVE-2016-9962 and CVE-2019-5736 being the
> most famous examples, but there are plenty of others). If a container
> has been configured with a restrictive hidepid, I would expect the
> kernel to block most attempts to interact with such a process to
> non-privileged users. But this patch would loosen said restrictions.
> 
> Now, many of the bits in /proc/self/* are additionally gated behind
> ptrace_may_access() checks, so this kind of change might be less
> catastrophic than at first glance, but the original concerns that
> motivated hidepid= were about /proc/self/cmdline and the uid/euid of
> processes being discoverable, and AFAICS this patch still undoes those
> protections for the cases we care about with SUID_DUMP_DISABLE.
> 
> What motivated you to want to change this behaviour?
> 

Well, the change is motivated by two things, the first one is the fact
that the only difference between ("noaccess", "invisible") and
"ptraceable" is the verification of the "gid" hidepid parameter. Thus,
in anyway it means that only ptraceable process can be seen. 

The second motivation is that the "ptraceable" mode didn't worked with
the yama LSM, which doesn't care about `PTRACE_MODE_READ_FSCREDS` trace
mode. Thus, using hidepid "ptraceable" mode with yama "restricted",
"admin-only" or "no attach" modes doesn't do much.

I have submited a fix to yama [1] in order to make it take into account
`PTRACE_MODE_READ_FSCREDS` traces. With this yama patch, any hidepid
modes would have been affected by yama decision even though the hidepid
documentation claim that processes belonging to users are visible.

The combination of the two patches thus makes the "ptraceable" hidepid
mode work with yama without locking "noaccess" and "invisible" modes.

With hidepid "ptraceable" mode, `SUID_DUMP_DISABLE` process would be
invisible to the user.

[1]: https://lore.kernel.org/all/cf43bc15-e42d-4fde-a2b7-4fe832e177a8@oss.cyber.gouv.fr/

> > This patch fixes the `has_pid_permissions()` function in order to make
> > its behavior to match the documentation.
> > 
> > Note that since `ptrace_may_access()` is not called anymore with
> > "noaccess" and "invisible", the `security_ptrace_access_check()` will no
> > longer be called either.
> > 
> > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > ---
> >  fs/proc/base.c | 27 ++++++++++++++++++++++++---
> >  1 file changed, 24 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index c667702dc69b8ca2531e88e12ed7a18533f294dd..fb128cb5f95fe65016fce96c75aee18c762a30f2 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -746,9 +746,12 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
> >  				 struct task_struct *task,
> >  				 enum proc_hidepid hide_pid_min)
> >  {
> > +	const struct cred *cred = current_cred(), *tcred;
> > +	kuid_t caller_uid;
> > +	kgid_t caller_gid;
> >  	/*
> > -	 * If 'hidpid' mount option is set force a ptrace check,
> > -	 * we indicate that we are using a filesystem syscall
> > +	 * If 'hidepid=ptraceable' mount option is set, force a ptrace check.
> > +	 * We indicate that we are using a filesystem syscall
> >  	 * by passing PTRACE_MODE_READ_FSCREDS
> >  	 */
> >  	if (fs_info->hide_pid == HIDEPID_NOT_PTRACEABLE)
> > @@ -758,7 +761,25 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
> >  		return true;
> >  	if (in_group_p(fs_info->pid_gid))
> >  		return true;
> > -	return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
> > +
> > +	task_lock(task);
> > +	rcu_read_lock();
> > +	caller_uid = cred->fsuid;
> > +	caller_gid = cred->fsgid;
> > +	tcred = __task_cred(task);
> > +	if (uid_eq(caller_uid, tcred->euid) &&
> > +	    uid_eq(caller_uid, tcred->suid) &&
> > +	    uid_eq(caller_uid, tcred->uid)  &&
> > +	    gid_eq(caller_gid, tcred->egid) &&
> > +	    gid_eq(caller_gid, tcred->sgid) &&
> > +	    gid_eq(caller_gid, tcred->gid)) {
> > +		rcu_read_unlock();
> > +		task_unlock(task);
> > +		return true;
> > +	}
> > +	rcu_read_unlock();
> > +	task_unlock(task);
> > +	return false;
> 
> At the very least, this check needs to be gated based on
> ns_capable(get_task_mm(task)->user_ns, CAP_SYS_PTRACE), to avoid
> containers from being able to introspect SUID_DUMP_DISABLE processes
> (such as container runtimes) in the process of joining a user namespaced
> container.
> 

IIUC, you want to hide non-dumpable processes joining other user
namespaces to avoid the data exposition of the non-dumpable process in
`/proc/<pid>/*` ?

Like whats is done in `__ptrace_may_access()` :

```C
mm = task->mm;
if (mm &&
    ((get_dumpable(mm) != SUID_DUMP_USER) &&
     !ptrace_has_cap(mm->user_ns, mode)))
```

> >  }
> >  
> >  
> > 
> > ---
> > base-commit: 884a80cc9208ce75831b2376f2b0464018d7f2c4
> > change-id: 20250718-hidepid_fix-d0743d0540e7
> > 
> > Best regards,
> > -- 
> > Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > 
> > 
> 
> -- 
> Aleksa Sarai
> Senior Software Engineer (Containers)
> SUSE Linux GmbH
> https://www.cyphar.com/

Re: [PATCH] fs: hidepid: Fixes hidepid non dumpable behavior

[Jann Horn]

On Fri, Jul 18, 2025 at 10:47 AM <nicolas.bouchinet@oss.cyber.gouv.fr> wrote:
> The hidepid mount option documentation defines the following modes:
>
> - "noaccess": user may not access any `/proc/<pid>/ directories but
>   their own.
> - "invisible": all `/proc/<pid>/` will be fully invisible to other users.
> - "ptraceable": means that procfs should only contain `/proc/<pid>/`
>   directories that the caller can ptrace.
>
> We thus expect that with "noaccess" and "invisible" users would be able to
> see their own processes in `/proc/<pid>/`.

"their own" is very fuzzy and could be interpreted many ways.

> The implementation of hidepid however control accesses using the
> `ptrace_may_access()` function in any cases. Thus, if a process set
> itself as non-dumpable using the `prctl(PR_SET_DUMPABLE,
> SUID_DUMP_DISABLE)` it becomes invisible to the user.

As Aleksa said, a non-dumpable processes is essentially like a setuid
process (even if its UIDs match yours, it may have some remaining
special privileges you don't have), so it's not really "your own".

> This patch fixes the `has_pid_permissions()` function in order to make
> its behavior to match the documentation.

I don't think "it doesn't match the documentation" is good enough
reason to change how the kernel works.

> Note that since `ptrace_may_access()` is not called anymore with
> "noaccess" and "invisible", the `security_ptrace_access_check()` will no
> longer be called either.
>
> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> ---
>  fs/proc/base.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index c667702dc69b8ca2531e88e12ed7a18533f294dd..fb128cb5f95fe65016fce96c75aee18c762a30f2 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -746,9 +746,12 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
>                                  struct task_struct *task,
>                                  enum proc_hidepid hide_pid_min)
>  {
> +       const struct cred *cred = current_cred(), *tcred;
> +       kuid_t caller_uid;
> +       kgid_t caller_gid;
>         /*
> -        * If 'hidpid' mount option is set force a ptrace check,
> -        * we indicate that we are using a filesystem syscall
> +        * If 'hidepid=ptraceable' mount option is set, force a ptrace check.
> +        * We indicate that we are using a filesystem syscall
>          * by passing PTRACE_MODE_READ_FSCREDS
>          */
>         if (fs_info->hide_pid == HIDEPID_NOT_PTRACEABLE)
> @@ -758,7 +761,25 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
>                 return true;
>         if (in_group_p(fs_info->pid_gid))
>                 return true;
> -       return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
> +
> +       task_lock(task);
> +       rcu_read_lock();
> +       caller_uid = cred->fsuid;
> +       caller_gid = cred->fsgid;
> +       tcred = __task_cred(task);
> +       if (uid_eq(caller_uid, tcred->euid) &&
> +           uid_eq(caller_uid, tcred->suid) &&
> +           uid_eq(caller_uid, tcred->uid)  &&
> +           gid_eq(caller_gid, tcred->egid) &&
> +           gid_eq(caller_gid, tcred->sgid) &&
> +           gid_eq(caller_gid, tcred->gid)) {
> +               rcu_read_unlock();
> +               task_unlock(task);
> +               return true;
> +       }
> +       rcu_read_unlock();
> +       task_unlock(task);
> +       return false;
>  }

I think this is a bad idea for several reasons:

1. It duplicates existing logic.
2. I think it prevents a process with euid!=ruid from introspecting
itself through procfs.
3. I think it prevents root from viewing all processes through procfs.
4. It allows processes to view metadata about each other when that was
previously blocked by the combination of hidepid and an LSM such as
Landlock or SELinux.
5. It ignores capabilities held by the introspected process but not
the process doing the introspection (which is currently checked by
cap_ptrace_access_check()).

What's the background here - do you have a specific usecase that
motivated this patch?

Re: [PATCH] fs: hidepid: Fixes hidepid non dumpable behavior

[Nicolas Bouchinet]

Hi Jann, thanks for your review !

On Fri, Jul 18, 2025 at 04:45:15PM +0200, Jann Horn wrote:
> On Fri, Jul 18, 2025 at 10:47 AM <nicolas.bouchinet@oss.cyber.gouv.fr> wrote:
> > The hidepid mount option documentation defines the following modes:
> >
> > - "noaccess": user may not access any `/proc/<pid>/ directories but
> >   their own.
> > - "invisible": all `/proc/<pid>/` will be fully invisible to other users.
> > - "ptraceable": means that procfs should only contain `/proc/<pid>/`
> >   directories that the caller can ptrace.
> >
> > We thus expect that with "noaccess" and "invisible" users would be able to
> > see their own processes in `/proc/<pid>/`.
> 
> "their own" is very fuzzy and could be interpreted many ways.
> 
> > The implementation of hidepid however control accesses using the
> > `ptrace_may_access()` function in any cases. Thus, if a process set
> > itself as non-dumpable using the `prctl(PR_SET_DUMPABLE,
> > SUID_DUMP_DISABLE)` it becomes invisible to the user.
> 
> As Aleksa said, a non-dumpable processes is essentially like a setuid
> process (even if its UIDs match yours, it may have some remaining
> special privileges you don't have), so it's not really "your own".
> 

Also replying to  :

> What's the background here - do you have a specific usecase that
> motivated this patch?

The case I encountered is using the zathura-sandbox pdf viewer which
sandboxes itself with Landlock and set itself as non-dumpable.
If my PDF viewer freezes and I want to kill it as an unprivileged user,
I'm not able to get its PID from `/proc` since its fully invisible to my
user.

> > This patch fixes the `has_pid_permissions()` function in order to make
> > its behavior to match the documentation.
> 
> I don't think "it doesn't match the documentation" is good enough
> reason to change how the kernel works.
> 
> > Note that since `ptrace_may_access()` is not called anymore with
> > "noaccess" and "invisible", the `security_ptrace_access_check()` will no
> > longer be called either.
> >
> > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > ---
> >  fs/proc/base.c | 27 ++++++++++++++++++++++++---
> >  1 file changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index c667702dc69b8ca2531e88e12ed7a18533f294dd..fb128cb5f95fe65016fce96c75aee18c762a30f2 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -746,9 +746,12 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
> >                                  struct task_struct *task,
> >                                  enum proc_hidepid hide_pid_min)
> >  {
> > +       const struct cred *cred = current_cred(), *tcred;
> > +       kuid_t caller_uid;
> > +       kgid_t caller_gid;
> >         /*
> > -        * If 'hidpid' mount option is set force a ptrace check,
> > -        * we indicate that we are using a filesystem syscall
> > +        * If 'hidepid=ptraceable' mount option is set, force a ptrace check.
> > +        * We indicate that we are using a filesystem syscall
> >          * by passing PTRACE_MODE_READ_FSCREDS
> >          */
> >         if (fs_info->hide_pid == HIDEPID_NOT_PTRACEABLE)
> > @@ -758,7 +761,25 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
> >                 return true;
> >         if (in_group_p(fs_info->pid_gid))
> >                 return true;
> > -       return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
> > +
> > +       task_lock(task);
> > +       rcu_read_lock();
> > +       caller_uid = cred->fsuid;
> > +       caller_gid = cred->fsgid;
> > +       tcred = __task_cred(task);
> > +       if (uid_eq(caller_uid, tcred->euid) &&
> > +           uid_eq(caller_uid, tcred->suid) &&
> > +           uid_eq(caller_uid, tcred->uid)  &&
> > +           gid_eq(caller_gid, tcred->egid) &&
> > +           gid_eq(caller_gid, tcred->sgid) &&
> > +           gid_eq(caller_gid, tcred->gid)) {
> > +               rcu_read_unlock();
> > +               task_unlock(task);
> > +               return true;
> > +       }
> > +       rcu_read_unlock();
> > +       task_unlock(task);
> > +       return false;
> >  }
> 
> I think this is a bad idea for several reasons:
> 
> 1. It duplicates existing logic.
I open to work on that.

> 2. I think it prevents a process with euid!=ruid from introspecting
> itself through procfs.
Great question, I'll test that and write some hidepid tests to check that.

> 3. I think it prevents root from viewing all processes through procfs.
Yes only if combined with yama="no attach", and IMHO, that would make sense.

> 4. It allows processes to view metadata about each other when that was
> previously blocked by the combination of hidepid and an LSM such as
> Landlock or SELinux.
Arf, you're absolutely right about this, my bad.

> 5. It ignores capabilities held by the introspected process but not
> the process doing the introspection (which is currently checked by
> cap_ptrace_access_check()).
As suggested by Aleksa, I can add some capabilities checks here.

> 
> What's the background here - do you have a specific usecase that
> motivated this patch?

The second motivation is that the "ptraceable" mode didn't worked with
the yama LSM, which doesn't care about `PTRACE_MODE_READ_FSCREDS` trace
mode. Thus, using hidepid "ptraceable" mode with yama "restricted",
"admin-only" or "no attach" modes doesn't do much.

As you have seen, I also have submited a fix to yama in order to make it
take into account `PTRACE_MODE_READ_FSCREDS` traces.

I have to admit I'm not really found of the fact that those two patch
are so tightly linked.

Thanks again for your review,

Nicolas

Re: [PATCH] fs: hidepid: Fixes hidepid non dumpable behavior

[Jann Horn]

On Fri, Jul 18, 2025 at 5:47 PM Nicolas Bouchinet
<nicolas.bouchinet@oss.cyber.gouv.fr> wrote:
> Hi Jann, thanks for your review !
>
> On Fri, Jul 18, 2025 at 04:45:15PM +0200, Jann Horn wrote:
> > On Fri, Jul 18, 2025 at 10:47 AM <nicolas.bouchinet@oss.cyber.gouv.fr> wrote:
> > > The hidepid mount option documentation defines the following modes:
> > >
> > > - "noaccess": user may not access any `/proc/<pid>/ directories but
> > >   their own.
> > > - "invisible": all `/proc/<pid>/` will be fully invisible to other users.
> > > - "ptraceable": means that procfs should only contain `/proc/<pid>/`
> > >   directories that the caller can ptrace.
> > >
> > > We thus expect that with "noaccess" and "invisible" users would be able to
> > > see their own processes in `/proc/<pid>/`.
> >
> > "their own" is very fuzzy and could be interpreted many ways.
> >
> > > The implementation of hidepid however control accesses using the
> > > `ptrace_may_access()` function in any cases. Thus, if a process set
> > > itself as non-dumpable using the `prctl(PR_SET_DUMPABLE,
> > > SUID_DUMP_DISABLE)` it becomes invisible to the user.
> >
> > As Aleksa said, a non-dumpable processes is essentially like a setuid
> > process (even if its UIDs match yours, it may have some remaining
> > special privileges you don't have), so it's not really "your own".
> >
>
> Also replying to  :
>
> > What's the background here - do you have a specific usecase that
> > motivated this patch?
>
> The case I encountered is using the zathura-sandbox pdf viewer which
> sandboxes itself with Landlock and set itself as non-dumpable.

It kind of sounds like an issue with your PDF viewer if that just sets
the non-dumpable flag for no reason...

> If my PDF viewer freezes and I want to kill it as an unprivileged user,
> I'm not able to get its PID from `/proc` since its fully invisible to my
> user.
>
> > > This patch fixes the `has_pid_permissions()` function in order to make
> > > its behavior to match the documentation.
> >
> > I don't think "it doesn't match the documentation" is good enough
> > reason to change how the kernel works.
> >
> > > Note that since `ptrace_may_access()` is not called anymore with
> > > "noaccess" and "invisible", the `security_ptrace_access_check()` will no
> > > longer be called either.
> > >
> > > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > > ---
> > >  fs/proc/base.c | 27 ++++++++++++++++++++++++---
> > >  1 file changed, 24 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > > index c667702dc69b8ca2531e88e12ed7a18533f294dd..fb128cb5f95fe65016fce96c75aee18c762a30f2 100644
> > > --- a/fs/proc/base.c
> > > +++ b/fs/proc/base.c
> > > @@ -746,9 +746,12 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
> > >                                  struct task_struct *task,
> > >                                  enum proc_hidepid hide_pid_min)
> > >  {
> > > +       const struct cred *cred = current_cred(), *tcred;
> > > +       kuid_t caller_uid;
> > > +       kgid_t caller_gid;
> > >         /*
> > > -        * If 'hidpid' mount option is set force a ptrace check,
> > > -        * we indicate that we are using a filesystem syscall
> > > +        * If 'hidepid=ptraceable' mount option is set, force a ptrace check.
> > > +        * We indicate that we are using a filesystem syscall
> > >          * by passing PTRACE_MODE_READ_FSCREDS
> > >          */
> > >         if (fs_info->hide_pid == HIDEPID_NOT_PTRACEABLE)
> > > @@ -758,7 +761,25 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
> > >                 return true;
> > >         if (in_group_p(fs_info->pid_gid))
> > >                 return true;
> > > -       return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
> > > +
> > > +       task_lock(task);
> > > +       rcu_read_lock();
> > > +       caller_uid = cred->fsuid;
> > > +       caller_gid = cred->fsgid;
> > > +       tcred = __task_cred(task);
> > > +       if (uid_eq(caller_uid, tcred->euid) &&
> > > +           uid_eq(caller_uid, tcred->suid) &&
> > > +           uid_eq(caller_uid, tcred->uid)  &&
> > > +           gid_eq(caller_gid, tcred->egid) &&
> > > +           gid_eq(caller_gid, tcred->sgid) &&
> > > +           gid_eq(caller_gid, tcred->gid)) {
> > > +               rcu_read_unlock();
> > > +               task_unlock(task);
> > > +               return true;
> > > +       }
> > > +       rcu_read_unlock();
> > > +       task_unlock(task);
> > > +       return false;
> > >  }
> >
> > I think this is a bad idea for several reasons:
> >
> > 1. It duplicates existing logic.
> I open to work on that.
>
> > 2. I think it prevents a process with euid!=ruid from introspecting
> > itself through procfs.
> Great question, I'll test that and write some hidepid tests to check that.
>
> > 3. I think it prevents root from viewing all processes through procfs.
> Yes only if combined with yama="no attach", and IMHO, that would make sense.

Why only if combined with yama? Doesn't your code always "return
false" on a UID/GID mismatch?

> > 4. It allows processes to view metadata about each other when that was
> > previously blocked by the combination of hidepid and an LSM such as
> > Landlock or SELinux.
> Arf, you're absolutely right about this, my bad.
>
> > 5. It ignores capabilities held by the introspected process but not
> > the process doing the introspection (which is currently checked by
> > cap_ptrace_access_check()).
> As suggested by Aleksa, I can add some capabilities checks here.
>
> >
> > What's the background here - do you have a specific usecase that
> > motivated this patch?
>
> The second motivation is that the "ptraceable" mode didn't worked with
> the yama LSM, which doesn't care about `PTRACE_MODE_READ_FSCREDS` trace
> mode. Thus, using hidepid "ptraceable" mode with yama "restricted",
> "admin-only" or "no attach" modes doesn't do much.
>
> As you have seen, I also have submited a fix to yama in order to make it
> take into account `PTRACE_MODE_READ_FSCREDS` traces.

I don't think that's really a fix - that's more of a new feature
you're proposing. Yama currently explicitly only restricts ATTACH-mode
ptrace access (which can read all process memory or modify the state
of a process), and it doesn't restrict less invasive introspection
that uses READ-mode ptrace checks.

> I have to admit I'm not really found of the fact that those two patch
> are so tightly linked.
>
> Thanks again for your review,
>
> Nicolas

Re: [PATCH] fs: hidepid: Fixes hidepid non dumpable behavior

[Nicolas Bouchinet]

On Fri, Jul 18, 2025 at 06:48:54PM +0200, Jann Horn wrote:
> On Fri, Jul 18, 2025 at 5:47 PM Nicolas Bouchinet
> <nicolas.bouchinet@oss.cyber.gouv.fr> wrote:
> > Hi Jann, thanks for your review !
> >
> > On Fri, Jul 18, 2025 at 04:45:15PM +0200, Jann Horn wrote:
> > > On Fri, Jul 18, 2025 at 10:47 AM <nicolas.bouchinet@oss.cyber.gouv.fr> wrote:
> > > > The hidepid mount option documentation defines the following modes:
> > > >
> > > > - "noaccess": user may not access any `/proc/<pid>/ directories but
> > > >   their own.
> > > > - "invisible": all `/proc/<pid>/` will be fully invisible to other users.
> > > > - "ptraceable": means that procfs should only contain `/proc/<pid>/`
> > > >   directories that the caller can ptrace.
> > > >
> > > > We thus expect that with "noaccess" and "invisible" users would be able to
> > > > see their own processes in `/proc/<pid>/`.
> > >
> > > "their own" is very fuzzy and could be interpreted many ways.
> > >
> > > > The implementation of hidepid however control accesses using the
> > > > `ptrace_may_access()` function in any cases. Thus, if a process set
> > > > itself as non-dumpable using the `prctl(PR_SET_DUMPABLE,
> > > > SUID_DUMP_DISABLE)` it becomes invisible to the user.
> > >
> > > As Aleksa said, a non-dumpable processes is essentially like a setuid
> > > process (even if its UIDs match yours, it may have some remaining
> > > special privileges you don't have), so it's not really "your own".
> > >
> >
> > Also replying to  :
> >
> > > What's the background here - do you have a specific usecase that
> > > motivated this patch?
> >
> > The case I encountered is using the zathura-sandbox pdf viewer which
> > sandboxes itself with Landlock and set itself as non-dumpable.
> 
> It kind of sounds like an issue with your PDF viewer if that just sets
> the non-dumpable flag for no reason...
> 
> > If my PDF viewer freezes and I want to kill it as an unprivileged user,
> > I'm not able to get its PID from `/proc` since its fully invisible to my
> > user.
> >
> > > > This patch fixes the `has_pid_permissions()` function in order to make
> > > > its behavior to match the documentation.
> > >
> > > I don't think "it doesn't match the documentation" is good enough
> > > reason to change how the kernel works.
> > >
> > > > Note that since `ptrace_may_access()` is not called anymore with
> > > > "noaccess" and "invisible", the `security_ptrace_access_check()` will no
> > > > longer be called either.
> > > >
> > > > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > > > ---
> > > >  fs/proc/base.c | 27 ++++++++++++++++++++++++---
> > > >  1 file changed, 24 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > > > index c667702dc69b8ca2531e88e12ed7a18533f294dd..fb128cb5f95fe65016fce96c75aee18c762a30f2 100644
> > > > --- a/fs/proc/base.c
> > > > +++ b/fs/proc/base.c
> > > > @@ -746,9 +746,12 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
> > > >                                  struct task_struct *task,
> > > >                                  enum proc_hidepid hide_pid_min)
> > > >  {
> > > > +       const struct cred *cred = current_cred(), *tcred;
> > > > +       kuid_t caller_uid;
> > > > +       kgid_t caller_gid;
> > > >         /*
> > > > -        * If 'hidpid' mount option is set force a ptrace check,
> > > > -        * we indicate that we are using a filesystem syscall
> > > > +        * If 'hidepid=ptraceable' mount option is set, force a ptrace check.
> > > > +        * We indicate that we are using a filesystem syscall
> > > >          * by passing PTRACE_MODE_READ_FSCREDS
> > > >          */
> > > >         if (fs_info->hide_pid == HIDEPID_NOT_PTRACEABLE)
> > > > @@ -758,7 +761,25 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
> > > >                 return true;
> > > >         if (in_group_p(fs_info->pid_gid))
> > > >                 return true;
> > > > -       return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
> > > > +
> > > > +       task_lock(task);
> > > > +       rcu_read_lock();
> > > > +       caller_uid = cred->fsuid;
> > > > +       caller_gid = cred->fsgid;
> > > > +       tcred = __task_cred(task);
> > > > +       if (uid_eq(caller_uid, tcred->euid) &&
> > > > +           uid_eq(caller_uid, tcred->suid) &&
> > > > +           uid_eq(caller_uid, tcred->uid)  &&
> > > > +           gid_eq(caller_gid, tcred->egid) &&
> > > > +           gid_eq(caller_gid, tcred->sgid) &&
> > > > +           gid_eq(caller_gid, tcred->gid)) {
> > > > +               rcu_read_unlock();
> > > > +               task_unlock(task);
> > > > +               return true;
> > > > +       }
> > > > +       rcu_read_unlock();
> > > > +       task_unlock(task);
> > > > +       return false;
> > > >  }
> > >
> > > I think this is a bad idea for several reasons:
> > >
> > > 1. It duplicates existing logic.
> > I open to work on that.
> >
> > > 2. I think it prevents a process with euid!=ruid from introspecting
> > > itself through procfs.
> > Great question, I'll test that and write some hidepid tests to check that.
> >
> > > 3. I think it prevents root from viewing all processes through procfs.
> > Yes only if combined with yama="no attach", and IMHO, that would make sense.
> 
> Why only if combined with yama? Doesn't your code always "return
> false" on a UID/GID mismatch?
> 
Ah yes indeed, if the gid=1000 mount option is used, processes become
invisible to root since the cap_sys_ptrace check is not called anymore.

I understand now it would need almost all of `ptrace_may_access()`
checks and thus will lead to the same exact behavior. So useless patch.

> > > 4. It allows processes to view metadata about each other when that was
> > > previously blocked by the combination of hidepid and an LSM such as
> > > Landlock or SELinux.
> > Arf, you're absolutely right about this, my bad.
> >
> > > 5. It ignores capabilities held by the introspected process but not
> > > the process doing the introspection (which is currently checked by
> > > cap_ptrace_access_check()).
> > As suggested by Aleksa, I can add some capabilities checks here.
> >
> > >
> > > What's the background here - do you have a specific usecase that
> > > motivated this patch?
> >
> > The second motivation is that the "ptraceable" mode didn't worked with
> > the yama LSM, which doesn't care about `PTRACE_MODE_READ_FSCREDS` trace
> > mode. Thus, using hidepid "ptraceable" mode with yama "restricted",
> > "admin-only" or "no attach" modes doesn't do much.
> >
> > As you have seen, I also have submited a fix to yama in order to make it
> > take into account `PTRACE_MODE_READ_FSCREDS` traces.
> 
> I don't think that's really a fix - that's more of a new feature
> you're proposing. Yama currently explicitly only restricts ATTACH-mode
> ptrace access (which can read all process memory or modify the state
> of a process), and it doesn't restrict less invasive introspection
> that uses READ-mode ptrace checks.
> 
Forget the patch sorry for that, I'll update hidepid documentation in
order to describe the exact behavior it have. Will send a V2.

> > I have to admit I'm not really found of the fact that those two patch
> > are so tightly linked.
> >
> > Thanks again for your review,
> >
> > Nicolas

Re: [PATCH] fs: hidepid: Fixes hidepid non dumpable behavior

[Nicolas Bouchinet]

Hi Jann,

While documenting "hidepid=" I encountered those clunky behavior, which
I think we should not have.

Let say we set the "hidepid=" option to "invisible", processes will be fully
invisible to other users than root and the user that reads the procfs entry.

The fact that root is able to see every processes is partialy due to the fact
that the "gid=" variable is set to "0" by default :

```C has_pid_permissions
[...]
	if (in_group_p(fs_info->pid_gid))
		return true;
[...]
```

This means that if a process GID is in the group defined by the "gid=" option,
an authorization is directly returned, and the `ptrace_may_access()` function
is never called.

Thus, if one sets the "gid=" option to "1000", if a process is in this group,
it will now bypass the `security_ptrace_access_check()` hook calls.

This comes with the side effect that now, root will go through the
`ptrace_may_access()` checks and thus, if I have a root process without the
cap_sys_ptrace in its effective set, it will not be able to see other processes
anymore.

```C __ptrace_may_access
[...]
	if (ptrace_has_cap(tcred->user_ns, mode))
		goto ok;
	rcu_read_unlock();
	return -EPERM;
[...]
```

The following behavior thus happens :

```bash
$ sudo capsh --user=root --drop=cap_sys_ptrace -- -c /bin/bash
# mount -o remount,hidepid=2 /proc
# getpcaps $$
=ep cap_sys_ptrace-ep
# ps aux
root         1  0.0  0.1 204724  1404 ?        S    09:43   0:00 /usr/lib/python3.13/site-packages/virtme/guest/bin/virtme-ng-init
root         2  0.0  0.0      0     0 ?        S    09:43   0:00 [kthreadd]
root         3  0.0  0.0      0     0 ?        S    09:43   0:00 [pool_workqueue_release]
[...]

# mount -o remount,hidepid=2,gid=1000 /proc
# getpcaps $$
=ep cap_sys_ptrace-ep
# ps aux
root       621  0.0  0.2   8656  2556 pts/0    S    09:46   0:00 /bin/bash
root       641  0.0  0.4   9592  4012 pts/0    R+   10:03   0:00 ps aux
root       642  0.0  0.2   6896  2452 pts/0    S+   10:03   0:00 less
```

This also means that if a process accesses were controled by an LSM, lets say
with the `audit ptrace` option of AppArmor, they magically disappears for the
process in the group defined in the "gid=" hidepid option and those controls
are "transfered" to root, which was not controlled at first.

Also, note that with "hidepid=ptraceable", the "gid=" option is ignored
and `ptrace_may_access()` is always called.

Shouldn't we always check for the "gid=0" and also call the
`security_ptrace_access_check()` even if the group is set ?

Best regards,

Nicolas