diff mbox series

[v2] fanotify: allow reporting errors on failure to open fd

Message ID 20241003142922.111539-1-amir73il@gmail.com (mailing list archive)
State New
Headers show
Series [v2] fanotify: allow reporting errors on failure to open fd | expand

Commit Message

Amir Goldstein Oct. 3, 2024, 2:29 p.m. UTC
When working in "fd mode", fanotify_read() needs to open an fd
from a dentry to report event->fd to userspace.

Opening an fd from dentry can fail for several reasons.
For example, when tasks are gone and we try to open their
/proc files or we try to open a WRONLY file like in sysfs
or when trying to open a file that was deleted on the
remote network server.

Add a new flag FAN_REPORT_FD_ERROR for fanotify_init().
For a group with FAN_REPORT_FD_ERROR, we will send the
event with the error instead of the open fd, otherwise
userspace may not get the error at all.

The FAN_REPORT_FD_ERROR flag is not allowed for groups in "fid mode"
which do not use open fd's as the object identifier.

For ean overflow event, we report -EBADF to avoid confusing FAN_NOFD
with -EPERM.  Similarly for pidfd open errors we report either -ESRCH
or the open error instead of FAN_NOPIDFD and FAN_EPIDFD.

In any case, userspace will not know which file failed to
open, so add a debug print for further investigation.

Reported-by: Krishna Vivek Vitta <kvitta@microsoft.com>
Closes: https://lore.kernel.org/linux-fsdevel/SI2P153MB07182F3424619EDDD1F393EED46D2@SI2P153MB0718.APCP153.PROD.OUTLOOK.COM/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
Jan,

This is my proposal for a slightly better UAPI for error reporting,
taking into account your review comments on v1 [1].

I have written some basic LTP tests for the simple cases [2], but
not yet for actual open errors.

I tested the open error manually using an enhanced fanotify_example [3]
and the reproducer of the 9p open unlinked file issue [4]:

$ ./fanotify_example /vtmp/
Press enter key to terminate.
Listening for events.
FAN_OPEN_PERM: File /vtmp/config.lock
FAN_CLOSE_WRITE: fd open failed: No such file or directory

And the debug print in kmsg:
[ 1836.619957] fanotify: create_fd(/config.lock) failed err=-2

fanotify_read() can still return an error with FAN_REPORT_FD_ERROR,
but not for a failure to open an fd.

Thanks,
Amir.

Changes since v1:
- Change pr_warn() => pr_debug()
- Restrict FAN_REPORT_FD_ERROR to group in fd mode
- Report fd error also for pidfd errors
- Report -EBAFD instead of FAN_NOFD in overflow event

[1] https://lore.kernel.org/linux-fsdevel/20240927125624.2198202-1-amir73il@gmail.com/
[2] https://github.com/amir73il/ltp/commits/fan_fd_error
[3] https://github.com/amir73il/fsnotify-utils/blob/fan_report_fd_error/src/test/fanotify_example.c
[4] https://lore.kernel.org/linux-fsdevel/CAOQ4uxgRnzB0E2ESeqgZBHW++zyRj8-VmvB38Vxm5OXgr=EM9g@mail.gmail.com/

 fs/notify/fanotify/fanotify_user.c | 83 +++++++++++++++++++++---------
 include/linux/fanotify.h           |  1 +
 include/uapi/linux/fanotify.h      |  1 +
 3 files changed, 62 insertions(+), 23 deletions(-)

Comments

Amir Goldstein Oct. 4, 2024, 3:34 p.m. UTC | #1
On Thu, Oct 3, 2024 at 4:29 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> When working in "fd mode", fanotify_read() needs to open an fd
> from a dentry to report event->fd to userspace.
>
> Opening an fd from dentry can fail for several reasons.
> For example, when tasks are gone and we try to open their
> /proc files or we try to open a WRONLY file like in sysfs
> or when trying to open a file that was deleted on the
> remote network server.
>
> Add a new flag FAN_REPORT_FD_ERROR for fanotify_init().
> For a group with FAN_REPORT_FD_ERROR, we will send the
> event with the error instead of the open fd, otherwise
> userspace may not get the error at all.
>
> The FAN_REPORT_FD_ERROR flag is not allowed for groups in "fid mode"
> which do not use open fd's as the object identifier.
>
> For ean overflow event, we report -EBADF to avoid confusing FAN_NOFD
> with -EPERM.  Similarly for pidfd open errors we report either -ESRCH
> or the open error instead of FAN_NOPIDFD and FAN_EPIDFD.
>
> In any case, userspace will not know which file failed to
> open, so add a debug print for further investigation.
>
> Reported-by: Krishna Vivek Vitta <kvitta@microsoft.com>
> Closes: https://lore.kernel.org/linux-fsdevel/SI2P153MB07182F3424619EDDD1F393EED46D2@SI2P153MB0718.APCP153.PROD.OUTLOOK.COM/
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> Jan,
>
> This is my proposal for a slightly better UAPI for error reporting,
> taking into account your review comments on v1 [1].
>
> I have written some basic LTP tests for the simple cases [2], but
> not yet for actual open errors.

FWIW, pushed a new test case for reporting -EROFS error either to
fanotify_read() or in event->fd.

Thanks,
Amir.

>
> I tested the open error manually using an enhanced fanotify_example [3]
> and the reproducer of the 9p open unlinked file issue [4]:
>
> $ ./fanotify_example /vtmp/
> Press enter key to terminate.
> Listening for events.
> FAN_OPEN_PERM: File /vtmp/config.lock
> FAN_CLOSE_WRITE: fd open failed: No such file or directory
>
> And the debug print in kmsg:
> [ 1836.619957] fanotify: create_fd(/config.lock) failed err=-2
>
> fanotify_read() can still return an error with FAN_REPORT_FD_ERROR,
> but not for a failure to open an fd.
>
> Thanks,
> Amir.
>
> Changes since v1:
> - Change pr_warn() => pr_debug()
> - Restrict FAN_REPORT_FD_ERROR to group in fd mode
> - Report fd error also for pidfd errors
> - Report -EBAFD instead of FAN_NOFD in overflow event
>
> [1] https://lore.kernel.org/linux-fsdevel/20240927125624.2198202-1-amir73il@gmail.com/
> [2] https://github.com/amir73il/ltp/commits/fan_fd_error
> [3] https://github.com/amir73il/fsnotify-utils/blob/fan_report_fd_error/src/test/fanotify_example.c
> [4] https://lore.kernel.org/linux-fsdevel/CAOQ4uxgRnzB0E2ESeqgZBHW++zyRj8-VmvB38Vxm5OXgr=EM9g@mail.gmail.com/
>
>  fs/notify/fanotify/fanotify_user.c | 83 +++++++++++++++++++++---------
>  include/linux/fanotify.h           |  1 +
>  include/uapi/linux/fanotify.h      |  1 +
>  3 files changed, 62 insertions(+), 23 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 9644bc72e457..37a0dd8ae883 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -266,13 +266,6 @@ static int create_fd(struct fsnotify_group *group, const struct path *path,
>                                group->fanotify_data.f_flags | __FMODE_NONOTIFY,
>                                current_cred());
>         if (IS_ERR(new_file)) {
> -               /*
> -                * we still send an event even if we can't open the file.  this
> -                * can happen when say tasks are gone and we try to open their
> -                * /proc files or we try to open a WRONLY file like in sysfs
> -                * we just send the errno to userspace since there isn't much
> -                * else we can do.
> -                */
>                 put_unused_fd(client_fd);
>                 client_fd = PTR_ERR(new_file);
>         } else {
> @@ -653,6 +646,19 @@ static int copy_info_records_to_user(struct fanotify_event *event,
>         return total_bytes;
>  }
>
> +/* Determine with value to report in event->fd */
> +static int event_fd_error(struct fsnotify_group *group, int fd, int nofd)
> +{
> +       /* An unprivileged user should never get an open fd or specific error */
> +       if (FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV))
> +               return nofd;
> +
> +       if (fd >= 0 || FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR))
> +               return fd;
> +
> +       return nofd;
> +}
> +
>  static ssize_t copy_event_to_user(struct fsnotify_group *group,
>                                   struct fanotify_event *event,
>                                   char __user *buf, size_t count)
> @@ -691,8 +697,32 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>         if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) &&
>             path && path->mnt && path->dentry) {
>                 fd = create_fd(group, path, &f);
> -               if (fd < 0)
> -                       return fd;
> +               /*
> +                * Opening an fd from dentry can fail for several reasons.
> +                * For example, when tasks are gone and we try to open their
> +                * /proc files or we try to open a WRONLY file like in sysfs
> +                * or when trying to open a file that was deleted on the
> +                * remote network server.
> +                *
> +                * For a group with FAN_REPORT_FD_ERROR, we will send the
> +                * event with the error instead of the open fd, otherwise
> +                * Userspace may not get the error at all.
> +                * In any case, userspace will not know which file failed to
> +                * open, so add a debug print for further investigation.
> +                */
> +               if (fd < 0) {
> +                       pr_debug("fanotify: create_fd(%pd2) failed err=%d\n",
> +                                path->dentry, fd);
> +                       if (!FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR))
> +                               return fd;
> +               }
> +       } else {
> +               /*
> +                * For a group with FAN_REPORT_FD_ERROR, report an event with
> +                * no file, such as an overflow event with -BADF instead of
> +                * FAN_NOFD, because FAN_NOFD collides with -EPERM.
> +                */
> +               fd = event_fd_error(group, -EBADF, FAN_NOFD);
>         }
>         metadata.fd = fd;
>
> @@ -709,17 +739,17 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>                  * 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. Report FAN_NOPIDFD to the event listener
> -                * in those cases, with all other pidfd creation errors being
> -                * reported as FAN_EPIDFD.
> +                * 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 == 0 ||
>                     !pid_has_task(event->pid, PIDTYPE_TGID)) {
> -                       pidfd = FAN_NOPIDFD;
> +                       pidfd = event_fd_error(group, -ESRCH, FAN_NOPIDFD);
>                 } else {
>                         pidfd = pidfd_prepare(event->pid, 0, &pidfd_file);
> -                       if (pidfd < 0)
> -                               pidfd = FAN_EPIDFD;
> +                       pidfd = event_fd_error(group, pidfd, FAN_NOPIDFD);
>                 }
>         }
>
> @@ -737,9 +767,6 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>         buf += FAN_EVENT_METADATA_LEN;
>         count -= FAN_EVENT_METADATA_LEN;
>
> -       if (fanotify_is_perm_event(event->mask))
> -               FANOTIFY_PERM(event)->fd = fd;
> -
>         if (info_mode) {
>                 ret = copy_info_records_to_user(event, info, info_mode, pidfd,
>                                                 buf, count);
> @@ -753,15 +780,18 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>         if (pidfd_file)
>                 fd_install(pidfd, pidfd_file);
>
> +       if (fanotify_is_perm_event(event->mask))
> +               FANOTIFY_PERM(event)->fd = fd;
> +
>         return metadata.event_len;
>
>  out_close_fd:
> -       if (fd != FAN_NOFD) {
> +       if (f) {
>                 put_unused_fd(fd);
>                 fput(f);
>         }
>
> -       if (pidfd >= 0) {
> +       if (pidfd_file) {
>                 put_unused_fd(pidfd);
>                 fput(pidfd_file);
>         }
> @@ -845,7 +875,7 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
>                 if (!fanotify_is_perm_event(event->mask)) {
>                         fsnotify_destroy_event(group, &event->fse);
>                 } else {
> -                       if (ret <= 0) {
> +                       if (ret <= 0 || FANOTIFY_PERM(event)->fd < 0) {
>                                 spin_lock(&group->notification_lock);
>                                 finish_permission_event(group,
>                                         FANOTIFY_PERM(event), FAN_DENY, NULL);
> @@ -1453,7 +1483,14 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>                 return -EINVAL;
>         }
>
> -       if (fid_mode && class != FAN_CLASS_NOTIF)
> +       /*
> +        * Legacy fanotify mode reports open fd's in event->fd.
> +        * With fid mode, open fd's are not reported and event->fd is FAN_NOFD.
> +        * High priority classes require reporting open fd's.
> +        * FAN_REPORT_FD_ERROR is only allowed when reporting open fd's.
> +        */
> +       if (fid_mode &&
> +           (class != FAN_CLASS_NOTIF || flags & FAN_REPORT_FD_ERROR))
>                 return -EINVAL;
>
>         /*
> @@ -1954,7 +1991,7 @@ static int __init fanotify_user_setup(void)
>                                      FANOTIFY_DEFAULT_MAX_USER_MARKS);
>
>         BUILD_BUG_ON(FANOTIFY_INIT_FLAGS & FANOTIFY_INTERNAL_GROUP_FLAGS);
> -       BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 12);
> +       BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 13);
>         BUILD_BUG_ON(HWEIGHT32(FANOTIFY_MARK_FLAGS) != 11);
>
>         fanotify_mark_cache = KMEM_CACHE(fanotify_mark,
> diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
> index 4f1c4f603118..89ff45bd6f01 100644
> --- a/include/linux/fanotify.h
> +++ b/include/linux/fanotify.h
> @@ -36,6 +36,7 @@
>  #define FANOTIFY_ADMIN_INIT_FLAGS      (FANOTIFY_PERM_CLASSES | \
>                                          FAN_REPORT_TID | \
>                                          FAN_REPORT_PIDFD | \
> +                                        FAN_REPORT_FD_ERROR | \
>                                          FAN_UNLIMITED_QUEUE | \
>                                          FAN_UNLIMITED_MARKS)
>
> diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
> index a37de58ca571..34f221d3a1b9 100644
> --- a/include/uapi/linux/fanotify.h
> +++ b/include/uapi/linux/fanotify.h
> @@ -60,6 +60,7 @@
>  #define FAN_REPORT_DIR_FID     0x00000400      /* Report unique directory id */
>  #define FAN_REPORT_NAME                0x00000800      /* Report events with name */
>  #define FAN_REPORT_TARGET_FID  0x00001000      /* Report dirent target id  */
> +#define FAN_REPORT_FD_ERROR    0x00002000      /* event->fd can report error */
>
>  /* Convenience macro - FAN_REPORT_NAME requires FAN_REPORT_DIR_FID */
>  #define FAN_REPORT_DFID_NAME   (FAN_REPORT_DIR_FID | FAN_REPORT_NAME)
> --
> 2.34.1
>
Jan Kara Oct. 16, 2024, 3:48 p.m. UTC | #2
Hello Amir!

On Thu 03-10-24 16:29:22, Amir Goldstein wrote:
> When working in "fd mode", fanotify_read() needs to open an fd
> from a dentry to report event->fd to userspace.
> 
> Opening an fd from dentry can fail for several reasons.
> For example, when tasks are gone and we try to open their
> /proc files or we try to open a WRONLY file like in sysfs
> or when trying to open a file that was deleted on the
> remote network server.
> 
> Add a new flag FAN_REPORT_FD_ERROR for fanotify_init().
> For a group with FAN_REPORT_FD_ERROR, we will send the
> event with the error instead of the open fd, otherwise
> userspace may not get the error at all.
> 
> The FAN_REPORT_FD_ERROR flag is not allowed for groups in "fid mode"
> which do not use open fd's as the object identifier.
> 
> For ean overflow event, we report -EBADF to avoid confusing FAN_NOFD
> with -EPERM.  Similarly for pidfd open errors we report either -ESRCH
> or the open error instead of FAN_NOPIDFD and FAN_EPIDFD.
> 
> In any case, userspace will not know which file failed to
> open, so add a debug print for further investigation.
> 
> Reported-by: Krishna Vivek Vitta <kvitta@microsoft.com>
> Closes: https://lore.kernel.org/linux-fsdevel/SI2P153MB07182F3424619EDDD1F393EED46D2@SI2P153MB0718.APCP153.PROD.OUTLOOK.COM/
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

I was mulling over this becase I wasn't quite happy with the result but I
could not clearly formulate my problems with the patch. So I've just sat
down and played with the code. Attached is what I've ended up with - please
have a look if it looks OK to you as well, it passes the LTP test you've
created. Functionally, I've just removed the check that FAN_REPORT_FD_ERROR
cannot be used in "fid mode" because when we decided to use the flag for
pidfd, it makes sense to combine it with "fid mode". I've also moved
EOPENSTALE special handling to a more logical place now.

								Honza
Amir Goldstein Oct. 16, 2024, 5:46 p.m. UTC | #3
On Wed, Oct 16, 2024 at 5:48 PM Jan Kara <jack@suse.cz> wrote:
>
> Hello Amir!
>
> On Thu 03-10-24 16:29:22, Amir Goldstein wrote:
> > When working in "fd mode", fanotify_read() needs to open an fd
> > from a dentry to report event->fd to userspace.
> >
> > Opening an fd from dentry can fail for several reasons.
> > For example, when tasks are gone and we try to open their
> > /proc files or we try to open a WRONLY file like in sysfs
> > or when trying to open a file that was deleted on the
> > remote network server.
> >
> > Add a new flag FAN_REPORT_FD_ERROR for fanotify_init().
> > For a group with FAN_REPORT_FD_ERROR, we will send the
> > event with the error instead of the open fd, otherwise
> > userspace may not get the error at all.
> >
> > The FAN_REPORT_FD_ERROR flag is not allowed for groups in "fid mode"
> > which do not use open fd's as the object identifier.
> >
> > For ean overflow event, we report -EBADF to avoid confusing FAN_NOFD
> > with -EPERM.  Similarly for pidfd open errors we report either -ESRCH
> > or the open error instead of FAN_NOPIDFD and FAN_EPIDFD.
> >
> > In any case, userspace will not know which file failed to
> > open, so add a debug print for further investigation.
> >
> > Reported-by: Krishna Vivek Vitta <kvitta@microsoft.com>
> > Closes: https://lore.kernel.org/linux-fsdevel/SI2P153MB07182F3424619EDDD1F393EED46D2@SI2P153MB0718.APCP153.PROD.OUTLOOK.COM/
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> I was mulling over this becase I wasn't quite happy with the result but I
> could not clearly formulate my problems with the patch. So I've just sat
> down and played with the code. Attached is what I've ended up with - please
> have a look if it looks OK to you as well, it passes the LTP test you've
> created. Functionally, I've just removed the check that FAN_REPORT_FD_ERROR
> cannot be used in "fid mode" because when we decided to use the flag for
> pidfd, it makes sense to combine it with "fid mode". I've also moved
> EOPENSTALE special handling to a more logical place now.
>

Yeh this looks nicer.
Thanks for the touch up.
Amir.
diff mbox series

Patch

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 9644bc72e457..37a0dd8ae883 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -266,13 +266,6 @@  static int create_fd(struct fsnotify_group *group, const struct path *path,
 			       group->fanotify_data.f_flags | __FMODE_NONOTIFY,
 			       current_cred());
 	if (IS_ERR(new_file)) {
-		/*
-		 * we still send an event even if we can't open the file.  this
-		 * can happen when say tasks are gone and we try to open their
-		 * /proc files or we try to open a WRONLY file like in sysfs
-		 * we just send the errno to userspace since there isn't much
-		 * else we can do.
-		 */
 		put_unused_fd(client_fd);
 		client_fd = PTR_ERR(new_file);
 	} else {
@@ -653,6 +646,19 @@  static int copy_info_records_to_user(struct fanotify_event *event,
 	return total_bytes;
 }
 
+/* Determine with value to report in event->fd */
+static int event_fd_error(struct fsnotify_group *group, int fd, int nofd)
+{
+	/* An unprivileged user should never get an open fd or specific error */
+	if (FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV))
+		return nofd;
+
+	if (fd >= 0 || FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR))
+		return fd;
+
+	return nofd;
+}
+
 static ssize_t copy_event_to_user(struct fsnotify_group *group,
 				  struct fanotify_event *event,
 				  char __user *buf, size_t count)
@@ -691,8 +697,32 @@  static ssize_t copy_event_to_user(struct fsnotify_group *group,
 	if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) &&
 	    path && path->mnt && path->dentry) {
 		fd = create_fd(group, path, &f);
-		if (fd < 0)
-			return fd;
+		/*
+		 * Opening an fd from dentry can fail for several reasons.
+		 * For example, when tasks are gone and we try to open their
+		 * /proc files or we try to open a WRONLY file like in sysfs
+		 * or when trying to open a file that was deleted on the
+		 * remote network server.
+		 *
+		 * For a group with FAN_REPORT_FD_ERROR, we will send the
+		 * event with the error instead of the open fd, otherwise
+		 * Userspace may not get the error at all.
+		 * In any case, userspace will not know which file failed to
+		 * open, so add a debug print for further investigation.
+		 */
+		if (fd < 0) {
+			pr_debug("fanotify: create_fd(%pd2) failed err=%d\n",
+				 path->dentry, fd);
+			if (!FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR))
+				return fd;
+		}
+	} else {
+		/*
+		 * For a group with FAN_REPORT_FD_ERROR, report an event with
+		 * no file, such as an overflow event with -BADF instead of
+		 * FAN_NOFD, because FAN_NOFD collides with -EPERM.
+		 */
+		fd = event_fd_error(group, -EBADF, FAN_NOFD);
 	}
 	metadata.fd = fd;
 
@@ -709,17 +739,17 @@  static ssize_t copy_event_to_user(struct fsnotify_group *group,
 		 * 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. Report FAN_NOPIDFD to the event listener
-		 * in those cases, with all other pidfd creation errors being
-		 * reported as FAN_EPIDFD.
+		 * 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 == 0 ||
 		    !pid_has_task(event->pid, PIDTYPE_TGID)) {
-			pidfd = FAN_NOPIDFD;
+			pidfd = event_fd_error(group, -ESRCH, FAN_NOPIDFD);
 		} else {
 			pidfd = pidfd_prepare(event->pid, 0, &pidfd_file);
-			if (pidfd < 0)
-				pidfd = FAN_EPIDFD;
+			pidfd = event_fd_error(group, pidfd, FAN_NOPIDFD);
 		}
 	}
 
@@ -737,9 +767,6 @@  static ssize_t copy_event_to_user(struct fsnotify_group *group,
 	buf += FAN_EVENT_METADATA_LEN;
 	count -= FAN_EVENT_METADATA_LEN;
 
-	if (fanotify_is_perm_event(event->mask))
-		FANOTIFY_PERM(event)->fd = fd;
-
 	if (info_mode) {
 		ret = copy_info_records_to_user(event, info, info_mode, pidfd,
 						buf, count);
@@ -753,15 +780,18 @@  static ssize_t copy_event_to_user(struct fsnotify_group *group,
 	if (pidfd_file)
 		fd_install(pidfd, pidfd_file);
 
+	if (fanotify_is_perm_event(event->mask))
+		FANOTIFY_PERM(event)->fd = fd;
+
 	return metadata.event_len;
 
 out_close_fd:
-	if (fd != FAN_NOFD) {
+	if (f) {
 		put_unused_fd(fd);
 		fput(f);
 	}
 
-	if (pidfd >= 0) {
+	if (pidfd_file) {
 		put_unused_fd(pidfd);
 		fput(pidfd_file);
 	}
@@ -845,7 +875,7 @@  static ssize_t fanotify_read(struct file *file, char __user *buf,
 		if (!fanotify_is_perm_event(event->mask)) {
 			fsnotify_destroy_event(group, &event->fse);
 		} else {
-			if (ret <= 0) {
+			if (ret <= 0 || FANOTIFY_PERM(event)->fd < 0) {
 				spin_lock(&group->notification_lock);
 				finish_permission_event(group,
 					FANOTIFY_PERM(event), FAN_DENY, NULL);
@@ -1453,7 +1483,14 @@  SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 		return -EINVAL;
 	}
 
-	if (fid_mode && class != FAN_CLASS_NOTIF)
+	/*
+	 * Legacy fanotify mode reports open fd's in event->fd.
+	 * With fid mode, open fd's are not reported and event->fd is FAN_NOFD.
+	 * High priority classes require reporting open fd's.
+	 * FAN_REPORT_FD_ERROR is only allowed when reporting open fd's.
+	 */
+	if (fid_mode &&
+	    (class != FAN_CLASS_NOTIF || flags & FAN_REPORT_FD_ERROR))
 		return -EINVAL;
 
 	/*
@@ -1954,7 +1991,7 @@  static int __init fanotify_user_setup(void)
 				     FANOTIFY_DEFAULT_MAX_USER_MARKS);
 
 	BUILD_BUG_ON(FANOTIFY_INIT_FLAGS & FANOTIFY_INTERNAL_GROUP_FLAGS);
-	BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 12);
+	BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 13);
 	BUILD_BUG_ON(HWEIGHT32(FANOTIFY_MARK_FLAGS) != 11);
 
 	fanotify_mark_cache = KMEM_CACHE(fanotify_mark,
diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index 4f1c4f603118..89ff45bd6f01 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -36,6 +36,7 @@ 
 #define FANOTIFY_ADMIN_INIT_FLAGS	(FANOTIFY_PERM_CLASSES | \
 					 FAN_REPORT_TID | \
 					 FAN_REPORT_PIDFD | \
+					 FAN_REPORT_FD_ERROR | \
 					 FAN_UNLIMITED_QUEUE | \
 					 FAN_UNLIMITED_MARKS)
 
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index a37de58ca571..34f221d3a1b9 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -60,6 +60,7 @@ 
 #define FAN_REPORT_DIR_FID	0x00000400	/* Report unique directory id */
 #define FAN_REPORT_NAME		0x00000800	/* Report events with name */
 #define FAN_REPORT_TARGET_FID	0x00001000	/* Report dirent target id  */
+#define FAN_REPORT_FD_ERROR	0x00002000	/* event->fd can report error */
 
 /* Convenience macro - FAN_REPORT_NAME requires FAN_REPORT_DIR_FID */
 #define FAN_REPORT_DFID_NAME	(FAN_REPORT_DIR_FID | FAN_REPORT_NAME)