fix-potential-use-after-free-after-remounting-with-noblock_validity

   1 ext4: fix potential use after free after remounting with noblock_validity
   2
   3 From: "zhangyi (F)" <yi.zhang@huawei.com>
   4
   5 Remount process will release system zone which was allocated before if
   6 "noblock_validity" is specified. If we mount an ext4 file system to two
   7 mountpoints with default mount options, and then remount one of them
   8 with "noblock_validity", it may trigger a use after free problem when
   9 someone accessing the other one.
  10
  11  # mount /dev/sda foo
  12  # mount /dev/sda bar
  13
  14 User access mountpoint "foo"   |   Remount mountpoint "bar"
  15                                |
  16 ext4_map_blocks()              |   ext4_remount()
  17 check_block_validity()         |   ext4_setup_system_zone()
  18 ext4_data_block_valid()        |   ext4_release_system_zone()
  19                                |   free system_blks rb nodes
  20 access system_blks rb nodes    |
  21 trigger use after free         |
  22
  23 This problem can also be reproduced by one mountpint, At the same time,
  24 add_system_zone() can get called during remount as well so there can be
  25 racing ext4_data_block_valid() reading the rbtree at the same time.
  26
  27 This patch add RCU to protect system zone from releasing or building
  28 when doing a remount which inverse current "noblock_validity" mount
  29 option. It assign the rbtree after the whole tree was complete and
  30 do actual freeing after rcu grace period, avoid any intermediate state.
  31
  32 Reported-by: syzbot+1e470567330b7ad711d5@syzkaller.appspotmail.com
  33 Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
  34 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  35 Reviewed-by: Jan Kara <jack@suse.cz>
  36 ---
  37 Changes since v4:
  38  - Update comments before ext4_[setup|release]_system_zone().
  39  - Add reviewed-by tag.
  40
  41  fs/ext4/block_validity.c | 188 ++++++++++++++++++++++++++++++++++-------------
  42  fs/ext4/ext4.h           |  10 ++-
  43  2 files changed, 146 insertions(+), 52 deletions(-)
  44
  45 diff --git a/fs/ext4/block_validity.c b/fs/ext4/block_validity.c
  46 index 8e83741..003dc1d 100644
  47 --- a/fs/ext4/block_validity.c
  48 +++ b/fs/ext4/block_validity.c
  49 @@ -38,6 +38,7 @@ int __init ext4_init_system_zone(void)
  50
  51  void ext4_exit_system_zone(void)
  52  {
  53 +       rcu_barrier();
  54         kmem_cache_destroy(ext4_system_zone_cachep);
  55  }
  56
  57 @@ -49,17 +50,26 @@ static inline int can_merge(struct ext4_system_zone *entry1,
  58         return 0;
  59  }
  60
  61 +static void release_system_zone(struct ext4_system_blocks *system_blks)
  62 +{
  63 +       struct ext4_system_zone *entry, *n;
  64 +
  65 +       rbtree_postorder_for_each_entry_safe(entry, n,
  66 +                               &system_blks->root, node)
  67 +               kmem_cache_free(ext4_system_zone_cachep, entry);
  68 +}
  69 +
  70  /*
  71   * Mark a range of blocks as belonging to the "system zone" --- that
  72   * is, filesystem metadata blocks which should never be used by
  73   * inodes.
  74   */
  75 -static int add_system_zone(struct ext4_sb_info *sbi,
  76 +static int add_system_zone(struct ext4_system_blocks *system_blks,
  77                            ext4_fsblk_t start_blk,
  78                            unsigned int count)
  79  {
  80         struct ext4_system_zone *new_entry = NULL, *entry;
  81 -       struct rb_node **n = &sbi->system_blks.rb_node, *node;
  82 +       struct rb_node **n = &system_blks->root.rb_node, *node;
  83         struct rb_node *parent = NULL, *new_node = NULL;
  84
  85         while (*n) {
  86 @@ -91,7 +101,7 @@ static int add_system_zone(struct ext4_sb_info *sbi,
  87                 new_node = &new_entry->node;
  88
  89                 rb_link_node(new_node, parent, n);
  90 -               rb_insert_color(new_node, &sbi->system_blks);
  91 +               rb_insert_color(new_node, &system_blks->root);
  92         }
  93
  94         /* Can we merge to the left? */
  95 @@ -101,7 +111,7 @@ static int add_system_zone(struct ext4_sb_info *sbi,
  96                 if (can_merge(entry, new_entry)) {
  97                         new_entry->start_blk = entry->start_blk;
  98                         new_entry->count += entry->count;
  99 -                       rb_erase(node, &sbi->system_blks);
 100 +                       rb_erase(node, &system_blks->root);
 101                         kmem_cache_free(ext4_system_zone_cachep, entry);
 102                 }
 103         }
 104 @@ -112,7 +122,7 @@ static int add_system_zone(struct ext4_sb_info *sbi,
 105                 entry = rb_entry(node, struct ext4_system_zone, node);
 106                 if (can_merge(new_entry, entry)) {
 107                         new_entry->count += entry->count;
 108 -                       rb_erase(node, &sbi->system_blks);
 109 +                       rb_erase(node, &system_blks->root);
 110                         kmem_cache_free(ext4_system_zone_cachep, entry);
 111                 }
 112         }
 113 @@ -126,7 +136,7 @@ static void debug_print_tree(struct ext4_sb_info *sbi)
 114         int first = 1;
 115
 116         printk(KERN_INFO "System zones: ");
 117 -       node = rb_first(&sbi->system_blks);
 118 +       node = rb_first(&sbi->system_blks->root);
 119         while (node) {
 120                 entry = rb_entry(node, struct ext4_system_zone, node);
 121                 printk(KERN_CONT "%s%llu-%llu", first ? "" : ", ",
 122 @@ -137,7 +147,47 @@ static void debug_print_tree(struct ext4_sb_info *sbi)
 123         printk(KERN_CONT "\n");
 124  }
 125
 126 -static int ext4_protect_reserved_inode(struct super_block *sb, u32 ino)
 127 +/*
 128 + * Returns 1 if the passed-in block region (start_blk,
 129 + * start_blk+count) is valid; 0 if some part of the block region
 130 + * overlaps with filesystem metadata blocks.
 131 + */
 132 +static int ext4_data_block_valid_rcu(struct ext4_sb_info *sbi,
 133 +                                    struct ext4_system_blocks *system_blks,
 134 +                                    ext4_fsblk_t start_blk,
 135 +                                    unsigned int count)
 136 +{
 137 +       struct ext4_system_zone *entry;
 138 +       struct rb_node *n;
 139 +
 140 +       if ((start_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
 141 +           (start_blk + count < start_blk) ||
 142 +           (start_blk + count > ext4_blocks_count(sbi->s_es))) {
 143 +               sbi->s_es->s_last_error_block = cpu_to_le64(start_blk);
 144 +               return 0;
 145 +       }
 146 +
 147 +       if (system_blks == NULL)
 148 +               return 1;
 149 +
 150 +       n = system_blks->root.rb_node;
 151 +       while (n) {
 152 +               entry = rb_entry(n, struct ext4_system_zone, node);
 153 +               if (start_blk + count - 1 < entry->start_blk)
 154 +                       n = n->rb_left;
 155 +               else if (start_blk >= (entry->start_blk + entry->count))
 156 +                       n = n->rb_right;
 157 +               else {
 158 +                       sbi->s_es->s_last_error_block = cpu_to_le64(start_blk);
 159 +                       return 0;
 160 +               }
 161 +       }
 162 +       return 1;
 163 +}
 164 +
 165 +static int ext4_protect_reserved_inode(struct super_block *sb,
 166 +                                      struct ext4_system_blocks *system_blks,
 167 +                                      u32 ino)
 168  {
 169         struct inode *inode;
 170         struct ext4_sb_info *sbi = EXT4_SB(sb);
 171 @@ -163,14 +213,15 @@ static int ext4_protect_reserved_inode(struct super_block *sb, u32 ino)
 172                 if (n == 0) {
 173                         i++;
 174                 } else {
 175 -                       if (!ext4_data_block_valid(sbi, map.m_pblk, n)) {
 176 +                       if (!ext4_data_block_valid_rcu(sbi, system_blks,
 177 +                                               map.m_pblk, n)) {
 178                                 ext4_error(sb, "blocks %llu-%llu from inode %u "
 179                                            "overlap system zone", map.m_pblk,
 180                                            map.m_pblk + map.m_len - 1, ino);
 181                                 err = -EFSCORRUPTED;
 182                                 break;
 183                         }
 184 -                       err = add_system_zone(sbi, map.m_pblk, n);
 185 +                       err = add_system_zone(system_blks, map.m_pblk, n);
 186                         if (err < 0)
 187                                 break;
 188                         i += n;
 189 @@ -180,94 +231,129 @@ static int ext4_protect_reserved_inode(struct super_block *sb, u32 ino)
 190         return err;
 191  }
 192
 193 +static void ext4_destroy_system_zone(struct rcu_head *rcu)
 194 +{
 195 +       struct ext4_system_blocks *system_blks;
 196 +
 197 +       system_blks = container_of(rcu, struct ext4_system_blocks, rcu);
 198 +       release_system_zone(system_blks);
 199 +       kfree(system_blks);
 200 +}
 201 +
 202 +/*
 203 + * Build system zone rbtree which is used for block validity checking.
 204 + *
 205 + * The update of system_blks pointer in this function is protected by
 206 + * sb->s_umount semaphore. However we have to be careful as we can be
 207 + * racing with ext4_data_block_valid() calls reading system_blks rbtree
 208 + * protected only by RCU. That's why we first build the rbtree and then
 209 + * swap it in place.
 210 + */
 211  int ext4_setup_system_zone(struct super_block *sb)
 212  {
 213         ext4_group_t ngroups = ext4_get_groups_count(sb);
 214         struct ext4_sb_info *sbi = EXT4_SB(sb);
 215 +       struct ext4_system_blocks *system_blks;
 216         struct ext4_group_desc *gdp;
 217         ext4_group_t i;
 218         int flex_size = ext4_flex_bg_size(sbi);
 219         int ret;
 220
 221         if (!test_opt(sb, BLOCK_VALIDITY)) {
 222 -               if (sbi->system_blks.rb_node)
 223 +               if (sbi->system_blks)
 224                         ext4_release_system_zone(sb);
 225                 return 0;
 226         }
 227 -       if (sbi->system_blks.rb_node)
 228 +       if (sbi->system_blks)
 229                 return 0;
 230
 231 +       system_blks = kzalloc(sizeof(*system_blks), GFP_KERNEL);
 232 +       if (!system_blks)
 233 +               return -ENOMEM;
 234 +
 235         for (i=0; i < ngroups; i++) {
 236                 cond_resched();
 237                 if (ext4_bg_has_super(sb, i) &&
 238                     ((i < 5) || ((i % flex_size) == 0)))
 239 -                       add_system_zone(sbi, ext4_group_first_block_no(sb, i),
 240 +                       add_system_zone(system_blks,
 241 +                                       ext4_group_first_block_no(sb, i),
 242                                         ext4_bg_num_gdb(sb, i) + 1);
 243                 gdp = ext4_get_group_desc(sb, i, NULL);
 244 -               ret = add_system_zone(sbi, ext4_block_bitmap(sb, gdp), 1);
 245 +               ret = add_system_zone(system_blks,
 246 +                               ext4_block_bitmap(sb, gdp), 1);
 247                 if (ret)
 248 -                       return ret;
 249 -               ret = add_system_zone(sbi, ext4_inode_bitmap(sb, gdp), 1);
 250 +                       goto err;
 251 +               ret = add_system_zone(system_blks,
 252 +                               ext4_inode_bitmap(sb, gdp), 1);
 253                 if (ret)
 254 -                       return ret;
 255 -               ret = add_system_zone(sbi, ext4_inode_table(sb, gdp),
 256 +                       goto err;
 257 +               ret = add_system_zone(system_blks,
 258 +                               ext4_inode_table(sb, gdp),
 259                                 sbi->s_itb_per_group);
 260                 if (ret)
 261 -                       return ret;
 262 +                       goto err;
 263         }
 264         if (ext4_has_feature_journal(sb) && sbi->s_es->s_journal_inum) {
 265 -               ret = ext4_protect_reserved_inode(sb,
 266 +               ret = ext4_protect_reserved_inode(sb, system_blks,
 267                                 le32_to_cpu(sbi->s_es->s_journal_inum));
 268                 if (ret)
 269 -                       return ret;
 270 +                       goto err;
 271         }
 272
 273 +       /*
 274 +        * System blks rbtree complete, announce it once to prevent racing
 275 +        * with ext4_data_block_valid() accessing the rbtree at the same
 276 +        * time.
 277 +        */
 278 +       rcu_assign_pointer(sbi->system_blks, system_blks);
 279 +
 280         if (test_opt(sb, DEBUG))
 281                 debug_print_tree(sbi);
 282         return 0;
 283 +err:
 284 +       release_system_zone(system_blks);
 285 +       kfree(system_blks);
 286 +       return ret;
 287  }
 288
 289 -/* Called when the filesystem is unmounted */
 290 +/*
 291 + * Called when the filesystem is unmounted or when remounting it with
 292 + * noblock_validity specified.
 293 + *
 294 + * The update of system_blks pointer in this function is protected by
 295 + * sb->s_umount semaphore. However we have to be careful as we can be
 296 + * racing with ext4_data_block_valid() calls reading system_blks rbtree
 297 + * protected only by RCU. So we first clear the system_blks pointer and
 298 + * then free the rbtree only after RCU grace period expires.
 299 + */
 300  void ext4_release_system_zone(struct super_block *sb)
 301  {
 302 -       struct ext4_system_zone *entry, *n;
 303 +       struct ext4_system_blocks *system_blks;
 304
 305 -       rbtree_postorder_for_each_entry_safe(entry, n,
 306 -                       &EXT4_SB(sb)->system_blks, node)
 307 -               kmem_cache_free(ext4_system_zone_cachep, entry);
 308 +       system_blks = rcu_dereference(EXT4_SB(sb)->system_blks);
 309 +       rcu_assign_pointer(EXT4_SB(sb)->system_blks, NULL);
 310
 311 -       EXT4_SB(sb)->system_blks = RB_ROOT;
 312 +       if (system_blks)
 313 +               call_rcu(&system_blks->rcu, ext4_destroy_system_zone);
 314  }
 315
 316 -/*
 317 - * Returns 1 if the passed-in block region (start_blk,
 318 - * start_blk+count) is valid; 0 if some part of the block region
 319 - * overlaps with filesystem metadata blocks.
 320 - */
 321  int ext4_data_block_valid(struct ext4_sb_info *sbi, ext4_fsblk_t start_blk,
 322                           unsigned int count)
 323  {
 324 -       struct ext4_system_zone *entry;
 325 -       struct rb_node *n = sbi->system_blks.rb_node;
 326 +       struct ext4_system_blocks *system_blks;
 327 +       int ret;
 328
 329 -       if ((start_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
 330 -           (start_blk + count < start_blk) ||
 331 -           (start_blk + count > ext4_blocks_count(sbi->s_es))) {
 332 -               sbi->s_es->s_last_error_block = cpu_to_le64(start_blk);
 333 -               return 0;
 334 -       }
 335 -       while (n) {
 336 -               entry = rb_entry(n, struct ext4_system_zone, node);
 337 -               if (start_blk + count - 1 < entry->start_blk)
 338 -                       n = n->rb_left;
 339 -               else if (start_blk >= (entry->start_blk + entry->count))
 340 -                       n = n->rb_right;
 341 -               else {
 342 -                       sbi->s_es->s_last_error_block = cpu_to_le64(start_blk);
 343 -                       return 0;
 344 -               }
 345 -       }
 346 -       return 1;
 347 +       /*
 348 +        * Lock the system zone to prevent it being released concurrently
 349 +        * when doing a remount which inverse current "[no]block_validity"
 350 +        * mount option.
 351 +        */
 352 +       rcu_read_lock();
 353 +       system_blks = rcu_dereference(sbi->system_blks);
 354 +       ret = ext4_data_block_valid_rcu(sbi, system_blks, start_blk,
 355 +                                       count);
 356 +       rcu_read_unlock();
 357 +       return ret;
 358  }
 359
 360  int ext4_check_blockref(const char *function, unsigned int line,
 361 diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
 362 index bf660aa..c025efc 100644
 363 --- a/fs/ext4/ext4.h
 364 +++ b/fs/ext4/ext4.h
 365 @@ -185,6 +185,14 @@ struct ext4_map_blocks {
 366  };
 367
 368  /*
 369 + * Block validity checking, system zone rbtree.
 370 + */
 371 +struct ext4_system_blocks {
 372 +       struct rb_root root;
 373 +       struct rcu_head rcu;
 374 +};
 375 +
 376 +/*
 377   * Flags for ext4_io_end->flags
 378   */
 379  #define        EXT4_IO_END_UNWRITTEN   0x0001
 380 @@ -1421,7 +1429,7 @@ struct ext4_sb_info {
 381         int s_jquota_fmt;                       /* Format of quota to use */
 382  #endif
 383         unsigned int s_want_extra_isize; /* New inodes should reserve # bytes */
 384 -       struct rb_root system_blks;
 385 +       struct ext4_system_blocks __rcu *system_blks;
 386
 387  #ifdef EXTENTS_STATS
 388         /* ext4 extents stats */
 389 --
 390 2.7.4
 391
 392