| Message ID | 20220211093527.3335518-1-yebin10@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [-next,v2] ext4:fix file system corrupted when rmdir non empty directory with IO error | expand |
ping... On 2022/2/11 17:35, Ye Bin wrote: > We inject IO error when rmdir non empty direcory, then got issue as follows: > step1: mkfs.ext4 -F /dev/sda > step2: mount /dev/sda test > step3: cd test > step4: mkdir -p 1/2 > step5: rmdir 1 > [ 110.920551] ext4_empty_dir: inject fault > [ 110.921926] EXT4-fs warning (device sda): ext4_rmdir:3113: inode #12: > comm rmdir: empty directory '1' has too many links (3) > step6: cd .. > step7: umount test > step8: fsck.ext4 -f /dev/sda > e2fsck 1.42.9 (28-Dec-2013) > Pass 1: Checking inodes, blocks, and sizes > Pass 2: Checking directory structure > Entry '..' in .../??? (13) has deleted/unused inode 12. Clear<y>? yes > Pass 3: Checking directory connectivity > Unconnected directory inode 13 (...) > Connect to /lost+found<y>? yes > Pass 4: Checking reference counts > Inode 13 ref count is 3, should be 2. Fix<y>? yes > Pass 5: Checking group summary information > > /dev/sda: ***** FILE SYSTEM WAS MODIFIED ***** > /dev/sda: 12/131072 files (0.0% non-contiguous), 26157/524288 blocks > > ext4_rmdir > if (!ext4_empty_dir(inode)) > goto end_rmdir; > ext4_empty_dir > bh = ext4_read_dirblock(inode, 0, DIRENT_HTREE); > if (IS_ERR(bh)) > return true; > Now if read directory block failed, 'ext4_empty_dir' will return true, assume > directory is empty. Obviously, it will lead to above issue. > To solve this issue, if read directory block failed 'ext4_empty_dir' just assume > directory isn't empty. To avoid making things worse when file system is already > corrupted, 'ext4_empty_dir' also assume directory isn't empty. > To distinguish the error type, return the exact error code to the caller. > > Signed-off-by: Ye Bin <yebin10@huawei.com> > --- > fs/crypto/policy.c | 4 +--- > fs/ext4/ext4.h | 4 ++-- > fs/ext4/inline.c | 23 +++++++++++------------ > fs/ext4/ioctl.c | 5 ++--- > fs/ext4/namei.c | 27 +++++++++++++++------------ > fs/f2fs/dir.c | 8 ++++---- > fs/f2fs/f2fs.h | 2 +- > fs/f2fs/file.c | 7 +++++-- > fs/f2fs/namei.c | 10 ++++------ > fs/ubifs/crypto.c | 4 ++-- > include/linux/fscrypt.h | 2 +- > 11 files changed, 48 insertions(+), 48 deletions(-) > > diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c > index ed3d623724cd..373945022bb6 100644 > --- a/fs/crypto/policy.c > +++ b/fs/crypto/policy.c > @@ -480,9 +480,7 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg) > ret = -ENOTDIR; > else if (IS_DEADDIR(inode)) > ret = -ENOENT; > - else if (!inode->i_sb->s_cop->empty_dir(inode)) > - ret = -ENOTEMPTY; > - else > + else if (!(ret = inode->i_sb->s_cop->empty_dir(inode))) > ret = set_encryption_policy(inode, &policy); > } else if (ret == -EINVAL || > (ret == 0 && !fscrypt_policies_equal(&policy, > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index bcd3b9bf8069..e799cc269f7f 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -3087,7 +3087,7 @@ extern int ext4_generic_delete_entry(struct inode *dir, > void *entry_buf, > int buf_size, > int csum_size); > -extern bool ext4_empty_dir(struct inode *inode); > +extern int ext4_empty_dir(struct inode *inode); > > /* resize.c */ > extern void ext4_kvfree_array_rcu(void *to_free); > @@ -3623,7 +3623,7 @@ extern int ext4_delete_inline_entry(handle_t *handle, > struct ext4_dir_entry_2 *de_del, > struct buffer_head *bh, > int *has_inline_data); > -extern bool empty_inline_dir(struct inode *dir, int *has_inline_data); > +extern int empty_inline_dir(struct inode *dir, int *has_inline_data); > extern struct buffer_head *ext4_get_first_inline_block(struct inode *inode, > struct ext4_dir_entry_2 **parent_de, > int *retval); > diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c > index e42941803605..c9b02127ff95 100644 > --- a/fs/ext4/inline.c > +++ b/fs/ext4/inline.c > @@ -1775,22 +1775,22 @@ ext4_get_inline_entry(struct inode *inode, > return (struct ext4_dir_entry_2 *)(inline_pos + offset); > } > > -bool empty_inline_dir(struct inode *dir, int *has_inline_data) > +int empty_inline_dir(struct inode *dir, int *has_inline_data) > { > - int err, inline_size; > + int inline_size; > struct ext4_iloc iloc; > size_t inline_len; > void *inline_pos; > unsigned int offset; > struct ext4_dir_entry_2 *de; > - bool ret = true; > + int ret = 0; > > - err = ext4_get_inode_loc(dir, &iloc); > - if (err) { > - EXT4_ERROR_INODE_ERR(dir, -err, > + ret = ext4_get_inode_loc(dir, &iloc); > + if (ret) { > + EXT4_ERROR_INODE_ERR(dir, -ret, > "error %d getting inode %lu block", > - err, dir->i_ino); > - return true; > + ret, dir->i_ino); > + return ret; > } > > down_read(&EXT4_I(dir)->xattr_sem); > @@ -1804,7 +1804,7 @@ bool empty_inline_dir(struct inode *dir, int *has_inline_data) > ext4_warning(dir->i_sb, > "bad inline directory (dir #%lu) - no `..'", > dir->i_ino); > - ret = true; > + ret = -EFSCORRUPTED; > goto out; > } > > @@ -1823,16 +1823,15 @@ bool empty_inline_dir(struct inode *dir, int *has_inline_data) > dir->i_ino, le32_to_cpu(de->inode), > le16_to_cpu(de->rec_len), de->name_len, > inline_size); > - ret = true; > + ret = -EFSCORRUPTED; > goto out; > } > if (le32_to_cpu(de->inode)) { > - ret = false; > + ret = -ENOTEMPTY; > goto out; > } > offset += ext4_rec_len_from_disk(de->rec_len, inline_size); > } > - > out: > up_read(&EXT4_I(dir)->xattr_sem); > brelse(iloc.bh); > diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c > index a8022c2c6a58..3845fd554249 100644 > --- a/fs/ext4/ioctl.c > +++ b/fs/ext4/ioctl.c > @@ -620,10 +620,9 @@ static int ext4_ioctl_setflags(struct inode *inode, > goto flags_out; > } > > - if (!ext4_empty_dir(inode)) { > - err = -ENOTEMPTY; > + err = ext4_empty_dir(inode); > + if (err) > goto flags_out; > - } > } > > /* > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 8cf0a924a49b..2862deb374f7 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -2976,8 +2976,11 @@ static int ext4_mkdir(struct user_namespace *mnt_userns, struct inode *dir, > > /* > * routine to check that the specified directory is empty (for rmdir) > + * return value: > + * 0: directory is empty > + * <0: error code > */ > -bool ext4_empty_dir(struct inode *inode) > +int ext4_empty_dir(struct inode *inode) > { > unsigned int offset; > struct buffer_head *bh; > @@ -2997,14 +3000,14 @@ bool ext4_empty_dir(struct inode *inode) > if (inode->i_size < ext4_dir_rec_len(1, NULL) + > ext4_dir_rec_len(2, NULL)) { > EXT4_ERROR_INODE(inode, "invalid size"); > - return true; > + return -EFSCORRUPTED; > } > /* The first directory block must not be a hole, > * so treat it as DIRENT_HTREE > */ > bh = ext4_read_dirblock(inode, 0, DIRENT_HTREE); > if (IS_ERR(bh)) > - return true; > + return PTR_ERR(bh); > > de = (struct ext4_dir_entry_2 *) bh->b_data; > if (ext4_check_dir_entry(inode, NULL, de, bh, bh->b_data, bh->b_size, > @@ -3012,7 +3015,7 @@ bool ext4_empty_dir(struct inode *inode) > le32_to_cpu(de->inode) != inode->i_ino || strcmp(".", de->name)) { > ext4_warning_inode(inode, "directory missing '.'"); > brelse(bh); > - return true; > + return -EFSCORRUPTED; > } > offset = ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize); > de = ext4_next_entry(de, sb->s_blocksize); > @@ -3021,7 +3024,7 @@ bool ext4_empty_dir(struct inode *inode) > le32_to_cpu(de->inode) == 0 || strcmp("..", de->name)) { > ext4_warning_inode(inode, "directory missing '..'"); > brelse(bh); > - return true; > + return -EFSCORRUPTED; > } > offset += ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize); > while (offset < inode->i_size) { > @@ -3035,7 +3038,7 @@ bool ext4_empty_dir(struct inode *inode) > continue; > } > if (IS_ERR(bh)) > - return true; > + return PTR_ERR(bh); > } > de = (struct ext4_dir_entry_2 *) (bh->b_data + > (offset & (sb->s_blocksize - 1))); > @@ -3046,12 +3049,12 @@ bool ext4_empty_dir(struct inode *inode) > } > if (le32_to_cpu(de->inode)) { > brelse(bh); > - return false; > + return -ENOTEMPTY; > } > offset += ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize); > } > brelse(bh); > - return true; > + return 0; > } > > static int ext4_rmdir(struct inode *dir, struct dentry *dentry) > @@ -3087,8 +3090,8 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry) > if (le32_to_cpu(de->inode) != inode->i_ino) > goto end_rmdir; > > - retval = -ENOTEMPTY; > - if (!ext4_empty_dir(inode)) > + retval = ext4_empty_dir(inode); > + if (retval) > goto end_rmdir; > > handle = ext4_journal_start(dir, EXT4_HT_DIR, > @@ -3787,8 +3790,8 @@ static int ext4_rename(struct user_namespace *mnt_userns, struct inode *old_dir, > > if (S_ISDIR(old.inode->i_mode)) { > if (new.inode) { > - retval = -ENOTEMPTY; > - if (!ext4_empty_dir(new.inode)) > + retval = ext4_empty_dir(new.inode); > + if (retval) > goto end_rename; > } else { > retval = -EMLINK; > diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c > index a0e51937d92e..3de5a1343070 100644 > --- a/fs/f2fs/dir.c > +++ b/fs/f2fs/dir.c > @@ -953,7 +953,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page, > f2fs_drop_nlink(dir, inode); > } > > -bool f2fs_empty_dir(struct inode *dir) > +int f2fs_empty_dir(struct inode *dir) > { > unsigned long bidx; > struct page *dentry_page; > @@ -970,7 +970,7 @@ bool f2fs_empty_dir(struct inode *dir) > if (PTR_ERR(dentry_page) == -ENOENT) > continue; > else > - return false; > + return PTR_ERR(dentry_page); > } > > dentry_blk = page_address(dentry_page); > @@ -985,9 +985,9 @@ bool f2fs_empty_dir(struct inode *dir) > f2fs_put_page(dentry_page, 1); > > if (bit_pos < NR_DENTRY_IN_BLOCK) > - return false; > + return -ENOTEMPTY; > } > - return true; > + return 0; > } > > int f2fs_fill_dentries(struct dir_context *ctx, struct f2fs_dentry_ptr *d, > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 5c30a65467e2..09617d7b37fd 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -3465,7 +3465,7 @@ int f2fs_do_add_link(struct inode *dir, const struct qstr *name, > void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page, > struct inode *dir, struct inode *inode); > int f2fs_do_tmpfile(struct inode *inode, struct inode *dir); > -bool f2fs_empty_dir(struct inode *dir); > +int f2fs_empty_dir(struct inode *dir); > > static inline int f2fs_add_link(struct dentry *dentry, struct inode *inode) > { > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index cfdc41f87f5d..a3b60d6a58f7 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -1846,10 +1846,13 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask) > return -EPERM; > > if ((iflags ^ masked_flags) & F2FS_CASEFOLD_FL) { > + int ret; > + > if (!f2fs_sb_has_casefold(F2FS_I_SB(inode))) > return -EOPNOTSUPP; > - if (!f2fs_empty_dir(inode)) > - return -ENOTEMPTY; > + ret = f2fs_empty_dir(inode); > + if (ret) > + return ret; > } > > if (iflags & (F2FS_COMPR_FL | F2FS_NOCOMP_FL)) { > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c > index 13a0ffc39fa4..e4d1821b707b 100644 > --- a/fs/f2fs/namei.c > +++ b/fs/f2fs/namei.c > @@ -786,10 +786,10 @@ static int f2fs_mkdir(struct user_namespace *mnt_userns, struct inode *dir, > static int f2fs_rmdir(struct inode *dir, struct dentry *dentry) > { > struct inode *inode = d_inode(dentry); > + int ret; > > - if (f2fs_empty_dir(inode)) > - return f2fs_unlink(dir, dentry); > - return -ENOTEMPTY; > + ret = f2fs_empty_dir(inode); > + return ret ? : f2fs_unlink(dir, dentry); > } > > static int f2fs_mknod(struct user_namespace *mnt_userns, struct inode *dir, > @@ -1001,9 +1001,7 @@ static int f2fs_rename(struct user_namespace *mnt_userns, struct inode *old_dir, > } > > if (new_inode) { > - > - err = -ENOTEMPTY; > - if (old_dir_entry && !f2fs_empty_dir(new_inode)) > + if (old_dir_entry && (err = f2fs_empty_dir(new_inode))) > goto out_dir; > > err = -ENOENT; > diff --git a/fs/ubifs/crypto.c b/fs/ubifs/crypto.c > index c57b46a352d8..3ef2017c1444 100644 > --- a/fs/ubifs/crypto.c > +++ b/fs/ubifs/crypto.c > @@ -19,9 +19,9 @@ static int ubifs_crypt_set_context(struct inode *inode, const void *ctx, > ctx, len, 0, false); > } > > -static bool ubifs_crypt_empty_dir(struct inode *inode) > +static int ubifs_crypt_empty_dir(struct inode *inode) > { > - return ubifs_check_dir_empty(inode) == 0; > + return ubifs_check_dir_empty(inode); > } > > int ubifs_encrypt(const struct inode *inode, struct ubifs_data_node *dn, > diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h > index 91ea9477e9bd..9d3b8df3f5ea 100644 > --- a/include/linux/fscrypt.h > +++ b/include/linux/fscrypt.h > @@ -116,7 +116,7 @@ struct fscrypt_operations { > /* > * Check whether a directory is empty. i_rwsem will be held for write. > */ > - bool (*empty_dir)(struct inode *inode); > + int (*empty_dir)(struct inode *inode); > > /* > * Check whether the filesystem's inode numbers and UUID are stable,
On Fri, Feb 11, 2022 at 05:35:27PM +0800, Ye Bin wrote: > Now if read directory block failed, 'ext4_empty_dir' will return true, assume > directory is empty. Obviously, it will lead to above issue. > To solve this issue, if read directory block failed 'ext4_empty_dir' just assume > directory isn't empty. To avoid making things worse when file system is already > corrupted, 'ext4_empty_dir' also assume directory isn't empty. > To distinguish the error type, return the exact error code to the caller. > Does the same issue exist for f2fs and ubifs? We could solve the specific bug much more simply by having ext4_empty_dir() return FALSE if we aren't able to read the directory block. Yes, it means that we don't return as specific an error code in the case of an I/O error --- although I believe we do syslog a warning --- but it makes for a much simpler patch that doesn't requiring getting acked-by's from the fscrypt, f2fs and ubifs folks. - Ted
On 2022/2/26 10:23, Theodore Ts'o wrote: > On Fri, Feb 11, 2022 at 05:35:27PM +0800, Ye Bin wrote: >> Now if read directory block failed, 'ext4_empty_dir' will return true, assume >> directory is empty. Obviously, it will lead to above issue. >> To solve this issue, if read directory block failed 'ext4_empty_dir' just assume >> directory isn't empty. To avoid making things worse when file system is already >> corrupted, 'ext4_empty_dir' also assume directory isn't empty. >> To distinguish the error type, return the exact error code to the caller. >> > Does the same issue exist for f2fs and ubifs? We could solve the > specific bug much more simply by having ext4_empty_dir() return FALSE > if we aren't able to read the directory block. Yes, it means that we > don't return as specific an error code in the case of an I/O error --- > although I believe we do syslog a warning --- but it makes for a much > simpler patch that doesn't requiring getting acked-by's from the > fscrypt, f2fs and ubifs folks. > > - Ted > . In fact, I only modified ext4 as you suggested in my v1 patch. [-next] ext4:fix file system corrupted when rmdir non empty directory with IO error : https://patchwork.ozlabs.org/project/linux-ext4/patch/20220209112819.3072220-1-yebin10@huawei.com/ >
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c index ed3d623724cd..373945022bb6 100644 --- a/fs/crypto/policy.c +++ b/fs/crypto/policy.c @@ -480,9 +480,7 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg) ret = -ENOTDIR; else if (IS_DEADDIR(inode)) ret = -ENOENT; - else if (!inode->i_sb->s_cop->empty_dir(inode)) - ret = -ENOTEMPTY; - else + else if (!(ret = inode->i_sb->s_cop->empty_dir(inode))) ret = set_encryption_policy(inode, &policy); } else if (ret == -EINVAL || (ret == 0 && !fscrypt_policies_equal(&policy, diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index bcd3b9bf8069..e799cc269f7f 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -3087,7 +3087,7 @@ extern int ext4_generic_delete_entry(struct inode *dir, void *entry_buf, int buf_size, int csum_size); -extern bool ext4_empty_dir(struct inode *inode); +extern int ext4_empty_dir(struct inode *inode); /* resize.c */ extern void ext4_kvfree_array_rcu(void *to_free); @@ -3623,7 +3623,7 @@ extern int ext4_delete_inline_entry(handle_t *handle, struct ext4_dir_entry_2 *de_del, struct buffer_head *bh, int *has_inline_data); -extern bool empty_inline_dir(struct inode *dir, int *has_inline_data); +extern int empty_inline_dir(struct inode *dir, int *has_inline_data); extern struct buffer_head *ext4_get_first_inline_block(struct inode *inode, struct ext4_dir_entry_2 **parent_de, int *retval); diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c index e42941803605..c9b02127ff95 100644 --- a/fs/ext4/inline.c +++ b/fs/ext4/inline.c @@ -1775,22 +1775,22 @@ ext4_get_inline_entry(struct inode *inode, return (struct ext4_dir_entry_2 *)(inline_pos + offset); } -bool empty_inline_dir(struct inode *dir, int *has_inline_data) +int empty_inline_dir(struct inode *dir, int *has_inline_data) { - int err, inline_size; + int inline_size; struct ext4_iloc iloc; size_t inline_len; void *inline_pos; unsigned int offset; struct ext4_dir_entry_2 *de; - bool ret = true; + int ret = 0; - err = ext4_get_inode_loc(dir, &iloc); - if (err) { - EXT4_ERROR_INODE_ERR(dir, -err, + ret = ext4_get_inode_loc(dir, &iloc); + if (ret) { + EXT4_ERROR_INODE_ERR(dir, -ret, "error %d getting inode %lu block", - err, dir->i_ino); - return true; + ret, dir->i_ino); + return ret; } down_read(&EXT4_I(dir)->xattr_sem); @@ -1804,7 +1804,7 @@ bool empty_inline_dir(struct inode *dir, int *has_inline_data) ext4_warning(dir->i_sb, "bad inline directory (dir #%lu) - no `..'", dir->i_ino); - ret = true; + ret = -EFSCORRUPTED; goto out; } @@ -1823,16 +1823,15 @@ bool empty_inline_dir(struct inode *dir, int *has_inline_data) dir->i_ino, le32_to_cpu(de->inode), le16_to_cpu(de->rec_len), de->name_len, inline_size); - ret = true; + ret = -EFSCORRUPTED; goto out; } if (le32_to_cpu(de->inode)) { - ret = false; + ret = -ENOTEMPTY; goto out; } offset += ext4_rec_len_from_disk(de->rec_len, inline_size); } - out: up_read(&EXT4_I(dir)->xattr_sem); brelse(iloc.bh); diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c index a8022c2c6a58..3845fd554249 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -620,10 +620,9 @@ static int ext4_ioctl_setflags(struct inode *inode, goto flags_out; } - if (!ext4_empty_dir(inode)) { - err = -ENOTEMPTY; + err = ext4_empty_dir(inode); + if (err) goto flags_out; - } } /* diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 8cf0a924a49b..2862deb374f7 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -2976,8 +2976,11 @@ static int ext4_mkdir(struct user_namespace *mnt_userns, struct inode *dir, /* * routine to check that the specified directory is empty (for rmdir) + * return value: + * 0: directory is empty + * <0: error code */ -bool ext4_empty_dir(struct inode *inode) +int ext4_empty_dir(struct inode *inode) { unsigned int offset; struct buffer_head *bh; @@ -2997,14 +3000,14 @@ bool ext4_empty_dir(struct inode *inode) if (inode->i_size < ext4_dir_rec_len(1, NULL) + ext4_dir_rec_len(2, NULL)) { EXT4_ERROR_INODE(inode, "invalid size"); - return true; + return -EFSCORRUPTED; } /* The first directory block must not be a hole, * so treat it as DIRENT_HTREE */ bh = ext4_read_dirblock(inode, 0, DIRENT_HTREE); if (IS_ERR(bh)) - return true; + return PTR_ERR(bh); de = (struct ext4_dir_entry_2 *) bh->b_data; if (ext4_check_dir_entry(inode, NULL, de, bh, bh->b_data, bh->b_size, @@ -3012,7 +3015,7 @@ bool ext4_empty_dir(struct inode *inode) le32_to_cpu(de->inode) != inode->i_ino || strcmp(".", de->name)) { ext4_warning_inode(inode, "directory missing '.'"); brelse(bh); - return true; + return -EFSCORRUPTED; } offset = ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize); de = ext4_next_entry(de, sb->s_blocksize); @@ -3021,7 +3024,7 @@ bool ext4_empty_dir(struct inode *inode) le32_to_cpu(de->inode) == 0 || strcmp("..", de->name)) { ext4_warning_inode(inode, "directory missing '..'"); brelse(bh); - return true; + return -EFSCORRUPTED; } offset += ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize); while (offset < inode->i_size) { @@ -3035,7 +3038,7 @@ bool ext4_empty_dir(struct inode *inode) continue; } if (IS_ERR(bh)) - return true; + return PTR_ERR(bh); } de = (struct ext4_dir_entry_2 *) (bh->b_data + (offset & (sb->s_blocksize - 1))); @@ -3046,12 +3049,12 @@ bool ext4_empty_dir(struct inode *inode) } if (le32_to_cpu(de->inode)) { brelse(bh); - return false; + return -ENOTEMPTY; } offset += ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize); } brelse(bh); - return true; + return 0; } static int ext4_rmdir(struct inode *dir, struct dentry *dentry) @@ -3087,8 +3090,8 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry) if (le32_to_cpu(de->inode) != inode->i_ino) goto end_rmdir; - retval = -ENOTEMPTY; - if (!ext4_empty_dir(inode)) + retval = ext4_empty_dir(inode); + if (retval) goto end_rmdir; handle = ext4_journal_start(dir, EXT4_HT_DIR, @@ -3787,8 +3790,8 @@ static int ext4_rename(struct user_namespace *mnt_userns, struct inode *old_dir, if (S_ISDIR(old.inode->i_mode)) { if (new.inode) { - retval = -ENOTEMPTY; - if (!ext4_empty_dir(new.inode)) + retval = ext4_empty_dir(new.inode); + if (retval) goto end_rename; } else { retval = -EMLINK; diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c index a0e51937d92e..3de5a1343070 100644 --- a/fs/f2fs/dir.c +++ b/fs/f2fs/dir.c @@ -953,7 +953,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page, f2fs_drop_nlink(dir, inode); } -bool f2fs_empty_dir(struct inode *dir) +int f2fs_empty_dir(struct inode *dir) { unsigned long bidx; struct page *dentry_page; @@ -970,7 +970,7 @@ bool f2fs_empty_dir(struct inode *dir) if (PTR_ERR(dentry_page) == -ENOENT) continue; else - return false; + return PTR_ERR(dentry_page); } dentry_blk = page_address(dentry_page); @@ -985,9 +985,9 @@ bool f2fs_empty_dir(struct inode *dir) f2fs_put_page(dentry_page, 1); if (bit_pos < NR_DENTRY_IN_BLOCK) - return false; + return -ENOTEMPTY; } - return true; + return 0; } int f2fs_fill_dentries(struct dir_context *ctx, struct f2fs_dentry_ptr *d, diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 5c30a65467e2..09617d7b37fd 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -3465,7 +3465,7 @@ int f2fs_do_add_link(struct inode *dir, const struct qstr *name, void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page, struct inode *dir, struct inode *inode); int f2fs_do_tmpfile(struct inode *inode, struct inode *dir); -bool f2fs_empty_dir(struct inode *dir); +int f2fs_empty_dir(struct inode *dir); static inline int f2fs_add_link(struct dentry *dentry, struct inode *inode) { diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index cfdc41f87f5d..a3b60d6a58f7 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -1846,10 +1846,13 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask) return -EPERM; if ((iflags ^ masked_flags) & F2FS_CASEFOLD_FL) { + int ret; + if (!f2fs_sb_has_casefold(F2FS_I_SB(inode))) return -EOPNOTSUPP; - if (!f2fs_empty_dir(inode)) - return -ENOTEMPTY; + ret = f2fs_empty_dir(inode); + if (ret) + return ret; } if (iflags & (F2FS_COMPR_FL | F2FS_NOCOMP_FL)) { diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index 13a0ffc39fa4..e4d1821b707b 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -786,10 +786,10 @@ static int f2fs_mkdir(struct user_namespace *mnt_userns, struct inode *dir, static int f2fs_rmdir(struct inode *dir, struct dentry *dentry) { struct inode *inode = d_inode(dentry); + int ret; - if (f2fs_empty_dir(inode)) - return f2fs_unlink(dir, dentry); - return -ENOTEMPTY; + ret = f2fs_empty_dir(inode); + return ret ? : f2fs_unlink(dir, dentry); } static int f2fs_mknod(struct user_namespace *mnt_userns, struct inode *dir, @@ -1001,9 +1001,7 @@ static int f2fs_rename(struct user_namespace *mnt_userns, struct inode *old_dir, } if (new_inode) { - - err = -ENOTEMPTY; - if (old_dir_entry && !f2fs_empty_dir(new_inode)) + if (old_dir_entry && (err = f2fs_empty_dir(new_inode))) goto out_dir; err = -ENOENT; diff --git a/fs/ubifs/crypto.c b/fs/ubifs/crypto.c index c57b46a352d8..3ef2017c1444 100644 --- a/fs/ubifs/crypto.c +++ b/fs/ubifs/crypto.c @@ -19,9 +19,9 @@ static int ubifs_crypt_set_context(struct inode *inode, const void *ctx, ctx, len, 0, false); } -static bool ubifs_crypt_empty_dir(struct inode *inode) +static int ubifs_crypt_empty_dir(struct inode *inode) { - return ubifs_check_dir_empty(inode) == 0; + return ubifs_check_dir_empty(inode); } int ubifs_encrypt(const struct inode *inode, struct ubifs_data_node *dn, diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index 91ea9477e9bd..9d3b8df3f5ea 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -116,7 +116,7 @@ struct fscrypt_operations { /* * Check whether a directory is empty. i_rwsem will be held for write. */ - bool (*empty_dir)(struct inode *inode); + int (*empty_dir)(struct inode *inode); /* * Check whether the filesystem's inode numbers and UUID are stable,
We inject IO error when rmdir non empty direcory, then got issue as follows: step1: mkfs.ext4 -F /dev/sda step2: mount /dev/sda test step3: cd test step4: mkdir -p 1/2 step5: rmdir 1 [ 110.920551] ext4_empty_dir: inject fault [ 110.921926] EXT4-fs warning (device sda): ext4_rmdir:3113: inode #12: comm rmdir: empty directory '1' has too many links (3) step6: cd .. step7: umount test step8: fsck.ext4 -f /dev/sda e2fsck 1.42.9 (28-Dec-2013) Pass 1: Checking inodes, blocks, and sizes Pass 2: Checking directory structure Entry '..' in .../??? (13) has deleted/unused inode 12. Clear<y>? yes Pass 3: Checking directory connectivity Unconnected directory inode 13 (...) Connect to /lost+found<y>? yes Pass 4: Checking reference counts Inode 13 ref count is 3, should be 2. Fix<y>? yes Pass 5: Checking group summary information /dev/sda: ***** FILE SYSTEM WAS MODIFIED ***** /dev/sda: 12/131072 files (0.0% non-contiguous), 26157/524288 blocks ext4_rmdir if (!ext4_empty_dir(inode)) goto end_rmdir; ext4_empty_dir bh = ext4_read_dirblock(inode, 0, DIRENT_HTREE); if (IS_ERR(bh)) return true; Now if read directory block failed, 'ext4_empty_dir' will return true, assume directory is empty. Obviously, it will lead to above issue. To solve this issue, if read directory block failed 'ext4_empty_dir' just assume directory isn't empty. To avoid making things worse when file system is already corrupted, 'ext4_empty_dir' also assume directory isn't empty. To distinguish the error type, return the exact error code to the caller. Signed-off-by: Ye Bin <yebin10@huawei.com> --- fs/crypto/policy.c | 4 +--- fs/ext4/ext4.h | 4 ++-- fs/ext4/inline.c | 23 +++++++++++------------ fs/ext4/ioctl.c | 5 ++--- fs/ext4/namei.c | 27 +++++++++++++++------------ fs/f2fs/dir.c | 8 ++++---- fs/f2fs/f2fs.h | 2 +- fs/f2fs/file.c | 7 +++++-- fs/f2fs/namei.c | 10 ++++------ fs/ubifs/crypto.c | 4 ++-- include/linux/fscrypt.h | 2 +- 11 files changed, 48 insertions(+), 48 deletions(-)