Re: [PATCH 1/2] btrfs-progs: convert: Prevent bit overflow for cctx->total_bytes

[Nikolay Borisov <nborisov@suse.com>]

On 20.07.20 г. 15:51 ч., Qu Wenruo wrote:
> [BUG]
> When convert is called on a 64GiB ext4 fs, it fails like this:
> 
>   $ btrfs-convert  /dev/loop0p1
>   create btrfs filesystem:
>           blocksize: 4096
>           nodesize:  16384
>           features:  extref, skinny-metadata (default)
>           checksum:  crc32c
>   creating ext2 image file
>   ERROR: missing data block for bytenr 1048576
>   ERROR: failed to create ext2_saved/image: -2
>   WARNING: an error occurred during conversion, filesystem is partially created but not finalized and not mountable
> 
> Btrfs-convert also corrupts the source fs:
>   $ LANG=C e2fsck /dev/loop0p1 -f
>   e2fsck 1.45.6 (20-Mar-2020)
>   Resize inode not valid.  Recreate<y>? yes
>   Pass 1: Checking inodes, blocks, and sizes
>   Deleted inode 3681 has zero dtime.  Fix<y>? yes
>   Inodes that were part of a corrupted orphan linked list found.  Fix<y>? yes
>   Inode 3744 was part of the orphaned inode list.  FIXED.
>   Deleted inode 3745 has zero dtime.  Fix<y>? yes
>   Inode 3747 has INLINE_DATA_FL flag on filesystem without inline data support.
>   Clear<y>? yes
>   ...
> 
> [CAUSE]
> After some debugging, the first strange behavior is, the value of
> cctx->total_bytes is 0 in ext2_open_fs().
> 
> It turns out that, the value assign for cctx->total_bytes could lead to
> bit overflow for the unsigned int value.
> 
> And that 0 cctx->total_bytes leads to vairous problems for later free
> space calculation.
> For example, in calculate_available_space(), we use cctx->total_bytes to
> ensure we won't create a data chunk beyond device end:
> 
> 		cue_len = min(cctx->total_bytes - cur_off, cur_len);
> 
> If that cur_offset is also 0, we will create a cache_extent with 0 size,
> which could cause a lot of problems for cache tree search.
> 
> [FIX]
> Do manual casting for the multiply operation, so we could got a real u64
> result.
> The fix will be applied to all supported fses (ext* and reiserfs).
> 
> Reported-by: Christian Zangl <coralllama@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>