[PATCH 0/3] Fix for huge faults regression

[PATCH 0/3] Fix for huge faults regression

[Amir Goldstein]

Christian,

I thought these fixes could go through your tree, because they are
mostly vfs/file related.

Hoping that Jan could provide an ACK.

The two Fix patches have been tested by Alex together and each one
independently.

I also verified that they pass the LTP inoityf/fanotify tests.

Thanks,
Amir.

Amir Goldstein (3):
  fsnotify: use accessor to set FMODE_NONOTIFY_*
  fsnotify: disable notification by default for all pseudo files
  fsnotify: disable pre-content and permission events by default

 drivers/tty/pty.c        |  2 +-
 fs/file_table.c          | 16 ++++++++++++++++
 fs/notify/fsnotify.c     | 18 ++++++++++++------
 fs/open.c                | 11 ++++++-----
 fs/pipe.c                |  6 ++++++
 include/linux/fs.h       |  9 ++++++++-
 include/linux/fsnotify.h |  4 ++--
 net/socket.c             |  5 +++++
 8 files changed, 56 insertions(+), 15 deletions(-)

-- 
2.34.1

[PATCH 1/3] fsnotify: use accessor to set FMODE_NONOTIFY_*

[Amir Goldstein]

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@gmail.com>
---
 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)
 #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

[PATCH 2/3] fsnotify: disable notification by default for all pseudo files

[Amir Goldstein]

Most pseudo files are not applicable for fsnotify events at all,
let alone to the new pre-content events.

Disable notifications to all files allocated with alloc_file_pseudo()
and enable legacy inotify events for the specific cases of pipe and
socket, which have known users of inotify events.

Pre-content events are also kept disabled for sockets and pipes.

Fixes: 20bf82a898b6 ("mm: don't allow huge faults for files with pre content watches")
Reported-by: Alex Williamson <alex.williamson@redhat.com>
Closes: https://lore.kernel.org/linux-fsdevel/20250131121703.1e4d00a7.alex.williamson@redhat.com/
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/linux-fsdevel/CAHk-=wi2pThSVY=zhO=ZKxViBj5QCRX-=AS2+rVknQgJnHXDFg@mail.gmail.com/
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/file_table.c | 11 +++++++++++
 fs/open.c       |  4 ++--
 fs/pipe.c       |  6 ++++++
 net/socket.c    |  5 +++++
 4 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/fs/file_table.c b/fs/file_table.c
index f0291a66f9db4..35b93da6c5cb1 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -375,7 +375,13 @@ struct file *alloc_file_pseudo(struct inode *inode, struct vfsmount *mnt,
 	if (IS_ERR(file)) {
 		ihold(inode);
 		path_put(&path);
+		return file;
 	}
+	/*
+	 * Disable all fsnotify events for pseudo files by default.
+	 * They may be enabled by caller with file_set_fsnotify_mode().
+	 */
+	file_set_fsnotify_mode(file, FMODE_NONOTIFY);
 	return file;
 }
 EXPORT_SYMBOL(alloc_file_pseudo);
@@ -400,6 +406,11 @@ struct file *alloc_file_pseudo_noaccount(struct inode *inode,
 		return file;
 	}
 	file_init_path(file, &path, fops);
+	/*
+	 * Disable all fsnotify events for pseudo files by default.
+	 * They may be enabled by caller with file_set_fsnotify_mode().
+	 */
+	file_set_fsnotify_mode(file, FMODE_NONOTIFY);
 	return file;
 }
 EXPORT_SYMBOL_GPL(alloc_file_pseudo_noaccount);
diff --git a/fs/open.c b/fs/open.c
index 3fcbfff8aede8..1be20de9f283a 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -936,8 +936,8 @@ static int do_dentry_open(struct file *f,
 
 	/*
 	 * Set FMODE_NONOTIFY_* bits according to existing permission watches.
-	 * If FMODE_NONOTIFY was already set for an fanotify fd, this doesn't
-	 * change anything.
+	 * 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);
diff --git a/fs/pipe.c b/fs/pipe.c
index 94b59045ab44b..ce1af7592780d 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -960,6 +960,12 @@ int create_pipe_files(struct file **res, int flags)
 	res[1] = f;
 	stream_open(inode, res[0]);
 	stream_open(inode, res[1]);
+	/*
+	 * Disable permission and pre-content events, but enable legacy
+	 * inotify events for legacy users.
+	 */
+	file_set_fsnotify_mode(res[0], FMODE_NONOTIFY_PERM);
+	file_set_fsnotify_mode(res[1], FMODE_NONOTIFY_PERM);
 	return 0;
 }
 
diff --git a/net/socket.c b/net/socket.c
index 262a28b59c7f0..28bae5a942341 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -479,6 +479,11 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
 	sock->file = file;
 	file->private_data = sock;
 	stream_open(SOCK_INODE(sock), file);
+	/*
+	 * Disable permission and pre-content events, but enable legacy
+	 * inotify events for legacy users.
+	 */
+	file_set_fsnotify_mode(file, FMODE_NONOTIFY_PERM);
 	return file;
 }
 EXPORT_SYMBOL(sock_alloc_file);
-- 
2.34.1

[PATCH 3/3] fsnotify: disable pre-content and permission events by default

[Amir Goldstein]

After introducing pre-content events, we had a regression related to
disabling huge faults on files that should never have pre-content events
enabled.

This happened because the default f_mode of allocated files (0) does
not disable pre-content events.

Pre-content events are disabled in file_set_fsnotify_mode_by_watchers()
but internal files may not get to call this helper.

Initialize f_mode to disable permission and pre-content events for all
files and if needed they will be enabled for the callers of
file_set_fsnotify_mode_by_watchers().

Fixes: 20bf82a898b6 ("mm: don't allow huge faults for files with pre content watches")
Reported-by: Alex Williamson <alex.williamson@redhat.com>
Closes: https://lore.kernel.org/linux-fsdevel/20250131121703.1e4d00a7.alex.williamson@redhat.com/
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/file_table.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/file_table.c b/fs/file_table.c
index 35b93da6c5cb1..5c00dc38558da 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -194,6 +194,11 @@ static int init_file(struct file *f, int flags, const struct cred *cred)
 	 * refcount bumps we should reinitialize the reused file first.
 	 */
 	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().
+	 */
+	file_set_fsnotify_mode(f, FMODE_NONOTIFY_PERM);
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH 1/3] fsnotify: use accessor to set FMODE_NONOTIFY_*

[Christian Brauner]

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@gmail.com>
> ---
>  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
> 

Re: [PATCH 1/3] fsnotify: use accessor to set FMODE_NONOTIFY_*

[Amir Goldstein]

On Tue, Feb 4, 2025 at 11:43 AM Christian Brauner <brauner@kernel.org> wrote:
>
> 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@gmail.com>
> > ---
> >  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.
>

I thought that
file_set_fsnotify_mode(file, FMODE_NONOTIFY_HSM);
is more readable then
file_set_fsnotify_mode(FMODE_NONOTIFY | FMODE_NONOTIFY_PERM);
and makes it clearer that the individual bits should not be messed with

but since Jan was one who pushed for open coding
f->f_mode = FMODE_PATH | FMODE_OPENED | FMODE_NONOTIFY;
I think he will agree with you,
so I humbly accept your suggestion :)

Thanks,
Amir.

Re: [PATCH 0/3] Fix for huge faults regression

[Christian Brauner]

On Mon, 03 Feb 2025 23:32:02 +0100, Amir Goldstein wrote:
> Christian,
> 
> I thought these fixes could go through your tree, because they are
> mostly vfs/file related.
> 
> Hoping that Jan could provide an ACK.
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/3] fsnotify: use accessor to set FMODE_NONOTIFY_*
      https://git.kernel.org/vfs/vfs/c/fe1052f7e420
[2/3] fsnotify: disable notification by default for all pseudo files
      https://git.kernel.org/vfs/vfs/c/54dbee0b21e1
[3/3] fsnotify: disable pre-content and permission events by default
      https://git.kernel.org/vfs/vfs/c/af6671679734

Re: [PATCH 1/3] fsnotify: use accessor to set FMODE_NONOTIFY_*

[Jan Kara]

On Tue 04-02-25 11:43:11, Christian Brauner wrote:
> 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@gmail.com>
> > ---
> >  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.

I don't care that much but overall I tend to agree that a helper for two
very localized uses is not too helpful. Either with or without
FMODE_NONOTIFY_HSM feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/3] fsnotify: disable notification by default for all pseudo files

[Jan Kara]

On Mon 03-02-25 23:32:04, Amir Goldstein wrote:
> Most pseudo files are not applicable for fsnotify events at all,
> let alone to the new pre-content events.
> 
> Disable notifications to all files allocated with alloc_file_pseudo()
> and enable legacy inotify events for the specific cases of pipe and
> socket, which have known users of inotify events.
> 
> Pre-content events are also kept disabled for sockets and pipes.
> 
> Fixes: 20bf82a898b6 ("mm: don't allow huge faults for files with pre content watches")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Closes: https://lore.kernel.org/linux-fsdevel/20250131121703.1e4d00a7.alex.williamson@redhat.com/
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Link: https://lore.kernel.org/linux-fsdevel/CAHk-=wi2pThSVY=zhO=ZKxViBj5QCRX-=AS2+rVknQgJnHXDFg@mail.gmail.com/
> Tested-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/file_table.c | 11 +++++++++++
>  fs/open.c       |  4 ++--
>  fs/pipe.c       |  6 ++++++
>  net/socket.c    |  5 +++++
>  4 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/file_table.c b/fs/file_table.c
> index f0291a66f9db4..35b93da6c5cb1 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -375,7 +375,13 @@ struct file *alloc_file_pseudo(struct inode *inode, struct vfsmount *mnt,
>  	if (IS_ERR(file)) {
>  		ihold(inode);
>  		path_put(&path);
> +		return file;
>  	}
> +	/*
> +	 * Disable all fsnotify events for pseudo files by default.
> +	 * They may be enabled by caller with file_set_fsnotify_mode().
> +	 */
> +	file_set_fsnotify_mode(file, FMODE_NONOTIFY);
>  	return file;
>  }
>  EXPORT_SYMBOL(alloc_file_pseudo);
> @@ -400,6 +406,11 @@ struct file *alloc_file_pseudo_noaccount(struct inode *inode,
>  		return file;
>  	}
>  	file_init_path(file, &path, fops);
> +	/*
> +	 * Disable all fsnotify events for pseudo files by default.
> +	 * They may be enabled by caller with file_set_fsnotify_mode().
> +	 */
> +	file_set_fsnotify_mode(file, FMODE_NONOTIFY);
>  	return file;
>  }
>  EXPORT_SYMBOL_GPL(alloc_file_pseudo_noaccount);
> diff --git a/fs/open.c b/fs/open.c
> index 3fcbfff8aede8..1be20de9f283a 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -936,8 +936,8 @@ static int do_dentry_open(struct file *f,
>  
>  	/*
>  	 * Set FMODE_NONOTIFY_* bits according to existing permission watches.
> -	 * If FMODE_NONOTIFY was already set for an fanotify fd, this doesn't
> -	 * change anything.
> +	 * 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);
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 94b59045ab44b..ce1af7592780d 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -960,6 +960,12 @@ int create_pipe_files(struct file **res, int flags)
>  	res[1] = f;
>  	stream_open(inode, res[0]);
>  	stream_open(inode, res[1]);
> +	/*
> +	 * Disable permission and pre-content events, but enable legacy
> +	 * inotify events for legacy users.
> +	 */
> +	file_set_fsnotify_mode(res[0], FMODE_NONOTIFY_PERM);
> +	file_set_fsnotify_mode(res[1], FMODE_NONOTIFY_PERM);
>  	return 0;
>  }
>  
> diff --git a/net/socket.c b/net/socket.c
> index 262a28b59c7f0..28bae5a942341 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -479,6 +479,11 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
>  	sock->file = file;
>  	file->private_data = sock;
>  	stream_open(SOCK_INODE(sock), file);
> +	/*
> +	 * Disable permission and pre-content events, but enable legacy
> +	 * inotify events for legacy users.
> +	 */
> +	file_set_fsnotify_mode(file, FMODE_NONOTIFY_PERM);
>  	return file;
>  }
>  EXPORT_SYMBOL(sock_alloc_file);
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 3/3] fsnotify: disable pre-content and permission events by default

[Jan Kara]

On Mon 03-02-25 23:32:05, Amir Goldstein wrote:
> After introducing pre-content events, we had a regression related to
> disabling huge faults on files that should never have pre-content events
> enabled.
> 
> This happened because the default f_mode of allocated files (0) does
> not disable pre-content events.
> 
> Pre-content events are disabled in file_set_fsnotify_mode_by_watchers()
> but internal files may not get to call this helper.
> 
> Initialize f_mode to disable permission and pre-content events for all
> files and if needed they will be enabled for the callers of
> file_set_fsnotify_mode_by_watchers().
> 
> Fixes: 20bf82a898b6 ("mm: don't allow huge faults for files with pre content watches")
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Closes: https://lore.kernel.org/linux-fsdevel/20250131121703.1e4d00a7.alex.williamson@redhat.com/
> Tested-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

What makes me somewhat uneasy is that this relies on the fact that
file_set_fsnotify_mode_from_watchers() will override the
FMODE_NONOTIFY_PERM (but it does not override FMODE_NONOTIFY). This seems a
bit subtle and I was looking into if we could somehow simplify the fsnotify
fmode initialization. But I didn't find anything that would be really
simpler so let's keep what we have for now.

								Honza

> ---
>  fs/file_table.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/file_table.c b/fs/file_table.c
> index 35b93da6c5cb1..5c00dc38558da 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -194,6 +194,11 @@ static int init_file(struct file *f, int flags, const struct cred *cred)
>  	 * refcount bumps we should reinitialize the reused file first.
>  	 */
>  	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().
> +	 */
> +	file_set_fsnotify_mode(f, FMODE_NONOTIFY_PERM);
>  	return 0;
>  }
>  
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 3/3] fsnotify: disable pre-content and permission events by default

[Amir Goldstein]

On Wed, Feb 5, 2025 at 5:59 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 03-02-25 23:32:05, Amir Goldstein wrote:
> > After introducing pre-content events, we had a regression related to
> > disabling huge faults on files that should never have pre-content events
> > enabled.
> >
> > This happened because the default f_mode of allocated files (0) does
> > not disable pre-content events.
> >
> > Pre-content events are disabled in file_set_fsnotify_mode_by_watchers()
> > but internal files may not get to call this helper.
> >
> > Initialize f_mode to disable permission and pre-content events for all
> > files and if needed they will be enabled for the callers of
> > file_set_fsnotify_mode_by_watchers().
> >
> > Fixes: 20bf82a898b6 ("mm: don't allow huge faults for files with pre content watches")
> > Reported-by: Alex Williamson <alex.williamson@redhat.com>
> > Closes: https://lore.kernel.org/linux-fsdevel/20250131121703.1e4d00a7.alex.williamson@redhat.com/
> > Tested-by: Alex Williamson <alex.williamson@redhat.com>
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> Looks good. Feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> What makes me somewhat uneasy is that this relies on the fact that
> file_set_fsnotify_mode_from_watchers() will override the
> FMODE_NONOTIFY_PERM (but it does not override FMODE_NONOTIFY).
> This seems a bit subtle and I was looking into if we could somehow simplify the fsnotify
> fmode initialization. But I didn't find anything that would be really
> simpler so let's keep what we have for now.
>

Yeh, I noticed that too.
I did not have a better idea :/

Thanks,
Amir.