linux-kernel - re: Btrfs: add a extent ref verify tool (static analysis bug report)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 2 Oct 2019 14:50:51 +0100
From:   Colin Ian King <colin.king@...onical.com>
To:     Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
        David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: re: Btrfs: add a extent ref verify tool (static analysis bug report)

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 <josef@...icpanda.com>
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