[RFC][PATCH] fanotify: disallow mount/sb marks on kernel internal pseudo fs

[Amir Goldstein <amir73il@gmail.com>]

Hopefully, nobody is trying to abuse mount/sb marks for watching all
anonymous pipes/inodes.

I cannot think of a good reason to allow this - it looks like an
oversight that dated back to the original fanotify API.

Link: https://lore.kernel.org/linux-fsdevel/20230628101132.kvchg544mczxv2pm@quack3/
Fixes: d54f4fba889b ("fanotify: add API to attach/detach super block mark")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Jan,

As discussed, allowing sb/mount mark on anonymous pipes
makes no sense and we should not allow it.

I've noted FAN_MARK_FILESYSTEM as the Fixes commit as a trigger to
backport to maintained LTS kernels event though this dates back to day one
with FAN_MARK_MOUNT. Not sure if we should keep the Fixes tag or not.

The reason this is an RFC and that I have not included also the
optimization patch is because we may want to consider banning kernel
internal inodes from fanotify and/or inotify altogether.

The tricky point in banning anonymous pipes from inotify, which
could have existing users (?), but maybe not, so maybe this is
something that we need to try out.

I think we can easily get away with banning anonymous pipes from
fanotify altogeter, but I would not like to get to into a situation
where new applications will be written to rely on inotify for
functionaly that fanotify is never going to have.

Thoughts?
Am I over thinking this?

Amir.

 fs/notify/fanotify/fanotify_user.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 95d7d8790bc3..8240a3fdbef0 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1622,6 +1622,20 @@ static int fanotify_events_supported(struct fsnotify_group *group,
 	    path->mnt->mnt_sb->s_type->fs_flags & FS_DISALLOW_NOTIFY_PERM)
 		return -EINVAL;
 
+	/*
+	 * mount and sb marks are not allowed on kernel internal pseudo fs,
+	 * like pipe_mnt, because that would subscribe to events on all the
+	 * anonynous pipes in the system.
+	 *
+	 * XXX: SB_NOUSER covers all of the internal pseudo fs whose objects
+	 * are not exposed to user's mount namespace, but there are other
+	 * SB_KERNMOUNT fs, like nsfs, debugfs, for which the value of
+	 * allowing sb and mount mark is questionable.
+	 */
+	if (mark_type != FAN_MARK_INODE &&
+	    path->mnt->mnt_sb->s_flags & SB_NOUSER)
+		return -EINVAL;
+
 	/*
 	 * We shouldn't have allowed setting dirent events and the directory
 	 * flags FAN_ONDIR and FAN_EVENT_ON_CHILD in mask of non-dir inode,
-- 
2.34.1