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] [thread-next>] [day] [month] [year] [list]

Message-ID: <5tq33ajgtu62tvaiymf3st74ctkurgskq6xpx2ax53vdbayoce@jffpxkthro3u>
Date: Tue, 4 Mar 2025 11:06:12 -0500
From: Seyediman Seyedarab <imandevel@...il.com>
To: Amir Goldstein <amir73il@...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 25/03/04 12:57PM, Amir Goldstein wrote:
> 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.

Thanks for the review! I may have overlooked overlayfs, but these
following discussions led me to include it in the blacklist:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/882147
https://github.com/libuv/libuv/issues/1201
https://github.com/moby/moby/issues/11705

Apparently, things have changed in v4.10, so I may have been wrong
about overlayfs. I can test each filesystem and provide a response
instead of blindly relying on various bug reports. However, let's
first discuss whether the patch is necessary in the first place.

> >
> > 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.

Following the same approach as 0b3b094ac9a7b ("fanotify: Disallow
permission events for the proc filesystem") would require setting
a specific flag for each fs that isn't supported by inotify. If this
is more suitable, I can work on implementing it.

I understand why it might seem like disallowing users from monitoring
these filesystems could break userspace in some way. BUT, programs
work incorrectly precisely because they do not receive any information
from the kernel, so in other words they are already broken. There is no
way for them to know if the fs is supported or not. I mean, even we are
not sure at the moment, then how would they know.

As an example, 'Waybar' is a userspace program affected by this patch.
Since it relies on monitoring sysfs, it isn't working properly anyway.
This is also due to the issue mentioned earlier... inotify_add_watch()
returns without an error, so the developers haven't realized that
inotify isn't actually supported on sysfs. There are over five
discussions regarding this issue that you can find them here:
https://github.com/Alexays/Waybar/pull/3474

That said, I understand if this isn't the approach you're looking for.

Kindest Regards,
Seyediman