| Message ID | 20250309115207.908112-3-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix for potential deadlock in pre-content event | expand |
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 >
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.
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(); } /**