[6/6] btrfs-progs: extent-tree: Fix a by-one error in exclude_super_stripes()
diff mbox series

Message ID 20191218011942.9830-7-wqu@suse.com
State New
Headers show
Series
  • btrfs-progs: Fixes for github issues
Related show

Commit Message

Qu Wenruo Dec. 18, 2019, 1:19 a.m. UTC
[BUG]
For certain btrfs images, a BUG_ON() can be triggered at open_ctree()
time:
  Opening filesystem to check...
  extent_io.c:158: insert_state: BUG_ON `end < start` triggered, value 1
  btrfs(+0x2de57)[0x560c4d7cfe57]
  btrfs(+0x2e210)[0x560c4d7d0210]
  btrfs(set_extent_bits+0x254)[0x560c4d7d0854]
  btrfs(exclude_super_stripes+0xbf)[0x560c4d7c65ff]
  btrfs(btrfs_read_block_groups+0x29d)[0x560c4d7c698d]
  btrfs(btrfs_setup_all_roots+0x3f3)[0x560c4d7c0b23]
  btrfs(+0x1ef53)[0x560c4d7c0f53]
  btrfs(open_ctree_fs_info+0x90)[0x560c4d7c11a0]
  btrfs(+0x6d3f9)[0x560c4d80f3f9]
  btrfs(main+0x94)[0x560c4d7b60c4]
  /usr/lib/libc.so.6(__libc_start_main+0xf3)[0x7fd189773ee3]
  btrfs(_start+0x2e)[0x560c4d7b635e]

[CAUSE]
This is caused by passing @len == 0 to add_excluded_extent(), which
means one revsere mapped range is just out of the block group range,
normally means a by-one error.

[FIX]
Fix the boundary check on the reserve mapped range against block group
range.
If a reverse mapped super block starts at the end of the block group, it
doesn't cover so we don't need to bother the case.

Issue: #210
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 extent-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

David Sterba Jan. 2, 2020, 4:56 p.m. UTC | #1
On Wed, Dec 18, 2019 at 09:19:42AM +0800, Qu Wenruo wrote:
> [BUG]
> For certain btrfs images, a BUG_ON() can be triggered at open_ctree()
> time:
>   Opening filesystem to check...
>   extent_io.c:158: insert_state: BUG_ON `end < start` triggered, value 1
>   btrfs(+0x2de57)[0x560c4d7cfe57]
>   btrfs(+0x2e210)[0x560c4d7d0210]
>   btrfs(set_extent_bits+0x254)[0x560c4d7d0854]
>   btrfs(exclude_super_stripes+0xbf)[0x560c4d7c65ff]
>   btrfs(btrfs_read_block_groups+0x29d)[0x560c4d7c698d]
>   btrfs(btrfs_setup_all_roots+0x3f3)[0x560c4d7c0b23]
>   btrfs(+0x1ef53)[0x560c4d7c0f53]
>   btrfs(open_ctree_fs_info+0x90)[0x560c4d7c11a0]
>   btrfs(+0x6d3f9)[0x560c4d80f3f9]
>   btrfs(main+0x94)[0x560c4d7b60c4]
>   /usr/lib/libc.so.6(__libc_start_main+0xf3)[0x7fd189773ee3]
>   btrfs(_start+0x2e)[0x560c4d7b635e]
> 
> [CAUSE]
> This is caused by passing @len == 0 to add_excluded_extent(), which
> means one revsere mapped range is just out of the block group range,
> normally means a by-one error.
> 
> [FIX]
> Fix the boundary check on the reserve mapped range against block group
> range.
> If a reverse mapped super block starts at the end of the block group, it
> doesn't cover so we don't need to bother the case.
> 
> Issue: #210
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  extent-tree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/extent-tree.c b/extent-tree.c
> index 6288c8a3..7ba80375 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -3640,7 +3640,7 @@ int exclude_super_stripes(struct btrfs_fs_info *fs_info,
>  		while (nr--) {
>  			u64 start, len;
>  
> -			if (logical[nr] > cache->key.objectid +
> +			if (logical[nr] >= cache->key.objectid +
>  			    cache->key.offset)

Do we have the same problem in kernel? The code does ">".
Qu Wenruo Jan. 3, 2020, 12:42 a.m. UTC | #2
On 2020/1/3 上午12:56, David Sterba wrote:
> On Wed, Dec 18, 2019 at 09:19:42AM +0800, Qu Wenruo wrote:
>> [BUG]
>> For certain btrfs images, a BUG_ON() can be triggered at open_ctree()
>> time:
>>   Opening filesystem to check...
>>   extent_io.c:158: insert_state: BUG_ON `end < start` triggered, value 1
>>   btrfs(+0x2de57)[0x560c4d7cfe57]
>>   btrfs(+0x2e210)[0x560c4d7d0210]
>>   btrfs(set_extent_bits+0x254)[0x560c4d7d0854]
>>   btrfs(exclude_super_stripes+0xbf)[0x560c4d7c65ff]
>>   btrfs(btrfs_read_block_groups+0x29d)[0x560c4d7c698d]
>>   btrfs(btrfs_setup_all_roots+0x3f3)[0x560c4d7c0b23]
>>   btrfs(+0x1ef53)[0x560c4d7c0f53]
>>   btrfs(open_ctree_fs_info+0x90)[0x560c4d7c11a0]
>>   btrfs(+0x6d3f9)[0x560c4d80f3f9]
>>   btrfs(main+0x94)[0x560c4d7b60c4]
>>   /usr/lib/libc.so.6(__libc_start_main+0xf3)[0x7fd189773ee3]
>>   btrfs(_start+0x2e)[0x560c4d7b635e]
>>
>> [CAUSE]
>> This is caused by passing @len == 0 to add_excluded_extent(), which
>> means one revsere mapped range is just out of the block group range,
>> normally means a by-one error.
>>
>> [FIX]
>> Fix the boundary check on the reserve mapped range against block group
>> range.
>> If a reverse mapped super block starts at the end of the block group, it
>> doesn't cover so we don't need to bother the case.
>>
>> Issue: #210
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  extent-tree.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/extent-tree.c b/extent-tree.c
>> index 6288c8a3..7ba80375 100644
>> --- a/extent-tree.c
>> +++ b/extent-tree.c
>> @@ -3640,7 +3640,7 @@ int exclude_super_stripes(struct btrfs_fs_info *fs_info,
>>  		while (nr--) {
>>  			u64 start, len;
>>  
>> -			if (logical[nr] > cache->key.objectid +
>> +			if (logical[nr] >= cache->key.objectid +
>>  			    cache->key.offset)
> 
> Do we have the same problem in kernel? The code does ">".
> 
Oh, kernel looks to have the same problem.

Time to fix it.

Thanks,
Qu
Su Yue Jan. 3, 2020, 3:04 a.m. UTC | #3
On 2020/1/3 8:42 AM, Qu Wenruo wrote:
>
>
> On 2020/1/3 上午12:56, David Sterba wrote:
>> On Wed, Dec 18, 2019 at 09:19:42AM +0800, Qu Wenruo wrote:
>>> [BUG]
>>> For certain btrfs images, a BUG_ON() can be triggered at open_ctree()
>>> time:
>>>    Opening filesystem to check...
>>>    extent_io.c:158: insert_state: BUG_ON `end < start` triggered, value 1
>>>    btrfs(+0x2de57)[0x560c4d7cfe57]
>>>    btrfs(+0x2e210)[0x560c4d7d0210]
>>>    btrfs(set_extent_bits+0x254)[0x560c4d7d0854]
>>>    btrfs(exclude_super_stripes+0xbf)[0x560c4d7c65ff]
>>>    btrfs(btrfs_read_block_groups+0x29d)[0x560c4d7c698d]
>>>    btrfs(btrfs_setup_all_roots+0x3f3)[0x560c4d7c0b23]
>>>    btrfs(+0x1ef53)[0x560c4d7c0f53]
>>>    btrfs(open_ctree_fs_info+0x90)[0x560c4d7c11a0]
>>>    btrfs(+0x6d3f9)[0x560c4d80f3f9]
>>>    btrfs(main+0x94)[0x560c4d7b60c4]
>>>    /usr/lib/libc.so.6(__libc_start_main+0xf3)[0x7fd189773ee3]
>>>    btrfs(_start+0x2e)[0x560c4d7b635e]
>>>
>>> [CAUSE]
>>> This is caused by passing @len == 0 to add_excluded_extent(), which
>>> means one revsere mapped range is just out of the block group range,
>>> normally means a by-one error.
>>>
>>> [FIX]
>>> Fix the boundary check on the reserve mapped range against block group
>>> range.
>>> If a reverse mapped super block starts at the end of the block group, it
>>> doesn't cover so we don't need to bother the case.
>>>
>>> Issue: #210
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>> ---
>>>   extent-tree.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/extent-tree.c b/extent-tree.c
>>> index 6288c8a3..7ba80375 100644
>>> --- a/extent-tree.c
>>> +++ b/extent-tree.c
>>> @@ -3640,7 +3640,7 @@ int exclude_super_stripes(struct btrfs_fs_info *fs_info,
>>>   		while (nr--) {
>>>   			u64 start, len;
>>>
>>> -			if (logical[nr] > cache->key.objectid +
>>> +			if (logical[nr] >= cache->key.objectid +
>>>   			    cache->key.offset)
>>
>> Do we have the same problem in kernel? The code does ">".
>>
> Oh, kernel looks to have the same problem.
>

Gentle reminder to save your time.

I saw the '>' as in the kernel code and send a patch for it.
Then Nikolay pointed out that the patch is unnecessary[1].
The fuzz image is rejected to be mounted by tree-checker earlier.

Besides, a cleanup series[2] was sent by him.

[1]:
https://lore.kernel.org/linux-btrfs/24721dc8-8bda-a086-ff1a-31a0b21a02b4@suse.com/T/#m4cc5153d1645177e0f81f00c86e10dfb36015def
[2]: https://patchwork.kernel.org/project/linux-btrfs/list/?series=205103

> Time to fix it.
>
> Thanks,
> Qu
>

Patch
diff mbox series

diff --git a/extent-tree.c b/extent-tree.c
index 6288c8a3..7ba80375 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -3640,7 +3640,7 @@  int exclude_super_stripes(struct btrfs_fs_info *fs_info,
 		while (nr--) {
 			u64 start, len;
 
-			if (logical[nr] > cache->key.objectid +
+			if (logical[nr] >= cache->key.objectid +
 			    cache->key.offset)
 				continue;