Hi,
Static analysis on linux-next with Coverity has picked up a potential
issue in file fs/btrfs/ref-verify.c, function process_leaf() in the
following commit:
commit fd708b81d972a0714b02a60eb4792fdbf15868c4
Author: Josef Bacik <[email protected]>
Date: Fri Sep 29 15:43:50 2017 -0400
Btrfs: add a extent ref verify tool
The potential issue is when on the unlikely event when all the items
contain unknown key.types and so ret is not assigned a value. Since ret
is not initialized then a garbage value is returned by this function in
this unlikely scenario.
In the previous function process_extent_item any unknown key types are
flagged up as an error and -EINVAL is returned. I'm unsure if this kind
of error handling should also be applied to function process_leaf with
invalid key types too.
The coverity analysis follows:
495static int process_leaf(struct btrfs_root *root,
496 struct btrfs_path *path, u64 *bytenr, u64
*num_bytes)
497{
498 struct btrfs_fs_info *fs_info = root->fs_info;
499 struct extent_buffer *leaf = path->nodes[0];
500 struct btrfs_extent_data_ref *dref;
501 struct btrfs_shared_data_ref *sref;
502 u32 count;
1. var_decl: Declaring variable ret without initializer.
503 int i = 0, tree_block_level = 0, ret;
504 struct btrfs_key key;
505 int nritems = btrfs_header_nritems(leaf);
506
2. Condition i < nritems, taking true branch.
507 for (i = 0; i < nritems; i++) {
508 btrfs_item_key_to_cpu(leaf, &key, i);
3. Switch case default.
509 switch (key.type) {
510 case BTRFS_EXTENT_ITEM_KEY:
511 *num_bytes = key.offset;
512 /* fall through */
513 case BTRFS_METADATA_ITEM_KEY:
514 *bytenr = key.objectid;
515 ret = process_extent_item(fs_info, path, &key, i,
516 &tree_block_level);
517 break;
518 case BTRFS_TREE_BLOCK_REF_KEY:
519 ret = add_tree_block(fs_info, key.offset, 0,
520 key.objectid,
tree_block_level);
521 break;
522 case BTRFS_SHARED_BLOCK_REF_KEY:
523 ret = add_tree_block(fs_info, 0, key.offset,
524 key.objectid,
tree_block_level);
525 break;
526 case BTRFS_EXTENT_DATA_REF_KEY:
527 dref = btrfs_item_ptr(leaf, i,
528 struct
btrfs_extent_data_ref);
529 ret = add_extent_data_ref(fs_info, leaf,
dref, *bytenr,
530 *num_bytes);
531 break;
532 case BTRFS_SHARED_DATA_REF_KEY:
533 sref = btrfs_item_ptr(leaf, i,
534 struct
btrfs_shared_data_ref);
535 count = btrfs_shared_data_ref_count(leaf, sref);
536 ret = add_shared_data_ref(fs_info,
key.offset, count,
537 *bytenr, *num_bytes);
538 break;
539 default:
4. Breaking from switch.
540 break;
541 }
CID 19605 (#1 of 1): Uninitialized scalar variable (UNINIT)
5. uninit_use: Using uninitialized value ret.
542 if (ret)
543 break;
544 }
545 return ret;
546}
Colin
On Wed, Oct 02, 2019 at 02:50:51PM +0100, Colin Ian King wrote:
> Hi,
>
> Static analysis on linux-next with Coverity has picked up a potential
> issue in file fs/btrfs/ref-verify.c, function process_leaf() in the
> following commit:
>
> commit fd708b81d972a0714b02a60eb4792fdbf15868c4
> Author: Josef Bacik <[email protected]>
> Date: Fri Sep 29 15:43:50 2017 -0400
>
> Btrfs: add a extent ref verify tool
>
> The potential issue is when on the unlikely event when all the items
> contain unknown key.types and so ret is not assigned a value. Since ret
> is not initialized then a garbage value is returned by this function in
> this unlikely scenario.
>
> In the previous function process_extent_item any unknown key types are
> flagged up as an error and -EINVAL is returned. I'm unsure if this kind
> of error handling should also be applied to function process_leaf with
> invalid key types too.
>
Thanks I'll fix this up. You can run into block group item key types and we
don't care about those, so we just need ret = 0; Thanks,
Josef