ext4: fix fs corruption when tring to remove a non-empty directory with IO error
authorYe Bin <yebin10@huawei.com>
Mon, 28 Feb 2022 02:48:15 +0000 (10:48 +0800)
committerTheodore Ts'o <tytso@mit.edu>
Thu, 3 Mar 2022 04:50:02 +0000 (23:50 -0500)
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
return false. To avoid making things worse when file system is already
corrupted, 'ext4_empty_dir' also return false.

Signed-off-by: Ye Bin <yebin10@huawei.com>
Cc: stable@kernel.org
Link: https://lore.kernel.org/r/20220228024815.3952506-1-yebin10@huawei.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
fs/ext4/inline.c
fs/ext4/namei.c

index e429418..9c07626 100644 (file)
@@ -1783,19 +1783,20 @@ bool empty_inline_dir(struct inode *dir, int *has_inline_data)
        void *inline_pos;
        unsigned int offset;
        struct ext4_dir_entry_2 *de;
-       bool ret = true;
+       bool ret = false;
 
        err = ext4_get_inode_loc(dir, &iloc);
        if (err) {
                EXT4_ERROR_INODE_ERR(dir, -err,
                                     "error %d getting inode %lu block",
                                     err, dir->i_ino);
-               return true;
+               return false;
        }
 
        down_read(&EXT4_I(dir)->xattr_sem);
        if (!ext4_has_inline_data(dir)) {
                *has_inline_data = 0;
+               ret = true;
                goto out;
        }
 
@@ -1804,7 +1805,6 @@ 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;
                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;
                        goto out;
                }
                if (le32_to_cpu(de->inode)) {
-                       ret = false;
                        goto out;
                }
                offset += ext4_rec_len_from_disk(de->rec_len, inline_size);
        }
 
+       ret = true;
 out:
        up_read(&EXT4_I(dir)->xattr_sem);
        brelse(iloc.bh);
index 8cf0a92..39e223f 100644 (file)
@@ -2997,14 +2997,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 false;
        }
        /* 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 false;
 
        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 +3012,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 false;
        }
        offset = ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize);
        de = ext4_next_entry(de, sb->s_blocksize);
@@ -3021,7 +3021,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 false;
        }
        offset += ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize);
        while (offset < inode->i_size) {
@@ -3035,7 +3035,7 @@ bool ext4_empty_dir(struct inode *inode)
                                continue;
                        }
                        if (IS_ERR(bh))
-                               return true;
+                               return false;
                }
                de = (struct ext4_dir_entry_2 *) (bh->b_data +
                                        (offset & (sb->s_blocksize - 1)));