linux-kernel - Re: [PATCH] inotify: disallow watches on unsupported filesystems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAOQ4uxiaY9cZFpj4m65SrAVXm7MqB2OFSfyH5D03hEwmdtiBVQ@mail.gmail.com>
Date: Tue, 4 Mar 2025 12:57:36 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Seyediman Seyedarab <imandevel@...il.com>
Cc: jack@...e.cz, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-kernel-mentees@...ts.linux.dev
Subject: Re: [PATCH] inotify: disallow watches on unsupported filesystems

On Tue, Mar 4, 2025 at 8:59 AM Seyediman Seyedarab <imandevel@...il.com> wrote:
>
> currently, inotify_add_watch() allows adding watches on filesystems
> where inotify does not work correctly, without returning an explicit
> error. This behavior is misleading and can cause confusion for users
> expecting inotify to work on a certain filesystem.

That maybe so, but it's not that inotify does not work at all,
in fact it probably works most of the time for those fs,
so there may be users setting inotify watches on those fs,
so it is not right to regress those users.

>
> This patch explicitly rejects inotify usage on filesystems where it
> is known to be unreliable, such as sysfs, procfs, overlayfs, 9p, fuse,
> and others.

Where did you get this list of fs from?
Why do you claim that inotify does not work on overlayfs?
Specifically, there are two LTP tests inotify07 and inotify08
that test inotify over overlayfs.

This makes me question other fs on your list.

>
> By returning -EOPNOTSUPP, the limitation is made explicit, preventing
> users from making incorrect assumptions about inotify behavior.
>
> Signed-off-by: Seyediman Seyedarab <ImanDevel@...il.com>
> ---
>  fs/notify/inotify/inotify_user.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
> index b372fb2c56bd..9b96438f4d46 100644
> --- a/fs/notify/inotify/inotify_user.c
> +++ b/fs/notify/inotify/inotify_user.c
> @@ -87,6 +87,13 @@ static const struct ctl_table inotify_table[] = {
>         },
>  };
>
> +static const unsigned long unwatchable_fs[] = {
> +       PROC_SUPER_MAGIC,      SYSFS_MAGIC,       TRACEFS_MAGIC,
> +       DEBUGFS_MAGIC,        CGROUP_SUPER_MAGIC, SECURITYFS_MAGIC,
> +       RAMFS_MAGIC,          DEVPTS_SUPER_MAGIC, BPF_FS_MAGIC,
> +       OVERLAYFS_SUPER_MAGIC, FUSE_SUPER_MAGIC,   NFS_SUPER_MAGIC
> +};
> +
>  static void __init inotify_sysctls_init(void)
>  {
>         register_sysctl("fs/inotify", inotify_table);
> @@ -690,6 +697,14 @@ static struct fsnotify_group *inotify_new_group(unsigned int max_events)
>  }
>
>
> +static inline bool is_unwatchable_fs(struct inode *inode)
> +{
> +       for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++)
> +               if (inode->i_sb->s_magic == unwatchable_fs[i])
> +                       return true;
> +       return false;
> +}

This is not a good practice for black listing fs.

See commit 0b3b094ac9a7b ("fanotify: Disallow permission events
for proc filesystem") for a better practice, but again, we cannot just
stop supporting inotify on fs where it was supported.

The assumption with the commit above was that setting permission
events on procfs is possible, but nobody (except for fuzzers) really does that
and if we have found out that there were actual users that do it, we
would have needed to revert that commit.

Thanks,
Amir.