[PATCH 0/2] fanotify: avid some premature LSM checks

[PATCH 0/2] fanotify: avid some premature LSM checks

[Ondrej Mosnacek]

Restructure some of the validity and security checks in
fs/notify/fanotify/fanotify_user.c to avoid generating LSM access
denials in the audit log where hey shouldn't be.

Ondrej Mosnacek (2):
  fanotify: avoid/silence premature LSM capability checks
  fanotify: call fanotify_events_supported() before path_permission()
    and security_path_notify()

 fs/notify/fanotify/fanotify_user.c | 50 ++++++++++++++----------------
 1 file changed, 23 insertions(+), 27 deletions(-)

-- 
2.53.0

[PATCH 1/2] fanotify: avoid/silence premature LSM capability checks

[Ondrej Mosnacek]

Make sure calling capable()/ns_capable() actually leads to access denied
when false is returned, because these functions emit an audit record
when a Linux Security Module denies the capability, which makes it
difficult to avoid allowing/silencing unnecessary permissions in
security policies (namely with SELinux).

Where the return value just used to set a flag, use the non-auditing
ns_capable_noaudit() instead.

Fixes: 7cea2a3c505e ("fanotify: support limited functionality for unprivileged users")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 fs/notify/fanotify/fanotify_user.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index d0b9b984002fe..9c9fca2976d2b 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1615,17 +1615,18 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 	pr_debug("%s: flags=%x event_f_flags=%x\n",
 		 __func__, flags, event_f_flags);
 
-	if (!capable(CAP_SYS_ADMIN)) {
-		/*
-		 * An unprivileged user can setup an fanotify group with
-		 * limited functionality - an unprivileged group is limited to
-		 * notification events with file handles or mount ids and it
-		 * cannot use unlimited queue/marks.
-		 */
-		if ((flags & FANOTIFY_ADMIN_INIT_FLAGS) ||
-		    !(flags & (FANOTIFY_FID_BITS | FAN_REPORT_MNT)))
-			return -EPERM;
+	/*
+	 * An unprivileged user can setup an fanotify group with
+	 * limited functionality - an unprivileged group is limited to
+	 * notification events with file handles or mount ids and it
+	 * cannot use unlimited queue/marks.
+	 */
+	if (((flags & FANOTIFY_ADMIN_INIT_FLAGS) ||
+	     !(flags & (FANOTIFY_FID_BITS | FAN_REPORT_MNT))) &&
+	    !capable(CAP_SYS_ADMIN))
+		return -EPERM;
 
+	if (!ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN)) {
 		/*
 		 * Setting the internal flag FANOTIFY_UNPRIV on the group
 		 * prevents setting mount/filesystem marks on this group and
@@ -1990,8 +1991,8 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 	 * A user is allowed to setup sb/mount/mntns marks only if it is
 	 * capable in the user ns where the group was created.
 	 */
-	if (!ns_capable(group->user_ns, CAP_SYS_ADMIN) &&
-	    mark_type != FAN_MARK_INODE)
+	if (mark_type != FAN_MARK_INODE &&
+	    !ns_capable(group->user_ns, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/*
-- 
2.53.0

[PATCH 2/2] fanotify: call fanotify_events_supported() before path_permission() and security_path_notify()

[Ondrej Mosnacek]

The latter trigger LSM (e.g. SELinux) checks, which will log a denial
when permission is denied, so it's better to do them after validity
checks to avoid logging a denial when the operation would fail anyway.

Fixes: 0b3b094ac9a7 ("fanotify: Disallow permission events for proc filesystem")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 fs/notify/fanotify/fanotify_user.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 9c9fca2976d2b..bfc4d09e6964a 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1210,6 +1210,7 @@ static int fanotify_find_path(int dfd, const char __user *filename,
 
 		*path = fd_file(f)->f_path;
 		path_get(path);
+		ret = 0;
 	} else {
 		unsigned int lookup_flags = 0;
 
@@ -1219,22 +1220,7 @@ static int fanotify_find_path(int dfd, const char __user *filename,
 			lookup_flags |= LOOKUP_DIRECTORY;
 
 		ret = user_path_at(dfd, filename, lookup_flags, path);
-		if (ret)
-			goto out;
 	}
-
-	/* you can only watch an inode if you have read permissions on it */
-	ret = path_permission(path, MAY_READ);
-	if (ret) {
-		path_put(path);
-		goto out;
-	}
-
-	ret = security_path_notify(path, mask, obj_type);
-	if (ret)
-		path_put(path);
-
-out:
 	return ret;
 }
 
@@ -2058,6 +2044,15 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 			goto path_put_and_out;
 	}
 
+	/* you can only watch an inode if you have read permissions on it */
+	ret = path_permission(&path, MAY_READ);
+	if (ret)
+		goto path_put_and_out;
+
+	ret = security_path_notify(&path, mask, obj_type);
+	if (ret)
+		goto path_put_and_out;
+
 	if (fid_mode) {
 		ret = fanotify_test_fsid(path.dentry, flags, &__fsid);
 		if (ret)
-- 
2.53.0

Re: [PATCH 1/2] fanotify: avoid/silence premature LSM capability checks

[Amir Goldstein]

On Mon, Feb 16, 2026 at 5:06 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> Make sure calling capable()/ns_capable() actually leads to access denied
> when false is returned, because these functions emit an audit record
> when a Linux Security Module denies the capability, which makes it
> difficult to avoid allowing/silencing unnecessary permissions in
> security policies (namely with SELinux).
>
> Where the return value just used to set a flag, use the non-auditing
> ns_capable_noaudit() instead.
>
> Fixes: 7cea2a3c505e ("fanotify: support limited functionality for unprivileged users")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  fs/notify/fanotify/fanotify_user.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index d0b9b984002fe..9c9fca2976d2b 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1615,17 +1615,18 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>         pr_debug("%s: flags=%x event_f_flags=%x\n",
>                  __func__, flags, event_f_flags);
>
> -       if (!capable(CAP_SYS_ADMIN)) {
> -               /*
> -                * An unprivileged user can setup an fanotify group with
> -                * limited functionality - an unprivileged group is limited to
> -                * notification events with file handles or mount ids and it
> -                * cannot use unlimited queue/marks.
> -                */
> -               if ((flags & FANOTIFY_ADMIN_INIT_FLAGS) ||
> -                   !(flags & (FANOTIFY_FID_BITS | FAN_REPORT_MNT)))
> -                       return -EPERM;
> +       /*
> +        * An unprivileged user can setup an fanotify group with
> +        * limited functionality - an unprivileged group is limited to
> +        * notification events with file handles or mount ids and it
> +        * cannot use unlimited queue/marks.

Please extend line breaks to 80 chars

> +        */
> +       if (((flags & FANOTIFY_ADMIN_INIT_FLAGS) ||
> +            !(flags & (FANOTIFY_FID_BITS | FAN_REPORT_MNT))) &&
> +           !capable(CAP_SYS_ADMIN))
> +               return -EPERM;
>
> +       if (!ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN)) {

Not super pretty, but I don't have a better idea, so with line breaks fix
feel free to add:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

>                 /*
>                  * Setting the internal flag FANOTIFY_UNPRIV on the group
>                  * prevents setting mount/filesystem marks on this group and
> @@ -1990,8 +1991,8 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>          * A user is allowed to setup sb/mount/mntns marks only if it is
>          * capable in the user ns where the group was created.
>          */
> -       if (!ns_capable(group->user_ns, CAP_SYS_ADMIN) &&
> -           mark_type != FAN_MARK_INODE)
> +       if (mark_type != FAN_MARK_INODE &&
> +           !ns_capable(group->user_ns, CAP_SYS_ADMIN))
>                 return -EPERM;
>
>         /*
> --
> 2.53.0
>

Re: [PATCH 2/2] fanotify: call fanotify_events_supported() before path_permission() and security_path_notify()

[Amir Goldstein]

On Mon, Feb 16, 2026 at 5:06 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> The latter trigger LSM (e.g. SELinux) checks, which will log a denial
> when permission is denied, so it's better to do them after validity
> checks to avoid logging a denial when the operation would fail anyway.
>
> Fixes: 0b3b094ac9a7 ("fanotify: Disallow permission events for proc filesystem")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---

Fine by me,
Feel free to add
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

>  fs/notify/fanotify/fanotify_user.c | 25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 9c9fca2976d2b..bfc4d09e6964a 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1210,6 +1210,7 @@ static int fanotify_find_path(int dfd, const char __user *filename,
>
>                 *path = fd_file(f)->f_path;
>                 path_get(path);
> +               ret = 0;
>         } else {
>                 unsigned int lookup_flags = 0;
>
> @@ -1219,22 +1220,7 @@ static int fanotify_find_path(int dfd, const char __user *filename,
>                         lookup_flags |= LOOKUP_DIRECTORY;
>
>                 ret = user_path_at(dfd, filename, lookup_flags, path);
> -               if (ret)
> -                       goto out;
>         }
> -
> -       /* you can only watch an inode if you have read permissions on it */
> -       ret = path_permission(path, MAY_READ);
> -       if (ret) {
> -               path_put(path);
> -               goto out;
> -       }
> -
> -       ret = security_path_notify(path, mask, obj_type);
> -       if (ret)
> -               path_put(path);
> -
> -out:
>         return ret;
>  }
>
> @@ -2058,6 +2044,15 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>                         goto path_put_and_out;
>         }
>
> +       /* you can only watch an inode if you have read permissions on it */
> +       ret = path_permission(&path, MAY_READ);
> +       if (ret)
> +               goto path_put_and_out;
> +
> +       ret = security_path_notify(&path, mask, obj_type);
> +       if (ret)
> +               goto path_put_and_out;
> +
>         if (fid_mode) {
>                 ret = fanotify_test_fsid(path.dentry, flags, &__fsid);
>                 if (ret)
> --
> 2.53.0
>

Re: [PATCH 0/2] fanotify: avid some premature LSM checks

[Jan Kara]

On Mon 16-02-26 16:06:23, Ondrej Mosnacek wrote:
> Restructure some of the validity and security checks in
> fs/notify/fanotify/fanotify_user.c to avoid generating LSM access
> denials in the audit log where hey shouldn't be.
> 
> Ondrej Mosnacek (2):
>   fanotify: avoid/silence premature LSM capability checks
>   fanotify: call fanotify_events_supported() before path_permission()
>     and security_path_notify()
> 
>  fs/notify/fanotify/fanotify_user.c | 50 ++++++++++++++----------------
>  1 file changed, 23 insertions(+), 27 deletions(-)

The series looks good to me as well. Thanks! I'll commit the series to my
tree once the merge window closes and fixup the comment formatting on
commit. No need to resend.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/2] fanotify: avid some premature LSM checks

[Ondrej Mosnacek]

On Tue, Feb 17, 2026 at 12:09 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 16-02-26 16:06:23, Ondrej Mosnacek wrote:
> > Restructure some of the validity and security checks in
> > fs/notify/fanotify/fanotify_user.c to avoid generating LSM access
> > denials in the audit log where hey shouldn't be.
> >
> > Ondrej Mosnacek (2):
> >   fanotify: avoid/silence premature LSM capability checks
> >   fanotify: call fanotify_events_supported() before path_permission()
> >     and security_path_notify()
> >
> >  fs/notify/fanotify/fanotify_user.c | 50 ++++++++++++++----------------
> >  1 file changed, 23 insertions(+), 27 deletions(-)
>
> The series looks good to me as well. Thanks! I'll commit the series to my
> tree once the merge window closes and fixup the comment formatting on
> commit. No need to resend.

Great, thanks!

-- 
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

Re: [PATCH 1/2] fanotify: avoid/silence premature LSM capability checks

[Paul Moore]

On Mon, Feb 16, 2026 at 10:13 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> Make sure calling capable()/ns_capable() actually leads to access denied
> when false is returned, because these functions emit an audit record
> when a Linux Security Module denies the capability, which makes it
> difficult to avoid allowing/silencing unnecessary permissions in
> security policies (namely with SELinux).
>
> Where the return value just used to set a flag, use the non-auditing
> ns_capable_noaudit() instead.
>
> Fixes: 7cea2a3c505e ("fanotify: support limited functionality for unprivileged users")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  fs/notify/fanotify/fanotify_user.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)

Reviewed-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com

Re: [PATCH 2/2] fanotify: call fanotify_events_supported() before path_permission() and security_path_notify()

[Paul Moore]

On Mon, Feb 16, 2026 at 10:14 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> The latter trigger LSM (e.g. SELinux) checks, which will log a denial
> when permission is denied, so it's better to do them after validity
> checks to avoid logging a denial when the operation would fail anyway.
>
> Fixes: 0b3b094ac9a7 ("fanotify: Disallow permission events for proc filesystem")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  fs/notify/fanotify/fanotify_user.c | 25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)

Reviewed-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com

Re: [PATCH 0/2] fanotify: avid some premature LSM checks

[Jan Kara]

On Tue 17-02-26 12:09:34, Jan Kara wrote:
> On Mon 16-02-26 16:06:23, Ondrej Mosnacek wrote:
> > Restructure some of the validity and security checks in
> > fs/notify/fanotify/fanotify_user.c to avoid generating LSM access
> > denials in the audit log where hey shouldn't be.
> > 
> > Ondrej Mosnacek (2):
> >   fanotify: avoid/silence premature LSM capability checks
> >   fanotify: call fanotify_events_supported() before path_permission()
> >     and security_path_notify()
> > 
> >  fs/notify/fanotify/fanotify_user.c | 50 ++++++++++++++----------------
> >  1 file changed, 23 insertions(+), 27 deletions(-)
> 
> The series looks good to me as well. Thanks! I'll commit the series to my
> tree once the merge window closes and fixup the comment formatting on
> commit. No need to resend.

Pushed the series to my tree now.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR