[PATCH 0/2] Fix for potential deadlock in pre-content event

[PATCH 0/2] Fix for potential deadlock in pre-content event

[Amir Goldstein]

Jan,

Please consider this fix for the lockdep issue reported by syzbot.

Thanks,
Amir.

Amir Goldstein (2):
  fsnotify: remove check if file is actually being watched for
    pre-content events on open
  fsnotify: avoid pre-content events when faulting in user pages

 fs/notify/fsnotify.c | 29 ++++-------------------------
 include/linux/fs.h   | 18 +++++++++++++++++-
 2 files changed, 21 insertions(+), 26 deletions(-)

-- 
2.34.1

[PATCH 1/2] fsnotify: remove check if file is actually being watched for pre-content events on open

[Amir Goldstein]

Commit 318652e07fa5b ("fsnotify: check if file is actually being watched
for pre-content events on open") added an optimization that may be
premature.

Patially revert this change, leaving only the file type check, so that
we can use the FMODE_FSNOTIFY_HSM() flag to check if there are any
pre-content watches on the filesystem, which is needed in some cases.

If we find that we need the extra optimization we can reconsider adding
it later.

Fixes: 318652e07fa5b ("fsnotify: check if file is actually being watched for pre-content events on open")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fsnotify.c | 29 ++++-------------------------
 1 file changed, 4 insertions(+), 25 deletions(-)

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index fae1b6d397ea0..dafcaa6f8075f 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -650,9 +650,8 @@ EXPORT_SYMBOL_GPL(fsnotify);
  */
 void file_set_fsnotify_mode_from_watchers(struct file *file)
 {
-	struct dentry *dentry = file->f_path.dentry, *parent;
+	struct dentry *dentry = file->f_path.dentry;
 	struct super_block *sb = dentry->d_sb;
-	__u32 mnt_mask, p_mask;
 
 	/* Is it a file opened by fanotify? */
 	if (FMODE_FSNOTIFY_NONE(file->f_mode))
@@ -681,30 +680,10 @@ void file_set_fsnotify_mode_from_watchers(struct file *file)
 	}
 
 	/*
-	 * OK, there are some pre-content watchers. Check if anybody is
-	 * watching for pre-content events on *this* file.
+	 * OK, there are some pre-content watchers on this fs, so
+	 * Enable pre-content events.
 	 */
-	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 */
-		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) {
-			/* Enable pre-content events */
-			file_set_fsnotify_mode(file, 0);
-			return;
-		}
-	}
-	/* Nobody watching for pre-content events from this file */
-	file_set_fsnotify_mode(file, FMODE_NONOTIFY | FMODE_NONOTIFY_PERM);
+	file_set_fsnotify_mode(file, 0);
 }
 #endif
 
-- 
2.34.1

[PATCH 2/2] fsnotify: avoid pre-content events when faulting in user pages

[Amir Goldstein]

In the use case of buffered write whose input buffer is mmapped file on a
filesystem with a pre-content mark, the prefaulting of the buffer can
happen under the filesystem freeze protection (obtained in vfs_write())
which breaks assumptions of pre-content hook and introduces potential
deadlock of HSM handler in userspace with filesystem freezing.

Disable pagefaults in the context of filesystem freeze protection
if the filesystem has any pre-content marks to avert this potential
deadlock.

Reported-by: syzbot+7229071b47908b19d5b7@syzkaller.appspotmail.com
Tested-by: syzbot+7229071b47908b19d5b7@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/linux-fsdevel/7ehxrhbvehlrjwvrduoxsao5k3x4aw275patsb3krkwuq573yv@o2hskrfawbnc/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 include/linux/fs.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2788df98080f8..a8822b44d4967 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3033,13 +3033,27 @@ static inline void file_start_write(struct file *file)
 	if (!S_ISREG(file_inode(file)->i_mode))
 		return;
 	sb_start_write(file_inode(file)->i_sb);
+	/*
+	 * Prevent fault-in pages from user that may call HSM hooks with
+	 * sb_writers held.
+	 */
+	if (unlikely(FMODE_FSNOTIFY_HSM(file->f_mode)))
+		pagefault_disable();
 }
 
 static inline bool file_start_write_trylock(struct file *file)
 {
 	if (!S_ISREG(file_inode(file)->i_mode))
 		return true;
-	return sb_start_write_trylock(file_inode(file)->i_sb);
+	if (!sb_start_write_trylock(file_inode(file)->i_sb))
+		return false;
+	/*
+	 * Prevent fault-in pages from user that may call HSM hooks with
+	 * sb_writers held.
+	 */
+	if (unlikely(FMODE_FSNOTIFY_HSM(file->f_mode)))
+		pagefault_disable();
+	return true;
 }
 
 /**
@@ -3053,6 +3067,8 @@ static inline void file_end_write(struct file *file)
 	if (!S_ISREG(file_inode(file)->i_mode))
 		return;
 	sb_end_write(file_inode(file)->i_sb);
+	if (unlikely(FMODE_FSNOTIFY_HSM(file->f_mode)))
+		pagefault_enable();
 }
 
 /**
-- 
2.34.1

Re: [PATCH 2/2] fsnotify: avoid pre-content events when faulting in user pages

[Christian Brauner]

On Sun, Mar 09, 2025 at 12:52:07PM +0100, Amir Goldstein wrote:
> In the use case of buffered write whose input buffer is mmapped file on a
> filesystem with a pre-content mark, the prefaulting of the buffer can
> happen under the filesystem freeze protection (obtained in vfs_write())
> which breaks assumptions of pre-content hook and introduces potential
> deadlock of HSM handler in userspace with filesystem freezing.
> 
> Disable pagefaults in the context of filesystem freeze protection
> if the filesystem has any pre-content marks to avert this potential
> deadlock.
> 
> Reported-by: syzbot+7229071b47908b19d5b7@syzkaller.appspotmail.com
> Tested-by: syzbot+7229071b47908b19d5b7@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/linux-fsdevel/7ehxrhbvehlrjwvrduoxsao5k3x4aw275patsb3krkwuq573yv@o2hskrfawbnc/
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  include/linux/fs.h | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 2788df98080f8..a8822b44d4967 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3033,13 +3033,27 @@ static inline void file_start_write(struct file *file)
>  	if (!S_ISREG(file_inode(file)->i_mode))
>  		return;
>  	sb_start_write(file_inode(file)->i_sb);
> +	/*
> +	 * Prevent fault-in pages from user that may call HSM hooks with
> +	 * sb_writers held.
> +	 */
> +	if (unlikely(FMODE_FSNOTIFY_HSM(file->f_mode)))
> +		pagefault_disable();
>  }
>  
>  static inline bool file_start_write_trylock(struct file *file)
>  {
>  	if (!S_ISREG(file_inode(file)->i_mode))
>  		return true;
> -	return sb_start_write_trylock(file_inode(file)->i_sb);
> +	if (!sb_start_write_trylock(file_inode(file)->i_sb))
> +		return false;
> +	/*
> +	 * Prevent fault-in pages from user that may call HSM hooks with
> +	 * sb_writers held.
> +	 */
> +	if (unlikely(FMODE_FSNOTIFY_HSM(file->f_mode)))
> +		pagefault_disable();

That looks very iffy tbh.

> +	return true;
>  }
>  
>  /**
> @@ -3053,6 +3067,8 @@ static inline void file_end_write(struct file *file)
>  	if (!S_ISREG(file_inode(file)->i_mode))
>  		return;
>  	sb_end_write(file_inode(file)->i_sb);
> +	if (unlikely(FMODE_FSNOTIFY_HSM(file->f_mode)))
> +		pagefault_enable();
>  }
>  
>  /**
> -- 
> 2.34.1
> 

Re: [PATCH 2/2] fsnotify: avoid pre-content events when faulting in user pages

[Amir Goldstein]

On Mon, Mar 10, 2025 at 9:09 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Sun, Mar 09, 2025 at 12:52:07PM +0100, Amir Goldstein wrote:
> > In the use case of buffered write whose input buffer is mmapped file on a
> > filesystem with a pre-content mark, the prefaulting of the buffer can
> > happen under the filesystem freeze protection (obtained in vfs_write())
> > which breaks assumptions of pre-content hook and introduces potential
> > deadlock of HSM handler in userspace with filesystem freezing.
> >
> > Disable pagefaults in the context of filesystem freeze protection
> > if the filesystem has any pre-content marks to avert this potential
> > deadlock.
> >
> > Reported-by: syzbot+7229071b47908b19d5b7@syzkaller.appspotmail.com
> > Tested-by: syzbot+7229071b47908b19d5b7@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/linux-fsdevel/7ehxrhbvehlrjwvrduoxsao5k3x4aw275patsb3krkwuq573yv@o2hskrfawbnc/
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  include/linux/fs.h | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 2788df98080f8..a8822b44d4967 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -3033,13 +3033,27 @@ static inline void file_start_write(struct file *file)
> >       if (!S_ISREG(file_inode(file)->i_mode))
> >               return;
> >       sb_start_write(file_inode(file)->i_sb);
> > +     /*
> > +      * Prevent fault-in pages from user that may call HSM hooks with
> > +      * sb_writers held.
> > +      */
> > +     if (unlikely(FMODE_FSNOTIFY_HSM(file->f_mode)))
> > +             pagefault_disable();
> >  }
> >
> >  static inline bool file_start_write_trylock(struct file *file)
> >  {
> >       if (!S_ISREG(file_inode(file)->i_mode))
> >               return true;
> > -     return sb_start_write_trylock(file_inode(file)->i_sb);
> > +     if (!sb_start_write_trylock(file_inode(file)->i_sb))
> > +             return false;
> > +     /*
> > +      * Prevent fault-in pages from user that may call HSM hooks with
> > +      * sb_writers held.
> > +      */
> > +     if (unlikely(FMODE_FSNOTIFY_HSM(file->f_mode)))
> > +             pagefault_disable();
>
> That looks very iffy tbh.
>

Yes. not pretty.
I am testing the alternative approach suggested by Josef.
Will post the patch as soon as I am done testing.

Thanks,
Amir.