On Mon, Feb 03, 2025 at 11:32:03PM +0100, Amir Goldstein wrote:
> The FMODE_NONOTIFY_* bits are a 2-bits mode. Open coding manipulation
> of those bits is risky. Use an accessor file_set_fsnotify_mode() to
> set the mode.
>
> Rename file_set_fsnotify_mode() => file_set_fsnotify_mode_from_watchers()
> to make way for the simple accessor name.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> drivers/tty/pty.c | 2 +-
> fs/notify/fsnotify.c | 18 ++++++++++++------
> fs/open.c | 7 ++++---
> include/linux/fs.h | 9 ++++++++-
> include/linux/fsnotify.h | 4 ++--
> 5 files changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> index df08f13052ff4..8bb1a01fef2a1 100644
> --- a/drivers/tty/pty.c
> +++ b/drivers/tty/pty.c
> @@ -798,7 +798,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
> nonseekable_open(inode, filp);
>
> /* We refuse fsnotify events on ptmx, since it's a shared resource */
> - filp->f_mode |= FMODE_NONOTIFY;
> + file_set_fsnotify_mode(filp, FMODE_NONOTIFY);
>
> retval = tty_alloc_file(filp);
> if (retval)
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 8ee495a58d0ad..77a1521335a10 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -648,7 +648,7 @@ EXPORT_SYMBOL_GPL(fsnotify);
> * 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(struct file *file)
> +void file_set_fsnotify_mode_from_watchers(struct file *file)
> {
> struct dentry *dentry = file->f_path.dentry, *parent;
> struct super_block *sb = dentry->d_sb;
> @@ -665,7 +665,7 @@ void file_set_fsnotify_mode(struct file *file)
> */
> if (likely(!fsnotify_sb_has_priority_watchers(sb,
> FSNOTIFY_PRIO_CONTENT))) {
> - file->f_mode |= FMODE_NONOTIFY_PERM;
> + file_set_fsnotify_mode(file, FMODE_NONOTIFY_PERM);
> return;
> }
>
> @@ -676,7 +676,7 @@ void file_set_fsnotify_mode(struct file *file)
> if ((!d_is_dir(dentry) && !d_is_reg(dentry)) ||
> likely(!fsnotify_sb_has_priority_watchers(sb,
> FSNOTIFY_PRIO_PRE_CONTENT))) {
> - file->f_mode |= FMODE_NONOTIFY | FMODE_NONOTIFY_PERM;
> + file_set_fsnotify_mode(file, FMODE_NONOTIFY_HSM);
> return;
> }
>
> @@ -686,19 +686,25 @@ void file_set_fsnotify_mode(struct file *file)
> */
> 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)))
> + FSNOTIFY_PRE_CONTENT_EVENTS))) {
> + /* Enable pre-content events */
> + file_set_fsnotify_mode(file, 0);
> return;
> + }
>
> /* 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)
> + if (p_mask & FSNOTIFY_PRE_CONTENT_EVENTS) {
> + /* Enable pre-content events */
> + file_set_fsnotify_mode(file, 0);
> return;
> + }
> }
> /* Nobody watching for pre-content events from this file */
> - file->f_mode |= FMODE_NONOTIFY | FMODE_NONOTIFY_PERM;
> + file_set_fsnotify_mode(file, FMODE_NONOTIFY_HSM);
> }
> #endif
>
> diff --git a/fs/open.c b/fs/open.c
> index 932e5a6de63bb..3fcbfff8aede8 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -905,7 +905,8 @@ static int do_dentry_open(struct file *f,
> f->f_sb_err = file_sample_sb_err(f);
>
> if (unlikely(f->f_flags & O_PATH)) {
> - f->f_mode = FMODE_PATH | FMODE_OPENED | FMODE_NONOTIFY;
> + f->f_mode = FMODE_PATH | FMODE_OPENED;
> + file_set_fsnotify_mode(f, FMODE_NONOTIFY);
> f->f_op = &empty_fops;
> return 0;
> }
> @@ -938,7 +939,7 @@ static int do_dentry_open(struct file *f,
> * If FMODE_NONOTIFY was already set for an fanotify fd, this doesn't
> * change anything.
> */
> - file_set_fsnotify_mode(f);
> + file_set_fsnotify_mode_from_watchers(f);
> error = fsnotify_open_perm(f);
> if (error)
> goto cleanup_all;
> @@ -1122,7 +1123,7 @@ struct file *dentry_open_nonotify(const struct path *path, int flags,
> if (!IS_ERR(f)) {
> int error;
>
> - f->f_mode |= FMODE_NONOTIFY;
> + file_set_fsnotify_mode(f, FMODE_NONOTIFY);
> error = vfs_open(path, f);
> if (error) {
> fput(f);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index be3ad155ec9f7..e73d9b998780d 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -206,6 +206,8 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
> * FMODE_NONOTIFY_PERM - suppress permission (incl. pre-content) events.
> * FMODE_NONOTIFY | FMODE_NONOTIFY_PERM - suppress only pre-content events.
> */
> +#define FMODE_NONOTIFY_HSM \
> + (FMODE_NONOTIFY | FMODE_NONOTIFY_PERM)
After this patch series this define is used exactly twice and it's
currently identical to FMODE_FSNOTIFY_HSM. I suggest to remove it and
simply pass FMODE_NONOTIFY | FMODE_NONOTIFY_PERM in the two places it's
used. I can do this myself though so if Jan doesn't have other comments
don't bother resending.
> #define FMODE_FSNOTIFY_MASK \
> (FMODE_NONOTIFY | FMODE_NONOTIFY_PERM)
>
> @@ -222,7 +224,6 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
> #define FMODE_FSNOTIFY_HSM(mode) 0
> #endif
>
> -
> /*
> * Attribute flags. These should be or-ed together to figure out what
> * has been changed!
> @@ -3140,6 +3141,12 @@ static inline void exe_file_allow_write_access(struct file *exe_file)
> allow_write_access(exe_file);
> }
>
> +static inline void file_set_fsnotify_mode(struct file *file, fmode_t mode)
> +{
> + file->f_mode &= ~FMODE_FSNOTIFY_MASK;
> + file->f_mode |= mode;
> +}
> +
> static inline bool inode_is_open_for_write(const struct inode *inode)
> {
> return atomic_read(&inode->i_writecount) > 0;
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index 1a9ef8f6784dd..6a33288bd6a1f 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(struct file *file);
> +void file_set_fsnotify_mode_from_watchers(struct file *file);
>
> /*
> * fsnotify_file_area_perm - permission hook before access to file range
> @@ -213,7 +213,7 @@ static inline int fsnotify_open_perm(struct file *file)
> }
>
> #else
> -static inline void file_set_fsnotify_mode(struct file *file)
> +static inline void file_set_fsnotify_mode_from_watchers(struct file *file)
> {
> }
>
> --
> 2.34.1
>
