public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] fs/pidfs: Add permission check to pidfd_info()
@ 2026-02-06 18:02 danieldurning.work
  2026-02-09 14:01 ` Christian Brauner
  0 siblings, 1 reply; 6+ messages in thread
From: danieldurning.work @ 2026-02-06 18:02 UTC (permalink / raw)
  To: linux-fsdevel, linux-security-module, selinux
  Cc: viro, brauner, jack, paul, stephen.smalley.work, omosnace

From: Daniel Durning <danieldurning.work@gmail.com>

Added a permission check to pidfd_info(). Originally, process info
could be retrieved with a pidfd even if proc was mounted with hidepid
enabled, allowing pidfds to be used to bypass those protections. We
now call ptrace_may_access() to perform some DAC checking as well
as call the appropriate LSM hook.

The downside to this approach is that there are now more restrictions
on accessing this info from a pidfd than when just using proc (without
hidepid). I am open to suggestions if anyone can think of a better way
to handle this.

I have also noticed that it is possible to use pidfds to poll on any
process regardless of whether the process is a child of the caller,
has a different UID, or has a different security context. Is this
also worth addressing? If so, what exactly should the DAC checks be?

Signed-off-by: Daniel Durning <danieldurning.work@gmail.com>
---
 fs/pidfs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/pidfs.c b/fs/pidfs.c
index dba703d4ce4a..058a7d798bca 100644
--- a/fs/pidfs.c
+++ b/fs/pidfs.c
@@ -365,6 +365,13 @@ static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg)
 		goto copy_out;
 	}
 
+	/*
+	 * Do a filesystem cred ptrace check to verify access
+	 * to the task's info.
+	 */
+	if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS))
+		return -EACCES;
+
 	c = get_task_cred(task);
 	if (!c)
 		return -ESRCH;
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] fs/pidfs: Add permission check to pidfd_info()
  2026-02-06 18:02 [RFC PATCH] fs/pidfs: Add permission check to pidfd_info() danieldurning.work
@ 2026-02-09 14:01 ` Christian Brauner
  2026-02-11 19:43   ` Daniel Durning
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Brauner @ 2026-02-09 14:01 UTC (permalink / raw)
  To: danieldurning.work
  Cc: linux-fsdevel, linux-security-module, selinux, viro, jack, paul,
	stephen.smalley.work, omosnace, Oleg Nesterov

On Fri, Feb 06, 2026 at 06:02:48PM +0000, danieldurning.work@gmail.com wrote:
> From: Daniel Durning <danieldurning.work@gmail.com>
> 
> Added a permission check to pidfd_info(). Originally, process info
> could be retrieved with a pidfd even if proc was mounted with hidepid
> enabled, allowing pidfds to be used to bypass those protections. We
> now call ptrace_may_access() to perform some DAC checking as well
> as call the appropriate LSM hook.
> 
> The downside to this approach is that there are now more restrictions
> on accessing this info from a pidfd than when just using proc (without
> hidepid). I am open to suggestions if anyone can think of a better way
> to handle this.

This isn't really workable since this would regress userspace quite a
bit. I think we need a different approach. I've given it some thought
and everything's kinda ugly but this might work.

In struct pid_namespace record whether anyone ever mounted a procfs
with hidepid turned on for this pidns. In pidfd_info() we check whether
hidepid was ever turned on. If it wasn't we're done and can just return
the info. This will be the common case. If hidepid was ever turned on
use kern_path("/proc") to lookup procfs. If not found check
ptrace_may_access() to decide whether to return the info or not. If
/proc is found check it's hidepid settings and make a decision based on
that.

You can probably reorder this to call ptrace_may_access() first and then
do the procfs lookup dance. Thoughts?

> I have also noticed that it is possible to use pidfds to poll on any
> process regardless of whether the process is a child of the caller,
> has a different UID, or has a different security context. Is this
> also worth addressing? If so, what exactly should the DAC checks be?

Oleg and I had discusses this and decided that such polling isn't
sensitive information so by default this should just work and it's
relied upon in Android and in a bunch of other workloads. An LSM can of
course restrict access via security_file_ioctl().

Fwiw, pidfds now support persistent trusted extended attributes so if
the LSM folks wanted we can add security.* extended attribute support
and they can mark pidfds with persistent security labels - persistent as
in for the lifetime of the task.

> Signed-off-by: Daniel Durning <danieldurning.work@gmail.com>
> ---
>  fs/pidfs.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/pidfs.c b/fs/pidfs.c
> index dba703d4ce4a..058a7d798bca 100644
> --- a/fs/pidfs.c
> +++ b/fs/pidfs.c
> @@ -365,6 +365,13 @@ static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg)
>  		goto copy_out;
>  	}
>  
> +	/*
> +	 * Do a filesystem cred ptrace check to verify access
> +	 * to the task's info.
> +	 */
> +	if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS))
> +		return -EACCES;
> +
>  	c = get_task_cred(task);
>  	if (!c)
>  		return -ESRCH;
> -- 
> 2.52.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] fs/pidfs: Add permission check to pidfd_info()
  2026-02-09 14:01 ` Christian Brauner
@ 2026-02-11 19:43   ` Daniel Durning
  2026-02-17 12:01     ` Christian Brauner
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Durning @ 2026-02-11 19:43 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-fsdevel, linux-security-module, selinux, viro, jack, paul,
	stephen.smalley.work, omosnace, Oleg Nesterov

On Mon, Feb 9, 2026 at 9:01 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Fri, Feb 06, 2026 at 06:02:48PM +0000, danieldurning.work@gmail.com wrote:
> > From: Daniel Durning <danieldurning.work@gmail.com>
> >
> > Added a permission check to pidfd_info(). Originally, process info
> > could be retrieved with a pidfd even if proc was mounted with hidepid
> > enabled, allowing pidfds to be used to bypass those protections. We
> > now call ptrace_may_access() to perform some DAC checking as well
> > as call the appropriate LSM hook.
> >
> > The downside to this approach is that there are now more restrictions
> > on accessing this info from a pidfd than when just using proc (without
> > hidepid). I am open to suggestions if anyone can think of a better way
> > to handle this.
>
> This isn't really workable since this would regress userspace quite a
> bit. I think we need a different approach. I've given it some thought
> and everything's kinda ugly but this might work.
>
> In struct pid_namespace record whether anyone ever mounted a procfs
> with hidepid turned on for this pidns. In pidfd_info() we check whether
> hidepid was ever turned on. If it wasn't we're done and can just return
> the info. This will be the common case. If hidepid was ever turned on
> use kern_path("/proc") to lookup procfs. If not found check
> ptrace_may_access() to decide whether to return the info or not. If
> /proc is found check it's hidepid settings and make a decision based on
> that.
>
> You can probably reorder this to call ptrace_may_access() first and then
> do the procfs lookup dance. Thoughts?

Thanks for the feedback. I think your solution makes sense.

Unfortunately, it seems like systemd mounts procfs with hidepid enabled on
boot for services with the ProtectProc option enabled. This means that
procfs will always have been mounted with hidepid in the init pid namespace.
Do you think it would be viable to record whether or not procfs was mounted
with hidepid enabled in the mount namespace instead?

> > I have also noticed that it is possible to use pidfds to poll on any
> > process regardless of whether the process is a child of the caller,
> > has a different UID, or has a different security context. Is this
> > also worth addressing? If so, what exactly should the DAC checks be?
>
> Oleg and I had discusses this and decided that such polling isn't
> sensitive information so by default this should just work and it's
> relied upon in Android and in a bunch of other workloads. An LSM can of
> course restrict access via security_file_ioctl().
>
> Fwiw, pidfds now support persistent trusted extended attributes so if
> the LSM folks wanted we can add security.* extended attribute support
> and they can mark pidfds with persistent security labels - persistent as
> in for the lifetime of the task.
>
> > Signed-off-by: Daniel Durning <danieldurning.work@gmail.com>
> > ---
> >  fs/pidfs.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/fs/pidfs.c b/fs/pidfs.c
> > index dba703d4ce4a..058a7d798bca 100644
> > --- a/fs/pidfs.c
> > +++ b/fs/pidfs.c
> > @@ -365,6 +365,13 @@ static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg)
> >               goto copy_out;
> >       }
> >
> > +     /*
> > +      * Do a filesystem cred ptrace check to verify access
> > +      * to the task's info.
> > +      */
> > +     if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS))
> > +             return -EACCES;
> > +
> >       c = get_task_cred(task);
> >       if (!c)
> >               return -ESRCH;
> > --
> > 2.52.0
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] fs/pidfs: Add permission check to pidfd_info()
  2026-02-11 19:43   ` Daniel Durning
@ 2026-02-17 12:01     ` Christian Brauner
  2026-02-20 20:45       ` Daniel Durning
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Brauner @ 2026-02-17 12:01 UTC (permalink / raw)
  To: Daniel Durning
  Cc: linux-fsdevel, linux-security-module, selinux, viro, jack, paul,
	stephen.smalley.work, omosnace, Oleg Nesterov

On Wed, Feb 11, 2026 at 02:43:21PM -0500, Daniel Durning wrote:
> On Mon, Feb 9, 2026 at 9:01 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Fri, Feb 06, 2026 at 06:02:48PM +0000, danieldurning.work@gmail.com wrote:
> > > From: Daniel Durning <danieldurning.work@gmail.com>
> > >
> > > Added a permission check to pidfd_info(). Originally, process info
> > > could be retrieved with a pidfd even if proc was mounted with hidepid
> > > enabled, allowing pidfds to be used to bypass those protections. We
> > > now call ptrace_may_access() to perform some DAC checking as well
> > > as call the appropriate LSM hook.
> > >
> > > The downside to this approach is that there are now more restrictions
> > > on accessing this info from a pidfd than when just using proc (without
> > > hidepid). I am open to suggestions if anyone can think of a better way
> > > to handle this.
> >
> > This isn't really workable since this would regress userspace quite a
> > bit. I think we need a different approach. I've given it some thought
> > and everything's kinda ugly but this might work.
> >
> > In struct pid_namespace record whether anyone ever mounted a procfs
> > with hidepid turned on for this pidns. In pidfd_info() we check whether
> > hidepid was ever turned on. If it wasn't we're done and can just return
> > the info. This will be the common case. If hidepid was ever turned on
> > use kern_path("/proc") to lookup procfs. If not found check
> > ptrace_may_access() to decide whether to return the info or not. If
> > /proc is found check it's hidepid settings and make a decision based on
> > that.
> >
> > You can probably reorder this to call ptrace_may_access() first and then
> > do the procfs lookup dance. Thoughts?
> 
> Thanks for the feedback. I think your solution makes sense.
> 
> Unfortunately, it seems like systemd mounts procfs with hidepid enabled on
> boot for services with the ProtectProc option enabled. This means that
> procfs will always have been mounted with hidepid in the init pid namespace.
> Do you think it would be viable to record whether or not procfs was mounted
> with hidepid enabled in the mount namespace instead?

I guess we can see what it looks like.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] fs/pidfs: Add permission check to pidfd_info()
  2026-02-17 12:01     ` Christian Brauner
@ 2026-02-20 20:45       ` Daniel Durning
  2026-02-24 11:18         ` Christian Brauner
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Durning @ 2026-02-20 20:45 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-fsdevel, linux-security-module, selinux, viro, jack, paul,
	stephen.smalley.work, omosnace, Oleg Nesterov

On Tue, Feb 17, 2026 at 7:01 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Wed, Feb 11, 2026 at 02:43:21PM -0500, Daniel Durning wrote:
> > On Mon, Feb 9, 2026 at 9:01 AM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > On Fri, Feb 06, 2026 at 06:02:48PM +0000, danieldurning.work@gmail.com wrote:
> > > > From: Daniel Durning <danieldurning.work@gmail.com>
> > > >
> > > > Added a permission check to pidfd_info(). Originally, process info
> > > > could be retrieved with a pidfd even if proc was mounted with hidepid
> > > > enabled, allowing pidfds to be used to bypass those protections. We
> > > > now call ptrace_may_access() to perform some DAC checking as well
> > > > as call the appropriate LSM hook.
> > > >
> > > > The downside to this approach is that there are now more restrictions
> > > > on accessing this info from a pidfd than when just using proc (without
> > > > hidepid). I am open to suggestions if anyone can think of a better way
> > > > to handle this.
> > >
> > > This isn't really workable since this would regress userspace quite a
> > > bit. I think we need a different approach. I've given it some thought
> > > and everything's kinda ugly but this might work.
> > >
> > > In struct pid_namespace record whether anyone ever mounted a procfs
> > > with hidepid turned on for this pidns. In pidfd_info() we check whether
> > > hidepid was ever turned on. If it wasn't we're done and can just return
> > > the info. This will be the common case. If hidepid was ever turned on
> > > use kern_path("/proc") to lookup procfs. If not found check
> > > ptrace_may_access() to decide whether to return the info or not. If
> > > /proc is found check it's hidepid settings and make a decision based on
> > > that.
> > >
> > > You can probably reorder this to call ptrace_may_access() first and then
> > > do the procfs lookup dance. Thoughts?
> >
> > Thanks for the feedback. I think your solution makes sense.
> >
> > Unfortunately, it seems like systemd mounts procfs with hidepid enabled on
> > boot for services with the ProtectProc option enabled. This means that
> > procfs will always have been mounted with hidepid in the init pid namespace.
> > Do you think it would be viable to record whether or not procfs was mounted
> > with hidepid enabled in the mount namespace instead?
>
> I guess we can see what it looks like.

Having looked into this some more I am not sure if the mount
namespace is viable either since a single proc instance could be in
multiple mount namespaces. In addition the mount namespace
does not seem to be easily accessible in the function where proc
mount options are applied. I also considered adding an option
similar to hidepid to pidfs, but since pidfs is not userspace-mounted
I do not think that is possible without some significant changes.

Doing a proc lookup with kern_path() does work, but it does not seem
practical in terms of performance unless we had some other way to
skip it in the common case.

Curious if anyone else has any ideas or suggestions on how this
could be implemented.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] fs/pidfs: Add permission check to pidfd_info()
  2026-02-20 20:45       ` Daniel Durning
@ 2026-02-24 11:18         ` Christian Brauner
  0 siblings, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2026-02-24 11:18 UTC (permalink / raw)
  To: Daniel Durning
  Cc: linux-fsdevel, linux-security-module, selinux, viro, jack, paul,
	stephen.smalley.work, omosnace, Oleg Nesterov

On Fri, Feb 20, 2026 at 03:45:00PM -0500, Daniel Durning wrote:
> On Tue, Feb 17, 2026 at 7:01 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Wed, Feb 11, 2026 at 02:43:21PM -0500, Daniel Durning wrote:
> > > On Mon, Feb 9, 2026 at 9:01 AM Christian Brauner <brauner@kernel.org> wrote:
> > > >
> > > > On Fri, Feb 06, 2026 at 06:02:48PM +0000, danieldurning.work@gmail.com wrote:
> > > > > From: Daniel Durning <danieldurning.work@gmail.com>
> > > > >
> > > > > Added a permission check to pidfd_info(). Originally, process info
> > > > > could be retrieved with a pidfd even if proc was mounted with hidepid
> > > > > enabled, allowing pidfds to be used to bypass those protections. We
> > > > > now call ptrace_may_access() to perform some DAC checking as well
> > > > > as call the appropriate LSM hook.
> > > > >
> > > > > The downside to this approach is that there are now more restrictions
> > > > > on accessing this info from a pidfd than when just using proc (without
> > > > > hidepid). I am open to suggestions if anyone can think of a better way
> > > > > to handle this.
> > > >
> > > > This isn't really workable since this would regress userspace quite a
> > > > bit. I think we need a different approach. I've given it some thought
> > > > and everything's kinda ugly but this might work.
> > > >
> > > > In struct pid_namespace record whether anyone ever mounted a procfs
> > > > with hidepid turned on for this pidns. In pidfd_info() we check whether
> > > > hidepid was ever turned on. If it wasn't we're done and can just return
> > > > the info. This will be the common case. If hidepid was ever turned on
> > > > use kern_path("/proc") to lookup procfs. If not found check
> > > > ptrace_may_access() to decide whether to return the info or not. If
> > > > /proc is found check it's hidepid settings and make a decision based on
> > > > that.
> > > >
> > > > You can probably reorder this to call ptrace_may_access() first and then
> > > > do the procfs lookup dance. Thoughts?
> > >
> > > Thanks for the feedback. I think your solution makes sense.
> > >
> > > Unfortunately, it seems like systemd mounts procfs with hidepid enabled on
> > > boot for services with the ProtectProc option enabled. This means that
> > > procfs will always have been mounted with hidepid in the init pid namespace.
> > > Do you think it would be viable to record whether or not procfs was mounted
> > > with hidepid enabled in the mount namespace instead?
> >
> > I guess we can see what it looks like.
> 
> Having looked into this some more I am not sure if the mount
> namespace is viable either since a single proc instance could be in
> multiple mount namespaces. In addition the mount namespace
> does not seem to be easily accessible in the function where proc
> mount options are applied. I also considered adding an option
> similar to hidepid to pidfs, but since pidfs is not userspace-mounted
> I do not think that is possible without some significant changes.
> 
> Doing a proc lookup with kern_path() does work, but it does not seem
> practical in terms of performance unless we had some other way to
> skip it in the common case.
> 
> Curious if anyone else has any ideas or suggestions on how this
> could be implemented.

Ok, so there's another series that adds support for allowing to mount
procfs with subset=pid. That series currently uses an arcane mechanism
where it walks all mounts in the caller mounts namespace to find procfs
mounts and check its mount options (mount_too_revealing()). To get away
from this barbaric hack I proposed recording all fully visible procfs
mounts in a separate list on struct mnt_namespace that it can walk
whenever a new procfs mount gets plugged in. Once we've done that work
we effectively track whenever a procfs mount comes and goes from a given
mount namespace. When we plug in that mount we simply remember the
option used for the least restrictive procfs mount in that namespace in
a per-mntns "proc_visiblity" field or something.

Then in pidfs we simply do a:

visibility_restricted = READ_ONCE(current->ns_proxy->mnt_ns);

and be done with it. No locks, no lookup, no perf hit. Thoughts?

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-02-24 11:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-06 18:02 [RFC PATCH] fs/pidfs: Add permission check to pidfd_info() danieldurning.work
2026-02-09 14:01 ` Christian Brauner
2026-02-11 19:43   ` Daniel Durning
2026-02-17 12:01     ` Christian Brauner
2026-02-20 20:45       ` Daniel Durning
2026-02-24 11:18         ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox