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 <[email protected]>
---
 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,
-- 
2.31.1



_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to