| Message ID | 20250304080044.7623-1-ImanDevel@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | inotify: disallow watches on unsupported filesystems | expand |
On Tue, Mar 4, 2025 at 8:59 AM Seyediman Seyedarab <imandevel@gmail.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@gmail.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.
On 25/03/04 12:57PM, Amir Goldstein wrote: > On Tue, Mar 4, 2025 at 8:59 AM Seyediman Seyedarab <imandevel@gmail.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@gmail.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
On Tue, Mar 4, 2025 at 5:05 PM Seyediman Seyedarab <imandevel@gmail.com> wrote: > > On 25/03/04 12:57PM, Amir Goldstein wrote: > > On Tue, Mar 4, 2025 at 8:59 AM Seyediman Seyedarab <imandevel@gmail.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@gmail.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. > Programs not knowing is a problem that could be fixed with a new API or new init flag to fanotify/inotify. Existing programs that would break due to this change is unacceptable. > 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 > You need to distinguish "inotify does not work" from "inotify does not notify on 'remote' changes" that is changes that happen on the network fs server or inside the kernel (in sysfs case) vs. changes that happen via vfs syscalls on the mounted fs, be it p9, cifs, sysfs. There are several discussions about supporting "remote change" notifications for network filesystems - this is a more complex problem. In any case, I believe performing operations on the mounted fs generated inotify events for all the fs that you listed and without a claim that nobody is using this facility we cannot regress this behavior without an opt-in from the application. Thanks, Amir.
On 25/03/04 05:41PM, Amir Goldstein wrote: > On Tue, Mar 4, 2025 at 5:05 PM Seyediman Seyedarab <imandevel@gmail.com> wrote: > > > > On 25/03/04 12:57PM, Amir Goldstein wrote: > > > On Tue, Mar 4, 2025 at 8:59 AM Seyediman Seyedarab <imandevel@gmail.com> wrote: > > 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. > > Programs not knowing is a problem that could be fixed with a new API > or new init flag to fanotify/inotify. > > Existing programs that would break due to this change is unacceptable. > Maybe something like IN_DISALLOW_REMOTE could work for now, at least until remote change notifications are properly implemented for those specific filesystems? Later, if needed, it could evolve into a new API, and the flag could become the default behavior when passed to that API. > > 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 > > > > You need to distinguish "inotify does not work" > from "inotify does not notify on 'remote' changes" > that is changes that happen on the network fs server or inside the > kernel (in sysfs case) vs. changes that happen via vfs syscalls > on the mounted fs, be it p9, cifs, sysfs. > > There are several discussions about supporting "remote change" > notifications for network filesystems - this is a more complex problem. > > In any case, I believe performing operations on the mounted fs > generated inotify events for all the fs that you listed and without > a claim that nobody is using this facility we cannot regress this > behavior without an opt-in from the application. Understood. So this is what I should work on (correct me if anything seems off): 1. Carefully list all filesystems where "remote" changes occur. 2. Introduce a flag like FS_DISALLOW_INOTIFY_REMOTE in fs_flags for these filesystems. 3. Provide an option for userspace, such as IN_DISALLOW_REMOTE, so applications can explicitly handle this behavior. 4. (Possibly later, if it makes sense) Introduce a new syscall where FS_DISALLOW_INOTIFY_REMOTE is the default behavior. Regards, Seyediman
On Tue, Mar 4, 2025 at 8:06 PM Seyediman Seyedarab <imandevel@gmail.com> wrote: > > On 25/03/04 05:41PM, Amir Goldstein wrote: > > On Tue, Mar 4, 2025 at 5:05 PM Seyediman Seyedarab <imandevel@gmail.com> wrote: > > > > > > On 25/03/04 12:57PM, Amir Goldstein wrote: > > > > On Tue, Mar 4, 2025 at 8:59 AM Seyediman Seyedarab <imandevel@gmail.com> wrote: > > > > 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. > > > > Programs not knowing is a problem that could be fixed with a new API > > or new init flag to fanotify/inotify. > > > > Existing programs that would break due to this change is unacceptable. > > > > Maybe something like IN_DISALLOW_REMOTE could work for now, at least > until remote change notifications are properly implemented for those > specific filesystems? Later, if needed, it could evolve into a new API, > and the flag could become the default behavior when passed to that API. > > > > 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 > > > > > > > You need to distinguish "inotify does not work" > > from "inotify does not notify on 'remote' changes" > > that is changes that happen on the network fs server or inside the > > kernel (in sysfs case) vs. changes that happen via vfs syscalls > > on the mounted fs, be it p9, cifs, sysfs. > > > > There are several discussions about supporting "remote change" > > notifications for network filesystems - this is a more complex problem. > > > > In any case, I believe performing operations on the mounted fs > > generated inotify events for all the fs that you listed and without > > a claim that nobody is using this facility we cannot regress this > > behavior without an opt-in from the application. > > Understood. So this is what I should work on (correct me if anything > seems off): > 1. Carefully list all filesystems where "remote" changes occur. > 2. Introduce a flag like FS_DISALLOW_INOTIFY_REMOTE in fs_flags > for these filesystems. > 3. Provide an option for userspace, such as IN_DISALLOW_REMOTE, > so applications can explicitly handle this behavior. > 4. (Possibly later, if it makes sense) Introduce a new syscall where > FS_DISALLOW_INOTIFY_REMOTE is the default behavior. > Generally, this is a correct way to add new functionality, but I would rather not extend the inotify API. fanotify API is (mostly) a super set of inotify API, so any extension of the API better be of fanotify. If you try to use fanotify API with FAN_REPORT_FID and FAN_MARK_FILESYSTEM, for example, by running the tool fsnotifywait --filesystem, I think that you will find out that many of the fs that you wanted to blacklist already return -EOPNOTSUPP, because they do not support nfs export and some return -ENODEV (e.g. fuse) because they do not have an fsid. So essentially, you (almost) already have the new API that you wanted. As a matter of fact, before commit a95aef69a740 ("fanotify: support reporting non-decodeable file handles") in kernel v6.6, FAN_MARK_INODE would also yield the same result (i.e. fsnotifywait without --filesystem) and that is more or less equivalent to inotifywait, because unlike fsnotifywait --filesystem, it does not require CAP_SYS_ADMIN. Please see if fsnotifywait --filesystem is failing to watch the filesystems that you are interested in blacklisting and list the filesystems that do not fail and you think should fail to watch. If there are filesystems that do not fail (e.g. cifs) and you still want to blacklist them, please argue your reasons. If the behavior you get from fsnotifywait --filesystem is matching your expectations and the only problem is that fsnotifywait without --filesystem on recent kernel does not match your expectations, it would be easy to add a fanotify_init flag like FAN_REPORT_DECODEABLE_FID, which enforces the same strict requirements as --filesystem, but does not require CAP_SYS_ADMIN. But generally, the relation between the fs that you define as "unreliable" to the fs that work with --filesystem is circumstantial. A better way to identify fs that are remote fs is to check if they implement the d_revalidate() operation. It's easy to add an fanotify_init flag to disallow those remote fs, but we really need to first better define what is the desired goal. Exercise - try to write the man page of the proposed new flag in a way that is clear to anyone who reads the description. If you manage to do that, you are far more likely to argue your case. Thanks, Amir.
Hi Seyediman,
kernel test robot noticed the following build errors:
[auto build test ERROR on jack-fs/fsnotify]
[also build test ERROR on linus/master v6.14-rc5 next-20250304]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Seyediman-Seyedarab/inotify-disallow-watches-on-unsupported-filesystems/20250304-160213
base: https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
patch link: https://lore.kernel.org/r/20250304080044.7623-1-ImanDevel%40gmail.com
patch subject: [PATCH] inotify: disallow watches on unsupported filesystems
config: s390-randconfig-002-20250305 (https://download.01.org/0day-ci/archive/20250305/202503051755.5uktuxba-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250305/202503051755.5uktuxba-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503051755.5uktuxba-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/kernel.h:16,
from include/linux/cpumask.h:11,
from include/linux/smp.h:13,
from include/linux/lockdep.h:14,
from include/linux/spinlock.h:63,
from include/linux/wait.h:9,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from fs/notify/inotify/inotify_user.c:17:
fs/notify/inotify/inotify_user.c: In function 'is_unwatchable_fs':
>> fs/notify/inotify/inotify_user.c:702:40: error: 'unwatchable_fs' undeclared (first use in this function); did you mean 'is_unwatchable_fs'?
702 | for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++)
| ^~~~~~~~~~~~~~
include/linux/array_size.h:11:33: note: in definition of macro 'ARRAY_SIZE'
11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
| ^~~
fs/notify/inotify/inotify_user.c:702:40: note: each undeclared identifier is reported only once for each function it appears in
702 | for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++)
| ^~~~~~~~~~~~~~
include/linux/array_size.h:11:33: note: in definition of macro 'ARRAY_SIZE'
11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
| ^~~
In file included from include/linux/file.h:9,
from fs/notify/inotify/inotify_user.c:16:
include/linux/compiler.h:245:77: error: expression in static assertion is not an integer
245 | #define __BUILD_BUG_ON_ZERO_MSG(e, msg) ((int)sizeof(struct {_Static_assert(!(e), msg);}))
| ^
include/linux/compiler.h:249:33: note: in expansion of macro '__BUILD_BUG_ON_ZERO_MSG'
249 | #define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(__same_type((a), &(a)[0]), "must be array")
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/array_size.h:11:59: note: in expansion of macro '__must_be_array'
11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
| ^~~~~~~~~~~~~~~
fs/notify/inotify/inotify_user.c:702:29: note: in expansion of macro 'ARRAY_SIZE'
702 | for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++)
| ^~~~~~~~~~
vim +702 fs/notify/inotify/inotify_user.c
698
699
700 static inline bool is_unwatchable_fs(struct inode *inode)
701 {
> 702 for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++)
703 if (inode->i_sb->s_magic == unwatchable_fs[i])
704 return true;
705 return false;
706 }
707
Hi Seyediman, kernel test robot noticed the following build errors: [auto build test ERROR on jack-fs/fsnotify] [also build test ERROR on linus/master v6.14-rc5 next-20250305] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Seyediman-Seyedarab/inotify-disallow-watches-on-unsupported-filesystems/20250304-160213 base: https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify patch link: https://lore.kernel.org/r/20250304080044.7623-1-ImanDevel%40gmail.com patch subject: [PATCH] inotify: disallow watches on unsupported filesystems config: powerpc-randconfig-001-20250305 (https://download.01.org/0day-ci/archive/20250305/202503052203.vK0McbRm-lkp@intel.com/config) compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250305/202503052203.vK0McbRm-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202503052203.vK0McbRm-lkp@intel.com/ All errors (new ones prefixed by >>): >> fs/notify/inotify/inotify_user.c:702:33: error: use of undeclared identifier 'unwatchable_fs'; did you mean 'is_unwatchable_fs'? 702 | for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++) | ^~~~~~~~~~~~~~ | is_unwatchable_fs include/linux/array_size.h:11:33: note: expanded from macro 'ARRAY_SIZE' 11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) | ^ fs/notify/inotify/inotify_user.c:700:20: note: 'is_unwatchable_fs' declared here 700 | static inline bool is_unwatchable_fs(struct inode *inode) | ^ >> fs/notify/inotify/inotify_user.c:702:33: error: use of undeclared identifier 'unwatchable_fs'; did you mean 'is_unwatchable_fs'? 702 | for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++) | ^~~~~~~~~~~~~~ | is_unwatchable_fs include/linux/array_size.h:11:48: note: expanded from macro 'ARRAY_SIZE' 11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) | ^ fs/notify/inotify/inotify_user.c:700:20: note: 'is_unwatchable_fs' declared here 700 | static inline bool is_unwatchable_fs(struct inode *inode) | ^ >> fs/notify/inotify/inotify_user.c:702:22: error: subscript of pointer to function type 'bool (struct inode *)' (aka '_Bool (struct inode *)') 702 | for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/array_size.h:11:47: note: expanded from macro 'ARRAY_SIZE' 11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) | ^~~~~ >> fs/notify/inotify/inotify_user.c:702:33: error: use of undeclared identifier 'unwatchable_fs'; did you mean 'is_unwatchable_fs'? 702 | for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++) | ^~~~~~~~~~~~~~ | is_unwatchable_fs include/linux/array_size.h:11:75: note: expanded from macro 'ARRAY_SIZE' 11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) | ^ include/linux/compiler.h:249:65: note: expanded from macro '__must_be_array' 249 | #define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(__same_type((a), &(a)[0]), "must be array") | ^ include/linux/compiler_types.h:483:63: note: expanded from macro '__same_type' 483 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) | ^ include/linux/compiler.h:245:79: note: expanded from macro '__BUILD_BUG_ON_ZERO_MSG' 245 | #define __BUILD_BUG_ON_ZERO_MSG(e, msg) ((int)sizeof(struct {_Static_assert(!(e), msg);})) | ^ fs/notify/inotify/inotify_user.c:700:20: note: 'is_unwatchable_fs' declared here 700 | static inline bool is_unwatchable_fs(struct inode *inode) | ^ >> fs/notify/inotify/inotify_user.c:702:33: error: use of undeclared identifier 'unwatchable_fs'; did you mean 'is_unwatchable_fs'? 702 | for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++) | ^~~~~~~~~~~~~~ | is_unwatchable_fs include/linux/array_size.h:11:75: note: expanded from macro 'ARRAY_SIZE' 11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) | ^ include/linux/compiler.h:249:71: note: expanded from macro '__must_be_array' 249 | #define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(__same_type((a), &(a)[0]), "must be array") | ^ include/linux/compiler_types.h:483:74: note: expanded from macro '__same_type' 483 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) | ^ include/linux/compiler.h:245:79: note: expanded from macro '__BUILD_BUG_ON_ZERO_MSG' 245 | #define __BUILD_BUG_ON_ZERO_MSG(e, msg) ((int)sizeof(struct {_Static_assert(!(e), msg);})) | ^ fs/notify/inotify/inotify_user.c:700:20: note: 'is_unwatchable_fs' declared here 700 | static inline bool is_unwatchable_fs(struct inode *inode) | ^ >> fs/notify/inotify/inotify_user.c:702:22: error: subscript of pointer to function type 'bool (struct inode *)' (aka '_Bool (struct inode *)') 702 | for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/array_size.h:11:59: note: expanded from macro 'ARRAY_SIZE' 11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) | ^~~~~~~~~~~~~~~~~~~~ include/linux/compiler.h:249:70: note: expanded from macro '__must_be_array' 249 | #define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(__same_type((a), &(a)[0]), "must be array") | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/compiler_types.h:483:74: note: expanded from macro '__same_type' 483 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) | ^ include/linux/compiler.h:245:79: note: expanded from macro '__BUILD_BUG_ON_ZERO_MSG' 245 | #define __BUILD_BUG_ON_ZERO_MSG(e, msg) ((int)sizeof(struct {_Static_assert(!(e), msg);})) | ^ fs/notify/inotify/inotify_user.c:703:31: error: use of undeclared identifier 'unwatchable_fs'; did you mean 'is_unwatchable_fs'? 703 | if (inode->i_sb->s_magic == unwatchable_fs[i]) | ^~~~~~~~~~~~~~ | is_unwatchable_fs fs/notify/inotify/inotify_user.c:700:20: note: 'is_unwatchable_fs' declared here 700 | static inline bool is_unwatchable_fs(struct inode *inode) | ^ fs/notify/inotify/inotify_user.c:703:31: error: subscript of pointer to function type 'bool (struct inode *)' (aka '_Bool (struct inode *)') 703 | if (inode->i_sb->s_magic == unwatchable_fs[i]) | ^~~~~~~~~~~~~~ 8 errors generated. vim +702 fs/notify/inotify/inotify_user.c 698 699 700 static inline bool is_unwatchable_fs(struct inode *inode) 701 { > 702 for (int i = 0; i < ARRAY_SIZE(unwatchable_fs); i++) 703 if (inode->i_sb->s_magic == unwatchable_fs[i]) 704 return true; 705 return false; 706 } 707
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; +} + /* inotify syscalls */ static int do_inotify_init(int flags) { @@ -777,6 +792,13 @@ SYSCALL_DEFINE3(inotify_add_watch, int, fd, const char __user *, pathname, inode = path.dentry->d_inode; group = fd_file(f)->private_data; + /* ensure that inotify is only used on supported filesystems */ + if (is_unwatchable_fs(inode)) { + pr_debug("%s: inotify is not supported on filesystem with s_magic=0x%lx\n", + __func__, inode->i_sb->s_magic); + return -EOPNOTSUPP; + } + /* create/update an inode mark */ ret = inotify_update_watch(group, inode, mask); path_put(&path);
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. This patch explicitly rejects inotify usage on filesystems where it is known to be unreliable, such as sysfs, procfs, overlayfs, 9p, fuse, and others. By returning -EOPNOTSUPP, the limitation is made explicit, preventing users from making incorrect assumptions about inotify behavior. Signed-off-by: Seyediman Seyedarab <ImanDevel@gmail.com> --- fs/notify/inotify/inotify_user.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)