[1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
diff mbox series

Message ID 20200624115527.855816-1-wqu@suse.com
State New
Headers show
Series
  • [1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
Related show

Commit Message

Qu Wenruo June 24, 2020, 11:55 a.m. UTC
[BUG]
The following script could lead to corrupted btrfs fs after
btrfs-convert:

  fallocate -l 1G test.img
  mkfs.ext4 test.img
  mount test.img $mnt
  fallocate -l 200m $mnt/file1
  fallocate -l 200m $mnt/file2
  fallocate -l 200m $mnt/file3
  fallocate -l 200m $mnt/file4
  fallocate -l 205m $mnt/file1
  fallocate -l 205m $mnt/file2
  fallocate -l 205m $mnt/file3
  fallocate -l 205m $mnt/file4
  umount $mnt
  btrfs-convert test.img

The result btrfs will have a device extent beyond its boundary:
  pening filesystem to check...
  Checking filesystem on test.img
  UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
  [1/7] checking root items
  [2/7] checking extents
  ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
  ERROR: errors found in extent allocation tree or chunk allocation
  [3/7] checking free space cache
  [4/7] checking fs roots
  [5/7] checking only csums items (without verifying data)
  [6/7] checking root refs
  [7/7] checking quota groups skipped (not enabled on this FS)
  found 913960960 bytes used, error(s) found
  total csum bytes: 891500
  total tree bytes: 1064960
  total fs tree bytes: 49152
  total extent tree bytes: 16384
  btree space waste bytes: 144885
  file data blocks allocated: 2129063936
   referenced 1772728320

[CAUSE]
Btrfs-convert first collect all used blocks in the original fs, then
slightly enlarge the used blocks range as new btrfs data chunks.

However the enlarge part has a problem, that it doesn't take the device
boundary into consideration.

Thus it caused device extents and data chunks to go beyond device
boundary.

[FIX]
Just to extra check before inserting data chunks into
btrfs_convert_context::data_chunk.

Reported-by: Jiachen YANG <farseerfc@gmail.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/main.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

David Sterba June 24, 2020, 3:58 p.m. UTC | #1
On Wed, Jun 24, 2020 at 07:55:26PM +0800, Qu Wenruo wrote:
> [BUG]
> The following script could lead to corrupted btrfs fs after
> btrfs-convert:
> 
>   fallocate -l 1G test.img
>   mkfs.ext4 test.img
>   mount test.img $mnt
>   fallocate -l 200m $mnt/file1
>   fallocate -l 200m $mnt/file2
>   fallocate -l 200m $mnt/file3
>   fallocate -l 200m $mnt/file4
>   fallocate -l 205m $mnt/file1
>   fallocate -l 205m $mnt/file2
>   fallocate -l 205m $mnt/file3
>   fallocate -l 205m $mnt/file4
>   umount $mnt
>   btrfs-convert test.img
> 
> The result btrfs will have a device extent beyond its boundary:
>   pening filesystem to check...
>   Checking filesystem on test.img
>   UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
>   [1/7] checking root items
>   [2/7] checking extents
>   ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
>   ERROR: errors found in extent allocation tree or chunk allocation
>   [3/7] checking free space cache
>   [4/7] checking fs roots
>   [5/7] checking only csums items (without verifying data)
>   [6/7] checking root refs
>   [7/7] checking quota groups skipped (not enabled on this FS)
>   found 913960960 bytes used, error(s) found
>   total csum bytes: 891500
>   total tree bytes: 1064960
>   total fs tree bytes: 49152
>   total extent tree bytes: 16384
>   btree space waste bytes: 144885
>   file data blocks allocated: 2129063936
>    referenced 1772728320
> 
> [CAUSE]
> Btrfs-convert first collect all used blocks in the original fs, then
> slightly enlarge the used blocks range as new btrfs data chunks.
> 
> However the enlarge part has a problem, that it doesn't take the device
> boundary into consideration.
> 
> Thus it caused device extents and data chunks to go beyond device
> boundary.
> 
> [FIX]
> Just to extra check before inserting data chunks into
> btrfs_convert_context::data_chunk.
> 
> Reported-by: Jiachen YANG <farseerfc@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

With the minor tweaks, added to devel, thanks.
Anand Jain Aug. 15, 2020, 1:38 p.m. UTC | #2
Before this patch the converted ext4 was failing to mount due to 'beyond
device boundary' error.

After this patch

# ./btrfs-convert /dev/sda7
create btrfs filesystem:
	blocksize: 4096
	nodesize:  16384
	features:  extref, skinny-metadata (default)
	checksum:  crc32c
creating ext2 image file
ERROR: data bytenr 1644167168 is covered by non-data block group 
1644167168 flags 0x4
ERROR: failed to create ext2_saved/image: -22
WARNING: an error occurred during conversion, filesystem is partially 
created but not finalized and not mountable


Any idea?


On 24/6/20 7:55 pm, Qu Wenruo wrote:
> [BUG]
> The following script could lead to corrupted btrfs fs after
> btrfs-convert:
> 
>    fallocate -l 1G test.img
>    mkfs.ext4 test.img
>    mount test.img $mnt
>    fallocate -l 200m $mnt/file1
>    fallocate -l 200m $mnt/file2
>    fallocate -l 200m $mnt/file3
>    fallocate -l 200m $mnt/file4
>    fallocate -l 205m $mnt/file1
>    fallocate -l 205m $mnt/file2
>    fallocate -l 205m $mnt/file3
>    fallocate -l 205m $mnt/file4
>    umount $mnt
>    btrfs-convert test.img
> 
> The result btrfs will have a device extent beyond its boundary:
>    pening filesystem to check...
>    Checking filesystem on test.img
>    UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
>    [1/7] checking root items
>    [2/7] checking extents
>    ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
>    ERROR: errors found in extent allocation tree or chunk allocation
>    [3/7] checking free space cache
>    [4/7] checking fs roots
>    [5/7] checking only csums items (without verifying data)
>    [6/7] checking root refs
>    [7/7] checking quota groups skipped (not enabled on this FS)
>    found 913960960 bytes used, error(s) found
>    total csum bytes: 891500
>    total tree bytes: 1064960
>    total fs tree bytes: 49152
>    total extent tree bytes: 16384
>    btree space waste bytes: 144885
>    file data blocks allocated: 2129063936
>     referenced 1772728320
> 
> [CAUSE]
> Btrfs-convert first collect all used blocks in the original fs, then
> slightly enlarge the used blocks range as new btrfs data chunks.
> 
> However the enlarge part has a problem, that it doesn't take the device
> boundary into consideration.
> 
> Thus it caused device extents and data chunks to go beyond device
> boundary.
> 
> [FIX]
> Just to extra check before inserting data chunks into
> btrfs_convert_context::data_chunk.
> 
> Reported-by: Jiachen YANG <farseerfc@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   convert/main.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/convert/main.c b/convert/main.c
> index c86ddd988c63..7709e9a6c085 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -669,6 +669,8 @@ static int calculate_available_space(struct btrfs_convert_context *cctx)
>   			cur_off = cache->start;
>   		cur_len = max(cache->start + cache->size - cur_off,
>   			      min_stripe_size);
> +		/* data chunks should never exceed device boundary */
> +		cur_len = min(cctx->total_bytes - cur_off, cur_len);
>   		ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
>   		if (ret < 0)
>   			goto out;
>
Qu Wenruo Aug. 15, 2020, 11:40 p.m. UTC | #3
On 2020/8/15 下午9:38, Anand Jain wrote:
> 
> Before this patch the converted ext4 was failing to mount due to 'beyond
> device boundary' error.

e2image -Q please.

Thanks,
Qu
> 
> After this patch
> 
> # ./btrfs-convert /dev/sda7
> create btrfs filesystem:
>     blocksize: 4096
>     nodesize:  16384
>     features:  extref, skinny-metadata (default)
>     checksum:  crc32c
> creating ext2 image file
> ERROR: data bytenr 1644167168 is covered by non-data block group
> 1644167168 flags 0x4
> ERROR: failed to create ext2_saved/image: -22
> WARNING: an error occurred during conversion, filesystem is partially
> created but not finalized and not mountable
> 
> 
> Any idea?
> 
> 
> On 24/6/20 7:55 pm, Qu Wenruo wrote:
>> [BUG]
>> The following script could lead to corrupted btrfs fs after
>> btrfs-convert:
>>
>>    fallocate -l 1G test.img
>>    mkfs.ext4 test.img
>>    mount test.img $mnt
>>    fallocate -l 200m $mnt/file1
>>    fallocate -l 200m $mnt/file2
>>    fallocate -l 200m $mnt/file3
>>    fallocate -l 200m $mnt/file4
>>    fallocate -l 205m $mnt/file1
>>    fallocate -l 205m $mnt/file2
>>    fallocate -l 205m $mnt/file3
>>    fallocate -l 205m $mnt/file4
>>    umount $mnt
>>    btrfs-convert test.img
>>
>> The result btrfs will have a device extent beyond its boundary:
>>    pening filesystem to check...
>>    Checking filesystem on test.img
>>    UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
>>    [1/7] checking root items
>>    [2/7] checking extents
>>    ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is
>> beyond device boundary 1073741824
>>    ERROR: errors found in extent allocation tree or chunk allocation
>>    [3/7] checking free space cache
>>    [4/7] checking fs roots
>>    [5/7] checking only csums items (without verifying data)
>>    [6/7] checking root refs
>>    [7/7] checking quota groups skipped (not enabled on this FS)
>>    found 913960960 bytes used, error(s) found
>>    total csum bytes: 891500
>>    total tree bytes: 1064960
>>    total fs tree bytes: 49152
>>    total extent tree bytes: 16384
>>    btree space waste bytes: 144885
>>    file data blocks allocated: 2129063936
>>     referenced 1772728320
>>
>> [CAUSE]
>> Btrfs-convert first collect all used blocks in the original fs, then
>> slightly enlarge the used blocks range as new btrfs data chunks.
>>
>> However the enlarge part has a problem, that it doesn't take the device
>> boundary into consideration.
>>
>> Thus it caused device extents and data chunks to go beyond device
>> boundary.
>>
>> [FIX]
>> Just to extra check before inserting data chunks into
>> btrfs_convert_context::data_chunk.
>>
>> Reported-by: Jiachen YANG <farseerfc@gmail.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   convert/main.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/convert/main.c b/convert/main.c
>> index c86ddd988c63..7709e9a6c085 100644
>> --- a/convert/main.c
>> +++ b/convert/main.c
>> @@ -669,6 +669,8 @@ static int calculate_available_space(struct
>> btrfs_convert_context *cctx)
>>               cur_off = cache->start;
>>           cur_len = max(cache->start + cache->size - cur_off,
>>                     min_stripe_size);
>> +        /* data chunks should never exceed device boundary */
>> +        cur_len = min(cctx->total_bytes - cur_off, cur_len);
>>           ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
>>           if (ret < 0)
>>               goto out;
>>
>

Patch
diff mbox series

diff --git a/convert/main.c b/convert/main.c
index c86ddd988c63..7709e9a6c085 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -669,6 +669,8 @@  static int calculate_available_space(struct btrfs_convert_context *cctx)
 			cur_off = cache->start;
 		cur_len = max(cache->start + cache->size - cur_off,
 			      min_stripe_size);
+		/* data chunks should never exceed device boundary */
+		cur_len = min(cctx->total_bytes - cur_off, cur_len);
 		ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
 		if (ret < 0)
 			goto out;