Commit 94d7c16c authored by Akira Fujita's avatar Akira Fujita Committed by Theodore Ts'o

ext4: Fix double-free of blocks with EXT4_IOC_MOVE_EXT

At the beginning of ext4_move_extent(), we call
ext4_discard_preallocations() to discard inode PAs of orig and donor
inodes.  But in the following case, blocks can be double freed, so
move ext4_discard_preallocations() to the end of ext4_move_extents().

1. Discard inode PAs of orig and donor inodes with
   ext4_discard_preallocations() in ext4_move_extents().

   orig : [ DATA1 ]
   donor: [ DATA2 ]

2. While data blocks are exchanging between orig and donor inodes, new
   inode PAs is created to orig by other process's block allocation.
   (Since there are semaphore gaps in ext4_move_extents().)  And new
   inode PAs is used partially (2-1).

   2-1 Create new inode PAs to orig inode
   orig : [ DATA1 | used PA1 | free PA1 ]
   donor: [ DATA2 ]

3. Donor inode which has old orig inode's blocks is deleted after
   EXT4_IOC_MOVE_EXT finished (3-1, 3-2).  So the block bitmap
   corresponds to old orig inode's blocks are freed.

   3-1 After EXT4_IOC_MOVE_EXT finished
   orig : [ DATA2 |  free PA1 ]
   donor: [ DATA1 |  used PA1 ]

   3-2 Delete donor inode
   orig : [ DATA2 |  free PA1 ]
   donor: [ FREE SPACE(DATA1) | FREE SPACE(used PA1) ]

4. The double-free of blocks is occurred, when close() is called to
   orig inode.  Because ext4_discard_preallocations() for orig inode
   frees used PA1 and free PA1, though used PA1 is already freed in 3.

   4-1 Double-free of blocks is occurred
   orig : [ DATA2 |  FREE SPACE(free PA1) ]
   donor: [ FREE SPACE(DATA1) | DOUBLE FREE(used PA1) ]
Signed-off-by: default avatarAkira Fujita <a-fujita@rs.jp.nec.com>
Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
parent 9084d471
...@@ -1289,10 +1289,6 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ...@@ -1289,10 +1289,6 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
ext4_ext_get_actual_len(ext_cur), block_end + 1) - ext4_ext_get_actual_len(ext_cur), block_end + 1) -
max(le32_to_cpu(ext_cur->ee_block), block_start); max(le32_to_cpu(ext_cur->ee_block), block_start);
/* Discard preallocations of two inodes */
ext4_discard_preallocations(orig_inode);
ext4_discard_preallocations(donor_inode);
while (!last_extent && le32_to_cpu(ext_cur->ee_block) <= block_end) { while (!last_extent && le32_to_cpu(ext_cur->ee_block) <= block_end) {
seq_blocks += add_blocks; seq_blocks += add_blocks;
...@@ -1410,6 +1406,11 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ...@@ -1410,6 +1406,11 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
} }
out: out:
if (*moved_len) {
ext4_discard_preallocations(orig_inode);
ext4_discard_preallocations(donor_inode);
}
if (orig_path) { if (orig_path) {
ext4_ext_drop_refs(orig_path); ext4_ext_drop_refs(orig_path);
kfree(orig_path); kfree(orig_path);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment