From e2b4e2bc8865e03eecd49caa9713a2402a96bba9 Mon Sep 17 00:00:00 2001 From: Chao Yu Date: Wed, 19 Aug 2015 19:11:19 +0800 Subject: [PATCH] f2fs: fix incorrect mapping for bmap The test step is like below: 1. touch file 2. truncate -s $((1024*1024)) file 3. fallocate -o 0 -l $((1024*1024)) file 4. fibmap.f2fs file Our result of fibmap.f2fs showed below is not correct: file_pos start_blk end_blk blks 0 -937166132 -937166132 1 4096 -937166132 -937166132 1 8192 -937166132 -937166132 1 12288 -937166132 -937166132 1 16384 -937166132 -937166132 1 20480 -937166132 -937166132 1 ... 1040384 -937166132 -937166132 1 1044480 -937166132 -937166132 1 This is because f2fs_map_blocks will return with no error when meeting a hole or preallocated block, the caller __get_data_block will map the uninitialized variable value to bh->b_blocknr. Unfortunately generic_block_bmap will neither check the return value of get_data() nor check mapping info of buffer_head, result in returning the random block address. After fixing the issue, our result shows correctly: file_pos start_blk end_blk blks 0 0 0 256 Signed-off-by: Chao Yu Signed-off-by: Jaegeuk Kim --- fs/f2fs/data.c | 50 +++++++++++++++++++++++++++++++++++++------------- fs/f2fs/f2fs.h | 6 ++++++ 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index 726e58b76295..73713bbd4646 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -561,7 +561,7 @@ static void __allocate_data_blocks(struct inode *inode, loff_t offset, * c. give the block addresses to blockdev */ static int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, - int create, bool fiemap) + int create, int flag) { unsigned int maxblocks = map->m_len; struct dnode_of_data dn; @@ -595,8 +595,19 @@ static int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, err = 0; goto unlock_out; } - if (dn.data_blkaddr == NEW_ADDR && !fiemap) - goto put_out; + if (dn.data_blkaddr == NEW_ADDR) { + if (flag == F2FS_GET_BLOCK_BMAP) { + err = -ENOENT; + goto put_out; + } else if (flag == F2FS_GET_BLOCK_READ || + flag == F2FS_GET_BLOCK_DIO) { + goto put_out; + } + /* + * if it is in fiemap call path (flag = F2FS_GET_BLOCK_FIEMAP), + * mark it as mapped and unwritten block. + */ + } if (dn.data_blkaddr != NULL_ADDR) { map->m_flags = F2FS_MAP_MAPPED; @@ -611,6 +622,8 @@ static int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, map->m_flags = F2FS_MAP_NEW | F2FS_MAP_MAPPED; map->m_pblk = dn.data_blkaddr; } else { + if (flag == F2FS_GET_BLOCK_BMAP) + err = -ENOENT; goto put_out; } @@ -633,7 +646,9 @@ static int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, err = 0; goto unlock_out; } - if (dn.data_blkaddr == NEW_ADDR && !fiemap) + + if (dn.data_blkaddr == NEW_ADDR && + flag != F2FS_GET_BLOCK_FIEMAP) goto put_out; end_offset = ADDRS_PER_PAGE(dn.node_page, F2FS_I(inode)); @@ -675,7 +690,7 @@ static int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, } static int __get_data_block(struct inode *inode, sector_t iblock, - struct buffer_head *bh, int create, bool fiemap) + struct buffer_head *bh, int create, int flag) { struct f2fs_map_blocks map; int ret; @@ -683,7 +698,7 @@ static int __get_data_block(struct inode *inode, sector_t iblock, map.m_lblk = iblock; map.m_len = bh->b_size >> inode->i_blkbits; - ret = f2fs_map_blocks(inode, &map, create, fiemap); + ret = f2fs_map_blocks(inode, &map, create, flag); if (!ret) { map_bh(bh, inode->i_sb, map.m_pblk); bh->b_state = (bh->b_state & ~F2FS_MAP_FLAGS) | map.m_flags; @@ -693,15 +708,23 @@ static int __get_data_block(struct inode *inode, sector_t iblock, } static int get_data_block(struct inode *inode, sector_t iblock, - struct buffer_head *bh_result, int create) + struct buffer_head *bh_result, int create, int flag) { - return __get_data_block(inode, iblock, bh_result, create, false); + return __get_data_block(inode, iblock, bh_result, create, flag); } -static int get_data_block_fiemap(struct inode *inode, sector_t iblock, +static int get_data_block_dio(struct inode *inode, sector_t iblock, struct buffer_head *bh_result, int create) { - return __get_data_block(inode, iblock, bh_result, create, true); + return __get_data_block(inode, iblock, bh_result, create, + F2FS_GET_BLOCK_DIO); +} + +static int get_data_block_bmap(struct inode *inode, sector_t iblock, + struct buffer_head *bh_result, int create) +{ + return __get_data_block(inode, iblock, bh_result, create, + F2FS_GET_BLOCK_BMAP); } static inline sector_t logical_to_blk(struct inode *inode, loff_t offset) @@ -745,7 +768,8 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, memset(&map_bh, 0, sizeof(struct buffer_head)); map_bh.b_size = len; - ret = get_data_block_fiemap(inode, start_blk, &map_bh, 0); + ret = get_data_block(inode, start_blk, &map_bh, 0, + F2FS_GET_BLOCK_FIEMAP); if (ret) goto out; @@ -1530,7 +1554,7 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter, if (iov_iter_rw(iter) == WRITE) __allocate_data_blocks(inode, offset, count); - err = blockdev_direct_IO(iocb, inode, iter, offset, get_data_block); + err = blockdev_direct_IO(iocb, inode, iter, offset, get_data_block_dio); if (err < 0 && iov_iter_rw(iter) == WRITE) f2fs_write_failed(mapping, offset + count); @@ -1618,7 +1642,7 @@ static sector_t f2fs_bmap(struct address_space *mapping, sector_t block) if (err) return err; } - return generic_block_bmap(mapping, block, get_data_block); + return generic_block_bmap(mapping, block, get_data_block_bmap); } const struct address_space_operations f2fs_dblock_aops = { diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 00591f725744..51dfa8fcc505 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -375,6 +375,12 @@ struct f2fs_map_blocks { unsigned int m_flags; }; +/* for flag in get_data_block */ +#define F2FS_GET_BLOCK_READ 0 +#define F2FS_GET_BLOCK_DIO 1 +#define F2FS_GET_BLOCK_FIEMAP 2 +#define F2FS_GET_BLOCK_BMAP 3 + /* * i_advise uses FADVISE_XXX_BIT. We can add additional hints later. */