From 0b3b094ac9a7bb1fcf5d694f3ec981e6864a63d3 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Wed, 15 May 2019 16:28:34 +0200 Subject: fanotify: Disallow permission events for proc filesystem Proc filesystem has special locking rules for various files. Thus fanotify which opens files on event delivery can easily deadlock against another process that waits for fanotify permission event to be handled. Since permission events on /proc have doubtful value anyway, just disallow them. Link: https://lore.kernel.org/linux-fsdevel/20190320131642.GE9485@quack2.suse.cz/ Reviewed-by: Amir Goldstein Signed-off-by: Jan Kara --- fs/notify/fanotify/fanotify_user.c | 22 ++++++++++++++++++++++ fs/proc/root.c | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index a90bb19dcfa2..91006f47e420 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -920,6 +920,22 @@ static int fanotify_test_fid(struct path *path, __kernel_fsid_t *fsid) return 0; } +static int fanotify_events_supported(struct path *path, __u64 mask) +{ + /* + * Some filesystems such as 'proc' acquire unusual locks when opening + * files. For them fanotify permission events have high chances of + * deadlocking the system - open done when reporting fanotify event + * blocks on this "unusual" lock while another process holding the lock + * waits for fanotify permission event to be answered. Just disallow + * permission events for such filesystems. + */ + if (mask & FANOTIFY_PERM_EVENTS && + path->mnt->mnt_sb->s_type->fs_flags & FS_DISALLOW_NOTIFY_PERM) + return -EINVAL; + return 0; +} + static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask, int dfd, const char __user *pathname) { @@ -1018,6 +1034,12 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask, if (ret) goto fput_and_out; + if (flags & FAN_MARK_ADD) { + ret = fanotify_events_supported(&path, mask); + if (ret) + goto path_put_and_out; + } + if (FAN_GROUP_FLAG(group, FAN_REPORT_FID)) { ret = fanotify_test_fid(&path, &__fsid); if (ret) diff --git a/fs/proc/root.c b/fs/proc/root.c index 8b145e7b9661..522199e9525e 100644 --- a/fs/proc/root.c +++ b/fs/proc/root.c @@ -211,7 +211,7 @@ static struct file_system_type proc_fs_type = { .init_fs_context = proc_init_fs_context, .parameters = &proc_fs_parameters, .kill_sb = proc_kill_sb, - .fs_flags = FS_USERNS_MOUNT, + .fs_flags = FS_USERNS_MOUNT | FS_DISALLOW_NOTIFY_PERM, }; void __init proc_root_init(void) -- cgit v1.2.3-59-g8ed1b From 116b9731ad7614032a390bb9ad8998a14d6dc752 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 26 May 2019 17:34:02 +0300 Subject: fsnotify: add empty fsnotify_{unlink,rmdir}() hooks We would like to move fsnotify_nameremove() calls from d_delete() into a higher layer where the hook makes more sense and so we can consider every d_delete() call site individually. Start by creating empty hook fsnotify_{unlink,rmdir}() and place them in the proper VFS call sites. After all d_delete() call sites will be converted to use the new hook, the new hook will generate the delete events and fsnotify_nameremove() hook will be removed. Signed-off-by: Amir Goldstein Signed-off-by: Jan Kara --- fs/namei.c | 2 ++ include/linux/fsnotify.h | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) (limited to 'fs') diff --git a/fs/namei.c b/fs/namei.c index 20831c2fbb34..209c51a5226c 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3883,6 +3883,7 @@ int vfs_rmdir(struct inode *dir, struct dentry *dentry) dentry->d_inode->i_flags |= S_DEAD; dont_mount(dentry); detach_mounts(dentry); + fsnotify_rmdir(dir, dentry); out: inode_unlock(dentry->d_inode); @@ -3999,6 +4000,7 @@ int vfs_unlink(struct inode *dir, struct dentry *dentry, struct inode **delegate if (!error) { dont_mount(dentry); detach_mounts(dentry); + fsnotify_unlink(dir, dentry); } } } diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h index 94972e8eb6d1..7f23eddefcd0 100644 --- a/include/linux/fsnotify.h +++ b/include/linux/fsnotify.h @@ -188,6 +188,19 @@ static inline void fsnotify_link(struct inode *dir, struct inode *inode, struct fsnotify(dir, FS_CREATE, inode, FSNOTIFY_EVENT_INODE, &new_dentry->d_name, 0); } +/* + * fsnotify_unlink - 'name' was unlinked + * + * Caller must make sure that dentry->d_name is stable. + */ +static inline void fsnotify_unlink(struct inode *dir, struct dentry *dentry) +{ + /* Expected to be called before d_delete() */ + WARN_ON_ONCE(d_is_negative(dentry)); + + /* TODO: call fsnotify_dirent() */ +} + /* * fsnotify_mkdir - directory 'name' was created */ @@ -198,6 +211,19 @@ static inline void fsnotify_mkdir(struct inode *inode, struct dentry *dentry) fsnotify_dirent(inode, dentry, FS_CREATE | FS_ISDIR); } +/* + * fsnotify_rmdir - directory 'name' was removed + * + * Caller must make sure that dentry->d_name is stable. + */ +static inline void fsnotify_rmdir(struct inode *dir, struct dentry *dentry) +{ + /* Expected to be called before d_delete() */ + WARN_ON_ONCE(d_is_negative(dentry)); + + /* TODO: call fsnotify_dirent() */ +} + /* * fsnotify_access - file was read */ -- cgit v1.2.3-59-g8ed1b From 46008d9d3f0ef78524c327dbd5cc99843d4b3dae Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 26 May 2019 17:34:03 +0300 Subject: btrfs: call fsnotify_rmdir() hook This will allow generating fsnotify delete events after the fsnotify_nameremove() hook is removed from d_delete(). Signed-off-by: Amir Goldstein Acked-by: David Sterba Signed-off-by: Jan Kara --- fs/btrfs/ioctl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 6dafa857bbb9..2cfd1bfb3871 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -2930,8 +2930,10 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file, inode_lock(inode); err = btrfs_delete_subvolume(dir, dentry); inode_unlock(inode); - if (!err) + if (!err) { + fsnotify_rmdir(dir, dentry); d_delete(dentry); + } out_dput: dput(dentry); -- cgit v1.2.3-59-g8ed1b From 4bf2377472c7ca8f9523f8ccce9ade212483dba4 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 26 May 2019 17:34:05 +0300 Subject: tracefs: call fsnotify_{unlink,rmdir}() hooks This will allow generating fsnotify delete events after the fsnotify_nameremove() hook is removed from d_delete(). Cc: Steven Rostedt Signed-off-by: Amir Goldstein Signed-off-by: Jan Kara --- fs/tracefs/inode.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'fs') diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c index 7098c49f3693..497a8682b5b9 100644 --- a/fs/tracefs/inode.c +++ b/fs/tracefs/inode.c @@ -509,9 +509,12 @@ static int __tracefs_remove(struct dentry *dentry, struct dentry *parent) switch (dentry->d_inode->i_mode & S_IFMT) { case S_IFDIR: ret = simple_rmdir(parent->d_inode, dentry); + if (!ret) + fsnotify_rmdir(parent->d_inode, dentry); break; default: simple_unlink(parent->d_inode, dentry); + fsnotify_unlink(parent->d_inode, dentry); break; } if (!ret) -- cgit v1.2.3-59-g8ed1b From fd0d506f2b8247b3defcca2723622e404fb55c64 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 26 May 2019 17:34:06 +0300 Subject: devpts: call fsnotify_unlink() hook This will allow generating fsnotify delete events after the fsnotify_nameremove() hook is removed from d_delete(). Signed-off-by: Amir Goldstein Signed-off-by: Jan Kara --- fs/devpts/inode.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs') diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c index 2c14ae044dce..beeadca23b05 100644 --- a/fs/devpts/inode.c +++ b/fs/devpts/inode.c @@ -621,6 +621,7 @@ void devpts_pty_kill(struct dentry *dentry) dentry->d_fsdata = NULL; drop_nlink(dentry->d_inode); + fsnotify_unlink(d_inode(dentry->d_parent), dentry); d_delete(dentry); dput(dentry); /* d_alloc_name() in devpts_pty_new() */ } -- cgit v1.2.3-59-g8ed1b From 823e545c027795997f29ec5c255aff605cf39e85 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 26 May 2019 17:34:07 +0300 Subject: debugfs: simplify __debugfs_remove_file() Move simple_unlink()+d_delete() from __debugfs_remove_file() into caller __debugfs_remove() and rename helper for post remove file to __debugfs_file_removed(). This will simplify adding fsnotify_unlink() hook. Cc: Greg Kroah-Hartman Reviewed-by: Greg Kroah-Hartman Signed-off-by: Amir Goldstein Signed-off-by: Jan Kara --- fs/debugfs/inode.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) (limited to 'fs') diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index acef14ad53db..d89874da9791 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -617,13 +617,10 @@ struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent, } EXPORT_SYMBOL_GPL(debugfs_create_symlink); -static void __debugfs_remove_file(struct dentry *dentry, struct dentry *parent) +static void __debugfs_file_removed(struct dentry *dentry) { struct debugfs_fsdata *fsd; - simple_unlink(d_inode(parent), dentry); - d_delete(dentry); - /* * Paired with the closing smp_mb() implied by a successful * cmpxchg() in debugfs_file_get(): either @@ -644,16 +641,15 @@ static int __debugfs_remove(struct dentry *dentry, struct dentry *parent) if (simple_positive(dentry)) { dget(dentry); - if (!d_is_reg(dentry)) { - if (d_is_dir(dentry)) - ret = simple_rmdir(d_inode(parent), dentry); - else - simple_unlink(d_inode(parent), dentry); - if (!ret) - d_delete(dentry); + if (d_is_dir(dentry)) { + ret = simple_rmdir(d_inode(parent), dentry); } else { - __debugfs_remove_file(dentry, parent); + simple_unlink(d_inode(parent), dentry); } + if (!ret) + d_delete(dentry); + if (d_is_reg(dentry)) + __debugfs_file_removed(dentry); dput(dentry); } return ret; -- cgit v1.2.3-59-g8ed1b From 6679ea6dea15ec2fa7e2dd3c11cce639270c4386 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 26 May 2019 17:34:08 +0300 Subject: debugfs: call fsnotify_{unlink,rmdir}() hooks This will allow generating fsnotify delete events after the fsnotify_nameremove() hook is removed from d_delete(). Cc: Greg Kroah-Hartman Reviewed-by: Greg Kroah-Hartman Signed-off-by: Amir Goldstein Signed-off-by: Jan Kara --- fs/debugfs/inode.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'fs') diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index d89874da9791..1e444fe1f778 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -643,8 +643,11 @@ static int __debugfs_remove(struct dentry *dentry, struct dentry *parent) dget(dentry); if (d_is_dir(dentry)) { ret = simple_rmdir(d_inode(parent), dentry); + if (!ret) + fsnotify_rmdir(d_inode(parent), dentry); } else { simple_unlink(d_inode(parent), dentry); + fsnotify_unlink(d_inode(parent), dentry); } if (!ret) d_delete(dentry); -- cgit v1.2.3-59-g8ed1b From 6146e78c0364a881af9bec42eb40882b1e329327 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 26 May 2019 17:34:09 +0300 Subject: configfs: call fsnotify_rmdir() hook This will allow generating fsnotify delete events on unregister of group/subsystem after the fsnotify_nameremove() hook is removed from d_delete(). The rest of the d_delete() calls from this filesystem are either called recursively from within debugfs_unregister_{group,subsystem}, called from a vfs function that already has delete hooks or are called from shutdown/cleanup code. Cc: Joel Becker Cc: Christoph Hellwig Signed-off-by: Amir Goldstein Signed-off-by: Jan Kara --- fs/configfs/dir.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'fs') diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 5e7932d668ab..ba17881a8d84 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -27,6 +27,7 @@ #undef DEBUG #include +#include #include #include #include @@ -1804,6 +1805,7 @@ void configfs_unregister_group(struct config_group *group) configfs_detach_group(&group->cg_item); d_inode(dentry)->i_flags |= S_DEAD; dont_mount(dentry); + fsnotify_rmdir(d_inode(parent), dentry); d_delete(dentry); inode_unlock(d_inode(parent)); @@ -1932,6 +1934,7 @@ void configfs_unregister_subsystem(struct configfs_subsystem *subsys) configfs_detach_group(&group->cg_item); d_inode(dentry)->i_flags |= S_DEAD; dont_mount(dentry); + fsnotify_rmdir(d_inode(root), dentry); inode_unlock(d_inode(dentry)); d_delete(dentry); -- cgit v1.2.3-59-g8ed1b From 49246466a98996e78b68a0041807dbd2628c53fe Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 26 May 2019 17:34:10 +0300 Subject: fsnotify: move fsnotify_nameremove() hook out of d_delete() d_delete() was piggy backed for the fsnotify_nameremove() hook when in fact not all callers of d_delete() care about fsnotify events. For all callers of d_delete() that may be interested in fsnotify events, we made sure to call one of fsnotify_{unlink,rmdir}() hooks before calling d_delete(). Now we can move the fsnotify_nameremove() call from d_delete() to the fsnotify_{unlink,rmdir}() hooks. Two explicit calls to fsnotify_nameremove() from nfs/afs sillyrename are also removed. This will cause a change of behavior - nfs/afs will NOT generate an fsnotify delete event when renaming over a positive dentry. This change is desirable, because it is consistent with the behavior of all other filesystems. Signed-off-by: Amir Goldstein Signed-off-by: Jan Kara --- fs/afs/dir_silly.c | 5 ----- fs/dcache.c | 2 -- fs/nfs/unlink.c | 6 ------ include/linux/fsnotify.h | 2 ++ 4 files changed, 2 insertions(+), 13 deletions(-) (limited to 'fs') diff --git a/fs/afs/dir_silly.c b/fs/afs/dir_silly.c index 057b8d322422..361088a5edb9 100644 --- a/fs/afs/dir_silly.c +++ b/fs/afs/dir_silly.c @@ -60,11 +60,6 @@ static int afs_do_silly_rename(struct afs_vnode *dvnode, struct afs_vnode *vnode if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags)) afs_edit_dir_add(dvnode, &new->d_name, &vnode->fid, afs_edit_dir_for_silly_1); - - /* vfs_unlink and the like do not issue this when a file is - * sillyrenamed, so do it here. - */ - fsnotify_nameremove(old, 0); } kfree(scb); diff --git a/fs/dcache.c b/fs/dcache.c index c435398f2c81..f41121e5d1ec 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -2372,7 +2372,6 @@ EXPORT_SYMBOL(d_hash_and_lookup); void d_delete(struct dentry * dentry) { struct inode *inode = dentry->d_inode; - int isdir = d_is_dir(dentry); spin_lock(&inode->i_lock); spin_lock(&dentry->d_lock); @@ -2387,7 +2386,6 @@ void d_delete(struct dentry * dentry) spin_unlock(&dentry->d_lock); spin_unlock(&inode->i_lock); } - fsnotify_nameremove(dentry, isdir); } EXPORT_SYMBOL(d_delete); diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c index 52d533967485..0effeee28352 100644 --- a/fs/nfs/unlink.c +++ b/fs/nfs/unlink.c @@ -396,12 +396,6 @@ nfs_complete_sillyrename(struct rpc_task *task, struct nfs_renamedata *data) nfs_cancel_async_unlink(dentry); return; } - - /* - * vfs_unlink and the like do not issue this when a file is - * sillyrenamed, so do it here. - */ - fsnotify_nameremove(dentry, 0); } #define SILLYNAME_PREFIX ".nfs" diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h index 7f23eddefcd0..0145073c2b42 100644 --- a/include/linux/fsnotify.h +++ b/include/linux/fsnotify.h @@ -199,6 +199,7 @@ static inline void fsnotify_unlink(struct inode *dir, struct dentry *dentry) WARN_ON_ONCE(d_is_negative(dentry)); /* TODO: call fsnotify_dirent() */ + fsnotify_nameremove(dentry, 0); } /* @@ -222,6 +223,7 @@ static inline void fsnotify_rmdir(struct inode *dir, struct dentry *dentry) WARN_ON_ONCE(d_is_negative(dentry)); /* TODO: call fsnotify_dirent() */ + fsnotify_nameremove(dentry, 1); } /* -- cgit v1.2.3-59-g8ed1b From 7377f5bec13332bc470856f337935be6cabbcf24 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Sun, 26 May 2019 17:34:11 +0300 Subject: fsnotify: get rid of fsnotify_nameremove() For all callers of fsnotify_{unlink,rmdir}(), we made sure that d_parent and d_name are stable. Therefore, fsnotify_{unlink,rmdir}() do not need the safety measures in fsnotify_nameremove() to stabilize parent and name. We can now simplify those hooks and get rid of fsnotify_nameremove(). Signed-off-by: Amir Goldstein Signed-off-by: Jan Kara --- fs/notify/fsnotify.c | 41 ---------------------------------------- include/linux/fsnotify.h | 6 ++---- include/linux/fsnotify_backend.h | 4 ---- 3 files changed, 2 insertions(+), 49 deletions(-) (limited to 'fs') diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index 4eb2ebfac468..2ecef6155fc0 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -94,47 +94,6 @@ void fsnotify_sb_delete(struct super_block *sb) fsnotify_clear_marks_by_sb(sb); } -/* - * fsnotify_nameremove - a filename was removed from a directory - * - * This is mostly called under parent vfs inode lock so name and - * dentry->d_parent should be stable. However there are some corner cases where - * inode lock is not held. So to be on the safe side and be reselient to future - * callers and out of tree users of d_delete(), we do not assume that d_parent - * and d_name are stable and we use dget_parent() and - * take_dentry_name_snapshot() to grab stable references. - */ -void fsnotify_nameremove(struct dentry *dentry, int isdir) -{ - struct dentry *parent; - struct name_snapshot name; - __u32 mask = FS_DELETE; - - /* d_delete() of pseudo inode? (e.g. __ns_get_path() playing tricks) */ - if (IS_ROOT(dentry)) - return; - - if (isdir) - mask |= FS_ISDIR; - - parent = dget_parent(dentry); - /* Avoid unneeded take_dentry_name_snapshot() */ - if (!(d_inode(parent)->i_fsnotify_mask & FS_DELETE) && - !(dentry->d_sb->s_fsnotify_mask & FS_DELETE)) - goto out_dput; - - take_dentry_name_snapshot(&name, dentry); - - fsnotify(d_inode(parent), mask, d_inode(dentry), FSNOTIFY_EVENT_INODE, - &name.name, 0); - - release_dentry_name_snapshot(&name); - -out_dput: - dput(parent); -} -EXPORT_SYMBOL(fsnotify_nameremove); - /* * Given an inode, first check if we care what happens to our children. Inotify * and dnotify both tell their parents about events. If we care about any event diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h index 0145073c2b42..a2d5d175d3c1 100644 --- a/include/linux/fsnotify.h +++ b/include/linux/fsnotify.h @@ -198,8 +198,7 @@ static inline void fsnotify_unlink(struct inode *dir, struct dentry *dentry) /* Expected to be called before d_delete() */ WARN_ON_ONCE(d_is_negative(dentry)); - /* TODO: call fsnotify_dirent() */ - fsnotify_nameremove(dentry, 0); + fsnotify_dirent(dir, dentry, FS_DELETE); } /* @@ -222,8 +221,7 @@ static inline void fsnotify_rmdir(struct inode *dir, struct dentry *dentry) /* Expected to be called before d_delete() */ WARN_ON_ONCE(d_is_negative(dentry)); - /* TODO: call fsnotify_dirent() */ - fsnotify_nameremove(dentry, 1); + fsnotify_dirent(dir, dentry, FS_DELETE | FS_ISDIR); } /* diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h index a9f9dcc1e515..c28f6ed1f59b 100644 --- a/include/linux/fsnotify_backend.h +++ b/include/linux/fsnotify_backend.h @@ -355,7 +355,6 @@ extern int __fsnotify_parent(const struct path *path, struct dentry *dentry, __u extern void __fsnotify_inode_delete(struct inode *inode); extern void __fsnotify_vfsmount_delete(struct vfsmount *mnt); extern void fsnotify_sb_delete(struct super_block *sb); -extern void fsnotify_nameremove(struct dentry *dentry, int isdir); extern u32 fsnotify_get_cookie(void); static inline int fsnotify_inode_watches_children(struct inode *inode) @@ -525,9 +524,6 @@ static inline void __fsnotify_vfsmount_delete(struct vfsmount *mnt) static inline void fsnotify_sb_delete(struct super_block *sb) {} -static inline void fsnotify_nameremove(struct dentry *dentry, int isdir) -{} - static inline void fsnotify_update_flags(struct dentry *dentry) {} -- cgit v1.2.3-59-g8ed1b