Messages in this thread | | | Date | Tue, 6 Jun 2023 14:21:32 +0800 | Subject | Re: [PATCH v2] f2fs: fix to avoid mmap vs set_compress_option case | From | Chao Yu <> |
| |
Jaegeuk,
Any comments on this patch?
On 2023/5/29 18:47, Chao Yu wrote: > Compression option in inode should not be changed after they have > been used, however, it may happen in below race case: > > Thread A Thread B > - f2fs_ioc_set_compress_option > - check f2fs_is_mmap_file() > - check get_dirty_pages() > - check F2FS_HAS_BLOCKS() > - f2fs_file_mmap > - set_inode_flag(FI_MMAP_FILE) > - fault > - do_page_mkwrite > - f2fs_vm_page_mkwrite > - f2fs_get_block_locked > - fault_dirty_shared_page > - set_page_dirty > - update i_compress_algorithm > - update i_log_cluster_size > - update i_cluster_size > > Avoid such race condition by covering f2fs_file_mmap() w/ inode lock, > meanwhile add mmap file check condition in f2fs_may_compress() as well. > > Fixes: e1e8debec656 ("f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl") > Signed-off-by: Chao Yu <chao@kernel.org> > --- > v2: > - add mmap file check condition in f2fs_may_compress() > fs/f2fs/f2fs.h | 3 ++- > fs/f2fs/file.c | 14 +++++++++++--- > 2 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 9bd83fb28439..0db8b37c7a4d 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -4487,7 +4487,8 @@ static inline bool f2fs_low_mem_mode(struct f2fs_sb_info *sbi) > static inline bool f2fs_may_compress(struct inode *inode) > { > if (IS_SWAPFILE(inode) || f2fs_is_pinned_file(inode) || > - f2fs_is_atomic_file(inode) || f2fs_has_inline_data(inode)) > + f2fs_is_atomic_file(inode) || f2fs_has_inline_data(inode) || > + f2fs_is_mmap_file(inode)) > return false; > return S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode); > } > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index 7b097ab2f5e4..685ded62fc28 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -519,17 +519,25 @@ static loff_t f2fs_llseek(struct file *file, loff_t offset, int whence) > static int f2fs_file_mmap(struct file *file, struct vm_area_struct *vma) > { > struct inode *inode = file_inode(file); > + int ret = 0; > > if (unlikely(f2fs_cp_error(F2FS_I_SB(inode)))) > return -EIO; > > - if (!f2fs_is_compress_backend_ready(inode)) > - return -EOPNOTSUPP; > + inode_lock(inode); > + > + if (!f2fs_is_compress_backend_ready(inode)) { > + ret = -EOPNOTSUPP; > + goto out_unlock; > + } > > file_accessed(file); > vma->vm_ops = &f2fs_file_vm_ops; > set_inode_flag(inode, FI_MMAP_FILE); > - return 0; > + > +out_unlock: > + inode_unlock(inode); > + return ret; > } > > static int f2fs_file_open(struct inode *inode, struct file *filp)
| |