Re: [PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2608 bytes --]



On 2018年04月30日 11:49, Su Yue wrote:
> 
> 
> On 04/30/2018 11:15 AM, Qu Wenruo wrote:
>> For btrfs_print_tree(), if nr_items is corrupted, it can easily go
>> beyond extent buffer boundary.
>>
>> Add extra nr_item check, and only print as many valid slots as possible.
>>
> 
> Make sense.
> 
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   print-tree.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/print-tree.c b/print-tree.c
>> index 31a851ef4413..55db80bebb2a 100644
>> --- a/print-tree.c
>> +++ b/print-tree.c
>> @@ -1376,6 +1376,11 @@ void btrfs_print_tree(struct extent_buffer *eb,
>> int follow)
>>           btrfs_print_leaf(eb);
>>           return;
>>       }
>> +    /* We are crossing eb boundary, this node must be corrupted */
>> +    if (nr > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
>> +        warning(
>> +        "node nr_items corrupted, has %u limit %u, continue print
>> anyway",
>> +            nr, BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb));
>>       printf("node %llu level %d items %d free %u generation %llu
>> owner ",
>>              (unsigned long long)eb->start,
>>               btrfs_header_level(eb), nr,
>> @@ -1386,7 +1391,11 @@ void btrfs_print_tree(struct extent_buffer *eb,
>> int follow)
>>       print_uuids(eb);
>>       fflush(stdout);
>>          
>> -        u64 blocknr = btrfs_node_blockptr(eb, i);
>> +        u64 blocknr;
>> +
>> +        if (i > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
>> +            break;
> 
> Should it be i >= BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb)?

BTRFS_NODEPTRS_PER_EXTENT_BUFFER() provides the maximum valid number.
So it 's >=.

> 
> Here BTRFS_NODEPTRS_PER_EXTENT_BUFFER() is called during iterations.
> The judement can be calculated in advance like:
> 
>     ptr_num = BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb);
>     ...
>     for (i = 0; i < nr && i < ptr_num  ; i++) {

Indeed looks better.

Thanks,
Qu

> 
> Thanks,
> Su
> 
>> +        blocknr = btrfs_node_blockptr(eb, i);
>>           btrfs_node_key(eb, &disk_key, i);
>>           btrfs_disk_key_to_cpu(&key, &disk_key);
>>           printf("\t");
>>
> 
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]