Skip to content

Commit acf795d

Browse files
zhangyi089tytso
authored andcommitted
ext4: convert to exclusive lock while inserting delalloc extents
ext4_da_map_blocks() only hold i_data_sem in shared mode and i_rwsem when inserting delalloc extents, it could be raced by another querying path of ext4_map_blocks() without i_rwsem, .e.g buffered read path. Suppose we buffered read a file containing just a hole, and without any cached extents tree, then it is raced by another delayed buffered write to the same area or the near area belongs to the same hole, and the new delalloc extent could be overwritten to a hole extent. pread() pwrite() filemap_read_folio() ext4_mpage_readpages() ext4_map_blocks() down_read(i_data_sem) ext4_ext_determine_hole() //find hole ext4_ext_put_gap_in_cache() ext4_es_find_extent_range() //no delalloc extent ext4_da_map_blocks() down_read(i_data_sem) ext4_insert_delayed_block() //insert delalloc extent ext4_es_insert_extent() //overwrite delalloc extent to hole This race could lead to inconsistent delalloc extents tree and incorrect reserved space counter. Fix this by converting to hold i_data_sem in exclusive mode when adding a new delalloc extent in ext4_da_map_blocks(). Cc: stable@vger.kernel.org Signed-off-by: Zhang Yi <yi.zhang@huawei.com> Suggested-by: Jan Kara <jack@suse.cz> Reviewed-by: Jan Kara <jack@suse.cz> Link: https://lore.kernel.org/r/20240127015825.1608160-3-yi.zhang@huaweicloud.com Signed-off-by: Theodore Ts'o <tytso@mit.edu>
1 parent 3fcc2b8 commit acf795d

File tree

1 file changed

+11
-14
lines changed

1 file changed

+11
-14
lines changed

fs/ext4/inode.c

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1703,10 +1703,8 @@ static int ext4_da_map_blocks(struct inode *inode, sector_t iblock,
17031703

17041704
/* Lookup extent status tree firstly */
17051705
if (ext4_es_lookup_extent(inode, iblock, NULL, &es)) {
1706-
if (ext4_es_is_hole(&es)) {
1707-
down_read(&EXT4_I(inode)->i_data_sem);
1706+
if (ext4_es_is_hole(&es))
17081707
goto add_delayed;
1709-
}
17101708

17111709
/*
17121710
* Delayed extent could be allocated by fallocate.
@@ -1748,8 +1746,10 @@ static int ext4_da_map_blocks(struct inode *inode, sector_t iblock,
17481746
retval = ext4_ext_map_blocks(NULL, inode, map, 0);
17491747
else
17501748
retval = ext4_ind_map_blocks(NULL, inode, map, 0);
1751-
if (retval < 0)
1752-
goto out_unlock;
1749+
if (retval < 0) {
1750+
up_read(&EXT4_I(inode)->i_data_sem);
1751+
return retval;
1752+
}
17531753
if (retval > 0) {
17541754
unsigned int status;
17551755

@@ -1765,24 +1765,21 @@ static int ext4_da_map_blocks(struct inode *inode, sector_t iblock,
17651765
EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
17661766
ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
17671767
map->m_pblk, status);
1768-
goto out_unlock;
1768+
up_read(&EXT4_I(inode)->i_data_sem);
1769+
return retval;
17691770
}
1771+
up_read(&EXT4_I(inode)->i_data_sem);
17701772

17711773
add_delayed:
1772-
/*
1773-
* XXX: __block_prepare_write() unmaps passed block,
1774-
* is it OK?
1775-
*/
1774+
down_write(&EXT4_I(inode)->i_data_sem);
17761775
retval = ext4_insert_delayed_block(inode, map->m_lblk);
1776+
up_write(&EXT4_I(inode)->i_data_sem);
17771777
if (retval)
1778-
goto out_unlock;
1778+
return retval;
17791779

17801780
map_bh(bh, inode->i_sb, invalid_block);
17811781
set_buffer_new(bh);
17821782
set_buffer_delay(bh);
1783-
1784-
out_unlock:
1785-
up_read((&EXT4_I(inode)->i_data_sem));
17861783
return retval;
17871784
}
17881785

0 commit comments

Comments
 (0)