From 8631e01c2c5d1fe6705bcc0d733a0b7a17d3daac Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Fri, 27 Jun 2025 12:48:35 +0200 Subject: fanotify: sanitize handle_type values when reporting fid Unlike file_handle, type and len of struct fanotify_fh are u8. Traditionally, filesystem return handle_type < 0xff, but there is no enforecement for that in vfs. Add a sanity check in fanotify to avoid truncating handle_type if its value is > 0xff. Fixes: 7cdafe6cc4a6 ("exportfs: check for error return value from exportfs_encode_*()") Signed-off-by: Amir Goldstein Signed-off-by: Jan Kara Link: https://patch.msgid.link/20250627104835.184495-1-amir73il@gmail.com --- fs/notify/fanotify/fanotify.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'fs/notify') diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index 3083643b864b..bfe884d624e7 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -454,7 +454,13 @@ static int fanotify_encode_fh(struct fanotify_fh *fh, struct inode *inode, dwords = fh_len >> 2; type = exportfs_encode_fid(inode, buf, &dwords); err = -EINVAL; - if (type <= 0 || type == FILEID_INVALID || fh_len != dwords << 2) + /* + * Unlike file_handle, type and len of struct fanotify_fh are u8. + * Traditionally, filesystem return handle_type < 0xff, but there + * is no enforecement for that in vfs. + */ + BUILD_BUG_ON(MAX_HANDLE_SZ > 0xff || FILEID_INVALID > 0xff); + if (type <= 0 || type >= FILEID_INVALID || fh_len != dwords << 2) goto out_err; fh->type = type; -- cgit v1.2.3 From 08da98e1b2f76cdbacf84b9affaa75960dbce515 Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Tue, 8 Jul 2025 16:36:40 +0200 Subject: fsnotify: merge file_set_fsnotify_mode_from_watchers() with open perm hook Create helper fsnotify_open_perm_and_set_mode() that moves the fsnotify_open_perm() hook into file_set_fsnotify_mode_from_watchers(). This will allow some more optimizations. Signed-off-by: Amir Goldstein Signed-off-by: Jan Kara Link: https://patch.msgid.link/20250708143641.418603-2-amir73il@gmail.com --- fs/file_table.c | 2 +- fs/notify/fsnotify.c | 22 +++++++++++++--------- fs/open.c | 6 +++--- include/linux/fsnotify.h | 8 +++----- 4 files changed, 20 insertions(+), 18 deletions(-) (limited to 'fs/notify') diff --git a/fs/file_table.c b/fs/file_table.c index 138114d64307..14ee3581c768 100644 --- a/fs/file_table.c +++ b/fs/file_table.c @@ -196,7 +196,7 @@ static int init_file(struct file *f, int flags, const struct cred *cred) file_ref_init(&f->f_ref, 1); /* * Disable permission and pre-content events for all files by default. - * They may be enabled later by file_set_fsnotify_mode_from_watchers(). + * They may be enabled later by fsnotify_open_perm_and_set_mode(). */ file_set_fsnotify_mode(f, FMODE_NONOTIFY_PERM); return 0; diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index e2b4f17a48bb..de7e7425428b 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -656,12 +656,12 @@ EXPORT_SYMBOL_GPL(fsnotify); #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS /* - * At open time we check fsnotify_sb_has_priority_watchers() and set the - * FMODE_NONOTIFY_ mode bits accordignly. + * At open time we check fsnotify_sb_has_priority_watchers(), call the open perm + * hook and set the FMODE_NONOTIFY_ mode bits accordignly. * Later, fsnotify permission hooks do not check if there are permission event * watches, but that there were permission event watches at open time. */ -void file_set_fsnotify_mode_from_watchers(struct file *file) +int fsnotify_open_perm_and_set_mode(struct file *file) { struct dentry *dentry = file->f_path.dentry, *parent; struct super_block *sb = dentry->d_sb; @@ -669,7 +669,7 @@ void file_set_fsnotify_mode_from_watchers(struct file *file) /* Is it a file opened by fanotify? */ if (FMODE_FSNOTIFY_NONE(file->f_mode)) - return; + return 0; /* * Permission events is a super set of pre-content events, so if there @@ -679,7 +679,7 @@ void file_set_fsnotify_mode_from_watchers(struct file *file) if (likely(!fsnotify_sb_has_priority_watchers(sb, FSNOTIFY_PRIO_CONTENT))) { file_set_fsnotify_mode(file, FMODE_NONOTIFY_PERM); - return; + return 0; } /* @@ -689,8 +689,9 @@ void file_set_fsnotify_mode_from_watchers(struct file *file) if ((!d_is_dir(dentry) && !d_is_reg(dentry)) || likely(!fsnotify_sb_has_priority_watchers(sb, FSNOTIFY_PRIO_PRE_CONTENT))) { - file_set_fsnotify_mode(file, FMODE_NONOTIFY | FMODE_NONOTIFY_PERM); - return; + file_set_fsnotify_mode(file, FMODE_NONOTIFY | + FMODE_NONOTIFY_PERM); + goto open_perm; } /* @@ -702,7 +703,7 @@ void file_set_fsnotify_mode_from_watchers(struct file *file) FSNOTIFY_PRE_CONTENT_EVENTS))) { /* Enable pre-content events */ file_set_fsnotify_mode(file, 0); - return; + goto open_perm; } /* Is parent watching for pre-content events on this file? */ @@ -713,11 +714,14 @@ void file_set_fsnotify_mode_from_watchers(struct file *file) if (p_mask & FSNOTIFY_PRE_CONTENT_EVENTS) { /* Enable pre-content events */ file_set_fsnotify_mode(file, 0); - return; + goto open_perm; } } /* Nobody watching for pre-content events from this file */ file_set_fsnotify_mode(file, FMODE_NONOTIFY | FMODE_NONOTIFY_PERM); + +open_perm: + return fsnotify_open_perm(file); } #endif diff --git a/fs/open.c b/fs/open.c index 7828234a7caa..f240b96ce586 100644 --- a/fs/open.c +++ b/fs/open.c @@ -943,12 +943,12 @@ static int do_dentry_open(struct file *f, goto cleanup_all; /* - * Set FMODE_NONOTIFY_* bits according to existing permission watches. + * Call fsnotify open permission hook and set FMODE_NONOTIFY_* bits + * according to existing permission watches. * If FMODE_NONOTIFY mode was already set for an fanotify fd or for a * pseudo file, this call will not change the mode. */ - file_set_fsnotify_mode_from_watchers(f); - error = fsnotify_open_perm(f); + error = fsnotify_open_perm_and_set_mode(f); if (error) goto cleanup_all; diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h index 454d8e466958..8c1fa617d375 100644 --- a/include/linux/fsnotify.h +++ b/include/linux/fsnotify.h @@ -129,7 +129,7 @@ static inline int fsnotify_file(struct file *file, __u32 mask) #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS -void file_set_fsnotify_mode_from_watchers(struct file *file); +int fsnotify_open_perm_and_set_mode(struct file *file); /* * fsnotify_file_area_perm - permission hook before access to file range @@ -215,9 +215,6 @@ static inline int fsnotify_open_perm(struct file *file) { int ret; - if (likely(!FMODE_FSNOTIFY_PERM(file->f_mode))) - return 0; - if (file->f_flags & __FMODE_EXEC) { ret = fsnotify_path(&file->f_path, FS_OPEN_EXEC_PERM); if (ret) @@ -228,8 +225,9 @@ static inline int fsnotify_open_perm(struct file *file) } #else -static inline void file_set_fsnotify_mode_from_watchers(struct file *file) +static inline int fsnotify_open_perm_and_set_mode(struct file *file) { + return 0; } static inline int fsnotify_file_area_perm(struct file *file, int perm_mask, -- cgit v1.2.3 From 0d4c4d4ea443babab6ec1a79f481260963fc969a Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Tue, 8 Jul 2025 16:36:41 +0200 Subject: fsnotify: optimize FMODE_NONOTIFY_PERM for the common cases The most unlikely watched permission event is FAN_ACCESS_PERM, because at the time that it was introduced there were no evictable ignore mark, so subscribing to FAN_ACCESS_PERM would have incured a very high overhead. Yet, when we set the fmode to FMODE_NOTIFY_HSM(), we never skip trying to send FAN_ACCESS_PERM, which is almost always a waste of cycles. We got to this logic because of bundling FAN_OPEN*_PERM and FAN_ACCESS_PERM in the same category and because FAN_OPEN_PERM is a commonly used event. By open coding fsnotify_open_perm() in fsnotify_open_perm_and_set_mode(), we no longer need to regard FAN_OPEN*_PERM when calculating fmode. This leaves the case of having pre-content events and not having any other permission event in the object masks a more likely case than the other way around. Rework the fmode macros and code so that their meaning now refers only to hooks on an already open file: - FMODE_NOTIFY_NONE() skip all events - FMODE_NOTIFY_ACCESS_PERM() send all permission events including FAN_ACCESS_PERM - FMODE_NOTIFY_HSM() send pre-content permission events Signed-off-by: Amir Goldstein Signed-off-by: Jan Kara Link: https://patch.msgid.link/20250708143641.418603-3-amir73il@gmail.com --- fs/notify/fsnotify.c | 75 +++++++++++++++++++++++++++++------------------- include/linux/fs.h | 12 ++++---- include/linux/fsnotify.h | 27 ++--------------- 3 files changed, 53 insertions(+), 61 deletions(-) (limited to 'fs/notify') diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index de7e7425428b..079b868552c2 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -199,8 +199,8 @@ static bool fsnotify_event_needs_parent(struct inode *inode, __u32 mnt_mask, } /* Are there any inode/mount/sb objects that watch for these events? */ -static inline bool fsnotify_object_watched(struct inode *inode, __u32 mnt_mask, - __u32 mask) +static inline __u32 fsnotify_object_watched(struct inode *inode, __u32 mnt_mask, + __u32 mask) { __u32 marks_mask = READ_ONCE(inode->i_fsnotify_mask) | mnt_mask | READ_ONCE(inode->i_sb->s_fsnotify_mask); @@ -665,7 +665,7 @@ int fsnotify_open_perm_and_set_mode(struct file *file) { struct dentry *dentry = file->f_path.dentry, *parent; struct super_block *sb = dentry->d_sb; - __u32 mnt_mask, p_mask; + __u32 mnt_mask, p_mask = 0; /* Is it a file opened by fanotify? */ if (FMODE_FSNOTIFY_NONE(file->f_mode)) @@ -683,45 +683,60 @@ int fsnotify_open_perm_and_set_mode(struct file *file) } /* - * If there are permission event watchers but no pre-content event - * watchers, set FMODE_NONOTIFY | FMODE_NONOTIFY_PERM to indicate that. + * OK, there are some permission event watchers. Check if anybody is + * watching for permission events on *this* file. */ - if ((!d_is_dir(dentry) && !d_is_reg(dentry)) || - likely(!fsnotify_sb_has_priority_watchers(sb, - FSNOTIFY_PRIO_PRE_CONTENT))) { - file_set_fsnotify_mode(file, FMODE_NONOTIFY | - FMODE_NONOTIFY_PERM); - goto open_perm; + mnt_mask = READ_ONCE(real_mount(file->f_path.mnt)->mnt_fsnotify_mask); + p_mask = fsnotify_object_watched(d_inode(dentry), mnt_mask, + ALL_FSNOTIFY_PERM_EVENTS); + if (dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED) { + parent = dget_parent(dentry); + p_mask |= fsnotify_inode_watches_children(d_inode(parent)); + dput(parent); } /* - * OK, there are some pre-content watchers. Check if anybody is - * watching for pre-content events on *this* file. + * Legacy FAN_ACCESS_PERM events have very high performance overhead, + * so unlikely to be used in the wild. If they are used there will be + * no optimizations at all. */ - mnt_mask = READ_ONCE(real_mount(file->f_path.mnt)->mnt_fsnotify_mask); - if (unlikely(fsnotify_object_watched(d_inode(dentry), mnt_mask, - FSNOTIFY_PRE_CONTENT_EVENTS))) { - /* Enable pre-content events */ + if (unlikely(p_mask & FS_ACCESS_PERM)) { + /* Enable all permission and pre-content events */ file_set_fsnotify_mode(file, 0); goto open_perm; } - /* Is parent watching for pre-content events on this file? */ - if (dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED) { - parent = dget_parent(dentry); - p_mask = fsnotify_inode_watches_children(d_inode(parent)); - dput(parent); - if (p_mask & FSNOTIFY_PRE_CONTENT_EVENTS) { - /* Enable pre-content events */ - file_set_fsnotify_mode(file, 0); - goto open_perm; - } + /* + * Pre-content events are only supported on regular files. + * If there are pre-content event watchers and no permission access + * watchers, set FMODE_NONOTIFY | FMODE_NONOTIFY_PERM to indicate that. + * That is the common case with HSM service. + */ + if (d_is_reg(dentry) && (p_mask & FSNOTIFY_PRE_CONTENT_EVENTS)) { + file_set_fsnotify_mode(file, FMODE_NONOTIFY | + FMODE_NONOTIFY_PERM); + goto open_perm; } - /* Nobody watching for pre-content events from this file */ - file_set_fsnotify_mode(file, FMODE_NONOTIFY | FMODE_NONOTIFY_PERM); + + /* Nobody watching permission and pre-content events on this file */ + file_set_fsnotify_mode(file, FMODE_NONOTIFY_PERM); open_perm: - return fsnotify_open_perm(file); + /* + * Send open perm events depending on object masks and regardless of + * FMODE_NONOTIFY_PERM. + */ + if (file->f_flags & __FMODE_EXEC && p_mask & FS_OPEN_EXEC_PERM) { + int ret = fsnotify_path(&file->f_path, FS_OPEN_EXEC_PERM); + + if (ret) + return ret; + } + + if (p_mask & FS_OPEN_PERM) + return fsnotify_path(&file->f_path, FS_OPEN_PERM); + + return 0; } #endif diff --git a/include/linux/fs.h b/include/linux/fs.h index b085f161ed22..bc92fdb8bfcc 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -200,12 +200,12 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset, /* * The two FMODE_NONOTIFY* define which fsnotify events should not be generated - * for a file. These are the possible values of (f->f_mode & - * FMODE_FSNOTIFY_MASK) and their meaning: + * for an open file. These are the possible values of + * (f->f_mode & FMODE_FSNOTIFY_MASK) and their meaning: * * FMODE_NONOTIFY - suppress all (incl. non-permission) events. * FMODE_NONOTIFY_PERM - suppress permission (incl. pre-content) events. - * FMODE_NONOTIFY | FMODE_NONOTIFY_PERM - suppress only pre-content events. + * FMODE_NONOTIFY | FMODE_NONOTIFY_PERM - suppress only FAN_ACCESS_PERM. */ #define FMODE_FSNOTIFY_MASK \ (FMODE_NONOTIFY | FMODE_NONOTIFY_PERM) @@ -213,13 +213,13 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset, #define FMODE_FSNOTIFY_NONE(mode) \ ((mode & FMODE_FSNOTIFY_MASK) == FMODE_NONOTIFY) #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS -#define FMODE_FSNOTIFY_PERM(mode) \ +#define FMODE_FSNOTIFY_HSM(mode) \ ((mode & FMODE_FSNOTIFY_MASK) == 0 || \ (mode & FMODE_FSNOTIFY_MASK) == (FMODE_NONOTIFY | FMODE_NONOTIFY_PERM)) -#define FMODE_FSNOTIFY_HSM(mode) \ +#define FMODE_FSNOTIFY_ACCESS_PERM(mode) \ ((mode & FMODE_FSNOTIFY_MASK) == 0) #else -#define FMODE_FSNOTIFY_PERM(mode) 0 +#define FMODE_FSNOTIFY_ACCESS_PERM(mode) 0 #define FMODE_FSNOTIFY_HSM(mode) 0 #endif diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h index 8c1fa617d375..28a9cb13fbfa 100644 --- a/include/linux/fsnotify.h +++ b/include/linux/fsnotify.h @@ -147,9 +147,6 @@ static inline int fsnotify_file_area_perm(struct file *file, int perm_mask, if (!(perm_mask & (MAY_READ | MAY_WRITE | MAY_ACCESS))) return 0; - if (likely(!FMODE_FSNOTIFY_PERM(file->f_mode))) - return 0; - /* * read()/write() and other types of access generate pre-content events. */ @@ -160,7 +157,8 @@ static inline int fsnotify_file_area_perm(struct file *file, int perm_mask, return ret; } - if (!(perm_mask & MAY_READ)) + if (!(perm_mask & MAY_READ) || + likely(!FMODE_FSNOTIFY_ACCESS_PERM(file->f_mode))) return 0; /* @@ -208,22 +206,6 @@ static inline int fsnotify_file_perm(struct file *file, int perm_mask) return fsnotify_file_area_perm(file, perm_mask, NULL, 0); } -/* - * fsnotify_open_perm - permission hook before file open - */ -static inline int fsnotify_open_perm(struct file *file) -{ - int ret; - - if (file->f_flags & __FMODE_EXEC) { - ret = fsnotify_path(&file->f_path, FS_OPEN_EXEC_PERM); - if (ret) - return ret; - } - - return fsnotify_path(&file->f_path, FS_OPEN_PERM); -} - #else static inline int fsnotify_open_perm_and_set_mode(struct file *file) { @@ -251,11 +233,6 @@ static inline int fsnotify_file_perm(struct file *file, int perm_mask) { return 0; } - -static inline int fsnotify_open_perm(struct file *file) -{ - return 0; -} #endif /* -- cgit v1.2.3