Re: [PATCH v2] ext4: fix the logical block counter overflow in indirect migration

[Jan Kara]

On Mon 14-09-26 16:25:16, Yichong Chen wrote:
> update_tind_extent_range() advances lb->curr_block, an ext4_lblk_t, by
> max_entries * max_entries for every empty triple-indirect slot.  One
> triple-indirect block spans max_entries^3 logical blocks, which exceeds
> 2^32 as soon as the block size is 8K or larger (16384^3 = 2^42 with 64K
> blocks), so the counter wraps while that block is walked.
> 
> A wrapped counter makes the migration store a block number that is 2^32
> blocks away from the one the pointer block describes.  Two ranges can then
> end up with the same ee_block, which trips
> BUG_ON(newext->ee_block == nearex->ee_block) in ext4_ext_insert_extent(),
> and without that collision the data is still moved to the wrong logical
> block while the migration reports success.
> 
> Keep the counter in 64 bit so that it cannot wrap, and refuse the
> migration with -EOPNOTSUPP once a data block would land on EXT_MAX_BLOCKS
> or beyond: an extent may not cover that block, because
> ext4_valid_extent() rejects a wrapping ee_block + ee_len, and only a
> corrupt block map has data there.
> 
> Fixes: c14c6fd5c56a ("ext4: Add EXT4_IOC_MIGRATE ioctl")
> Signed-off-by: Yichong Chen <chenyichong@xxxxxxxxxxxxx>

This is even better. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
> 
> Notes:
>     v2: refuse EXT_MAX_BLOCKS (0xffffffff) as well, so the bound is >= instead
>         of >.  An extent may not cover that block: ext4_valid_extent() rejects a
>         wrapping ee_block + ee_len, so storing it would leave the migrated inode
>         corrupt.
> 
>  fs/ext4/migrate.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index e06d847033a1..2ce587043945 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -14,7 +14,7 @@
>   * represented by a single extent
>   */
>  struct migrate_struct {
> -	ext4_lblk_t first_block, last_block, curr_block;
> +	u64 first_block, last_block, curr_block;
>  	ext4_fsblk_t first_pblock, last_pblock;
>  };
>  
> @@ -65,6 +65,10 @@ static int update_extent_range(handle_t *handle, struct inode *inode,
>  			       ext4_fsblk_t pblock, struct migrate_struct *lb)
>  {
>  	int retval;
> +
> +	if (lb->curr_block >= EXT_MAX_BLOCKS)
> +		return -EOPNOTSUPP;
> +
>  	/*
>  	 * See if we can add on to the existing range (if it exists)
>  	 */
> -- 
> 2.51.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR