ext2-file-fs-deadlock-while-reading-corrupted-xattr-block

   1 ext2: fix filesystem deadlock while reading corrupted xattr block
   2
   3 From: Carlos Maiolino <cmaiolino@redhat.com>
   4
   5 This bug can be reproducible with fsfuzzer, although, I couldn't reproduce it
   6 100% of my tries, it is quite easily reproducible.
   7
   8 During the deletion of an inode, ext2_xattr_delete_inode() does not check if the
   9 block pointed by EXT2_I(inode)->i_file_acl is a valid data block, this might
  10 lead to a deadlock, when i_file_acl == 1, and the filesystem block size is 1024.
  11
  12 In that situation, ext2_xattr_delete_inode, will load the superblock's buffer
  13 head (instead of a valid i_file_acl block), and then lock that buffer head,
  14 which, ext2_sync_super will also try to lock, making the filesystem deadlock in
  15 the following stack trace:
  16
  17 root     17180  0.0  0.0 113660   660 pts/0    D+   07:08   0:00 rmdir
  18 /media/test/dir1
  19
  20 [<ffffffff8125da9f>] __sync_dirty_buffer+0xaf/0x100
  21 [<ffffffff8125db03>] sync_dirty_buffer+0x13/0x20
  22 [<ffffffffa03f0d57>] ext2_sync_super+0xb7/0xc0 [ext2]
  23 [<ffffffffa03f10b9>] ext2_error+0x119/0x130 [ext2]
  24 [<ffffffffa03e9d93>] ext2_free_blocks+0x83/0x350 [ext2]
  25 [<ffffffffa03f3d03>] ext2_xattr_delete_inode+0x173/0x190 [ext2]
  26 [<ffffffffa03ee9e9>] ext2_evict_inode+0xc9/0x130 [ext2]
  27 [<ffffffff8123fd23>] evict+0xb3/0x180
  28 [<ffffffff81240008>] iput+0x1b8/0x240
  29 [<ffffffff8123c4ac>] d_delete+0x11c/0x150
  30 [<ffffffff8122fa7e>] vfs_rmdir+0xfe/0x120
  31 [<ffffffff812340ee>] do_rmdir+0x17e/0x1f0
  32 [<ffffffff81234dd6>] SyS_rmdir+0x16/0x20
  33 [<ffffffff81838cf2>] entry_SYSCALL_64_fastpath+0x1a/0xa4
  34 [<ffffffffffffffff>] 0xffffffffffffffff
  35
  36 Fix this by using the same approach ext4 uses to test data blocks validity,
  37 implementing ext2_data_block_valid.
  38
  39 An another possibility when the superblock is very corrupted, is that i_file_acl
  40 is 1, block_count is 1 and first_data_block is 0. For such situations, we might
  41 have i_file_acl pointing to a 'valid' block, but still step over the superblock.
  42 The approach I used was to also test if the superblock is not in the range
  43 described by ext2_data_block_valid() arguments
  44
  45 Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
  46 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  47 ---
  48  fs/ext2/balloc.c | 21 +++++++++++++++++++++
  49  fs/ext2/ext2.h   |  3 +++
  50  fs/ext2/inode.c  | 10 ++++++++++
  51  fs/ext2/xattr.c  |  9 +++++++++
  52  4 files changed, 43 insertions(+)
  53
  54 diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c
  55 index 9f9992b..4c40c07 100644
  56 --- a/fs/ext2/balloc.c
  57 +++ b/fs/ext2/balloc.c
  58 @@ -1194,6 +1194,27 @@ static int ext2_has_free_blocks(struct ext2_sb_info *sbi)
  59  }
  60
  61  /*
  62 + * Returns 1 if the passed-in block region is valid; 0 if some part overlaps
  63 + * with filesystem metadata blocksi.
  64 + */
  65 +int ext2_data_block_valid(struct ext2_sb_info *sbi, ext2_fsblk_t start_blk,
  66 +                         unsigned int count)
  67 +{
  68 +       if ((start_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
  69 +           (start_blk + count < start_blk) ||
  70 +           (start_blk > le32_to_cpu(sbi->s_es->s_blocks_count)))
  71 +               return 0;
  72 +
  73 +       /* Ensure we do not step over superblock */
  74 +       if ((start_blk <= sbi->s_sb_block) &&
  75 +           (start_blk + count >= sbi->s_sb_block))
  76 +               return 0;
  77 +
  78 +
  79 +       return 1;
  80 +}
  81 +
  82 +/*
  83   * ext2_new_blocks() -- core block(s) allocation function
  84   * @inode:             file inode
  85   * @goal:              given target block(filesystem wide)
  86 diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
  87 index 170939f..3fb9368 100644
  88 --- a/fs/ext2/ext2.h
  89 +++ b/fs/ext2/ext2.h
  90 @@ -367,6 +367,7 @@ struct ext2_inode {
  91   */
  92  #define        EXT2_VALID_FS                   0x0001  /* Unmounted cleanly */
  93  #define        EXT2_ERROR_FS                   0x0002  /* Errors detected */
  94 +#define        EFSCORRUPTED                    EUCLEAN /* Filesystem is corrupted */
  95
  96  /*
  97   * Mount flags
  98 @@ -739,6 +740,8 @@ extern unsigned long ext2_bg_num_gdb(struct super_block *sb, int group);
  99  extern ext2_fsblk_t ext2_new_block(struct inode *, unsigned long, int *);
 100  extern ext2_fsblk_t ext2_new_blocks(struct inode *, unsigned long,
 101                                 unsigned long *, int *);
 102 +extern int ext2_data_block_valid(struct ext2_sb_info *sbi, ext2_fsblk_t start_blk,
 103 +                                unsigned int count);
 104  extern void ext2_free_blocks (struct inode *, unsigned long,
 105                               unsigned long);
 106  extern unsigned long ext2_count_free_blocks (struct super_block *);
 107 diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
 108 index fcbe586..d5c7d09 100644
 109 --- a/fs/ext2/inode.c
 110 +++ b/fs/ext2/inode.c
 111 @@ -1389,6 +1389,16 @@ struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
 112         ei->i_frag_size = raw_inode->i_fsize;
 113         ei->i_file_acl = le32_to_cpu(raw_inode->i_file_acl);
 114         ei->i_dir_acl = 0;
 115 +
 116 +       if (ei->i_file_acl &&
 117 +           !ext2_data_block_valid(EXT2_SB(sb), ei->i_file_acl, 1)) {
 118 +               ext2_error(sb, "ext2_iget", "bad extended attribute block %u",
 119 +                          ei->i_file_acl);
 120 +               brelse(bh);
 121 +               ret = -EFSCORRUPTED;
 122 +               goto bad_inode;
 123 +       }
 124 +
 125         if (S_ISREG(inode->i_mode))
 126                 inode->i_size |= ((__u64)le32_to_cpu(raw_inode->i_size_high)) << 32;
 127         else
 128 diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
 129 index 1a5e3bf..b7f896f 100644
 130 --- a/fs/ext2/xattr.c
 131 +++ b/fs/ext2/xattr.c
 132 @@ -759,10 +759,19 @@ void
 133  ext2_xattr_delete_inode(struct inode *inode)
 134  {
 135         struct buffer_head *bh = NULL;
 136 +       struct ext2_sb_info *sbi = EXT2_SB(inode->i_sb);
 137
 138         down_write(&EXT2_I(inode)->xattr_sem);
 139         if (!EXT2_I(inode)->i_file_acl)
 140                 goto cleanup;
 141 +
 142 +       if (!ext2_data_block_valid(sbi, EXT2_I(inode)->i_file_acl, 0)) {
 143 +               ext2_error(inode->i_sb, "ext2_xattr_delete_inode",
 144 +                       "inode %ld: xattr block %d is out of data blocks range",
 145 +                       inode->i_ino, EXT2_I(inode)->i_file_acl);
 146 +               goto cleanup;
 147 +       }
 148 +
 149         bh = sb_bread(inode->i_sb, EXT2_I(inode)->i_file_acl);
 150         if (!bh) {
 151                 ext2_error(inode->i_sb, "ext2_xattr_delete_inode",