Re: [kdave-btrfs-devel:for-next-20220307 108/147] fs/btrfs/scrub.c:3146 scrub_raid56_data_stripe_for_parity() error: we previously assumed 'bioc' could be null (see line 3138)

From: Qu Wenruo
Date: Tue Mar 08 2022 - 08:33:52 EST




On 2022/3/8 21:09, Dan Carpenter wrote:
tree: https://github.com/kdave/btrfs-devel.git for-next-20220307
head: 912dedd70aeb485247c507115704ea7d137d758b
commit: 80cd926eefca522182ee3cf04d8e9984073d34d1 [108/147] btrfs: refactor scrub_raid56_parity()
config: i386-randconfig-m021-20220307 (https://download.01.org/0day-ci/archive/20220308/202203081837.zOttrQRN-lkp@xxxxxxxxx/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@xxxxxxxxx>
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>

New smatch warnings:
fs/btrfs/scrub.c:3171 scrub_raid56_data_stripe_for_parity() error: uninitialized symbol 'extent_start'.
fs/btrfs/scrub.c:3172 scrub_raid56_data_stripe_for_parity() error: uninitialized symbol 'extent_size'.

On that branch, the final code no longer has uninitialized @extent_start/@extent_size.

As the latest code calls get_extent_info() immediately after find_first_extent_item().

Furthermore, even at commit "btrfs: refactor scrub_raid56_parity()", IMHO those warnings are still false positives, explained below.


Old smatch warnings:
fs/btrfs/scrub.c:3419 scrub_simple_mirror() error: uninitialized symbol 'ret'.

vim +/bioc +3146 fs/btrfs/scrub.c

093741d4cda2cb4 Qu Wenruo 2022-02-18 3021
80cd926eefca522 Qu Wenruo 2022-02-18 3022 static int scrub_raid56_data_stripe_for_parity(struct scrub_ctx *sctx,
80cd926eefca522 Qu Wenruo 2022-02-18 3023 struct scrub_parity *sparity,
5a6ac9eacb49143 Miao Xie 2014-11-06 3024 struct map_lookup *map,
5a6ac9eacb49143 Miao Xie 2014-11-06 3025 struct btrfs_device *sdev,
80cd926eefca522 Qu Wenruo 2022-02-18 3026 struct btrfs_path *path,
80cd926eefca522 Qu Wenruo 2022-02-18 3027 u64 logical)
5a6ac9eacb49143 Miao Xie 2014-11-06 3028 {
fb456252d3d9c05 Jeff Mahoney 2016-06-22 3029 struct btrfs_fs_info *fs_info = sctx->fs_info;
80cd926eefca522 Qu Wenruo 2022-02-18 3030 struct btrfs_root *extent_root = btrfs_extent_root(fs_info, logical);
80cd926eefca522 Qu Wenruo 2022-02-18 3031 struct btrfs_root *csum_root = btrfs_csum_root(fs_info, logical);
5a6ac9eacb49143 Miao Xie 2014-11-06 3032 struct btrfs_key key;
80cd926eefca522 Qu Wenruo 2022-02-18 3033 u64 extent_start;
80cd926eefca522 Qu Wenruo 2022-02-18 3034 u64 extent_size;
80cd926eefca522 Qu Wenruo 2022-02-18 3035 int ret;
2522dbe86b54ff0 Qu Wenruo 2021-12-14 3036
80cd926eefca522 Qu Wenruo 2022-02-18 3037 ASSERT(map->type & BTRFS_BLOCK_GROUP_RAID56_MASK);
5a6ac9eacb49143 Miao Xie 2014-11-06 3038
80cd926eefca522 Qu Wenruo 2022-02-18 3039 /* Path should not be populated */
80cd926eefca522 Qu Wenruo 2022-02-18 3040 ASSERT(!path->nodes[0]);
5a6ac9eacb49143 Miao Xie 2014-11-06 3041
5a6ac9eacb49143 Miao Xie 2014-11-06 3042 if (btrfs_fs_incompat(fs_info, SKINNY_METADATA))
5a6ac9eacb49143 Miao Xie 2014-11-06 3043 key.type = BTRFS_METADATA_ITEM_KEY;
5a6ac9eacb49143 Miao Xie 2014-11-06 3044 else
5a6ac9eacb49143 Miao Xie 2014-11-06 3045 key.type = BTRFS_EXTENT_ITEM_KEY;
80cd926eefca522 Qu Wenruo 2022-02-18 3046 key.objectid = logical;
5a6ac9eacb49143 Miao Xie 2014-11-06 3047 key.offset = (u64)-1;
5a6ac9eacb49143 Miao Xie 2014-11-06 3048
80cd926eefca522 Qu Wenruo 2022-02-18 3049 ret = btrfs_search_slot(NULL, extent_root, &key, path, 0, 0);
5a6ac9eacb49143 Miao Xie 2014-11-06 3050 if (ret < 0)
80cd926eefca522 Qu Wenruo 2022-02-18 3051 return ret;
5a6ac9eacb49143 Miao Xie 2014-11-06 3052
5a6ac9eacb49143 Miao Xie 2014-11-06 3053 if (ret > 0) {
80cd926eefca522 Qu Wenruo 2022-02-18 3054 ret = btrfs_previous_extent_item(extent_root, path, 0);
5a6ac9eacb49143 Miao Xie 2014-11-06 3055 if (ret < 0)
80cd926eefca522 Qu Wenruo 2022-02-18 3056 return ret;
5a6ac9eacb49143 Miao Xie 2014-11-06 3057 if (ret > 0) {
5a6ac9eacb49143 Miao Xie 2014-11-06 3058 btrfs_release_path(path);
80cd926eefca522 Qu Wenruo 2022-02-18 3059 ret = btrfs_search_slot(NULL, extent_root, &key, path,
80cd926eefca522 Qu Wenruo 2022-02-18 3060 0, 0);
5a6ac9eacb49143 Miao Xie 2014-11-06 3061 if (ret < 0)
80cd926eefca522 Qu Wenruo 2022-02-18 3062 return ret;
5a6ac9eacb49143 Miao Xie 2014-11-06 3063 }
5a6ac9eacb49143 Miao Xie 2014-11-06 3064 }
5a6ac9eacb49143 Miao Xie 2014-11-06 3065
5a6ac9eacb49143 Miao Xie 2014-11-06 3066 while (1) {
80cd926eefca522 Qu Wenruo 2022-02-18 3067 struct btrfs_io_context *bioc = NULL;
80cd926eefca522 Qu Wenruo 2022-02-18 3068 struct btrfs_device *extent_dev;
80cd926eefca522 Qu Wenruo 2022-02-18 3069 struct btrfs_extent_item *ei;
80cd926eefca522 Qu Wenruo 2022-02-18 3070 struct extent_buffer *l;
80cd926eefca522 Qu Wenruo 2022-02-18 3071 int slot;
80cd926eefca522 Qu Wenruo 2022-02-18 3072 u64 mapped_length;
80cd926eefca522 Qu Wenruo 2022-02-18 3073 u64 extent_flags;
80cd926eefca522 Qu Wenruo 2022-02-18 3074 u64 extent_gen;
80cd926eefca522 Qu Wenruo 2022-02-18 3075 u64 extent_physical;
80cd926eefca522 Qu Wenruo 2022-02-18 3076 u64 extent_mirror_num;
5a6ac9eacb49143 Miao Xie 2014-11-06 3077
5a6ac9eacb49143 Miao Xie 2014-11-06 3078 l = path->nodes[0];
5a6ac9eacb49143 Miao Xie 2014-11-06 3079 slot = path->slots[0];
5a6ac9eacb49143 Miao Xie 2014-11-06 3080 if (slot >= btrfs_header_nritems(l)) {
80cd926eefca522 Qu Wenruo 2022-02-18 3081 ret = btrfs_next_leaf(extent_root, path);
5a6ac9eacb49143 Miao Xie 2014-11-06 3082 if (ret == 0)
5a6ac9eacb49143 Miao Xie 2014-11-06 3083 continue;
5a6ac9eacb49143 Miao Xie 2014-11-06 3084
80cd926eefca522 Qu Wenruo 2022-02-18 3085 /* No more extent items or error, exit */
5a6ac9eacb49143 Miao Xie 2014-11-06 3086 break;
5a6ac9eacb49143 Miao Xie 2014-11-06 3087 }
5a6ac9eacb49143 Miao Xie 2014-11-06 3088 btrfs_item_key_to_cpu(l, &key, slot);
5a6ac9eacb49143 Miao Xie 2014-11-06 3089
d7cad2389560f32 Zhao Lei 2015-07-22 3090 if (key.type != BTRFS_EXTENT_ITEM_KEY &&
d7cad2389560f32 Zhao Lei 2015-07-22 3091 key.type != BTRFS_METADATA_ITEM_KEY)
d7cad2389560f32 Zhao Lei 2015-07-22 3092 goto next;
d7cad2389560f32 Zhao Lei 2015-07-22 3093
5a6ac9eacb49143 Miao Xie 2014-11-06 3094 if (key.type == BTRFS_METADATA_ITEM_KEY)
80cd926eefca522 Qu Wenruo 2022-02-18 3095 extent_size = fs_info->nodesize;
5a6ac9eacb49143 Miao Xie 2014-11-06 3096 else
80cd926eefca522 Qu Wenruo 2022-02-18 3097 extent_size = key.offset;
5a6ac9eacb49143 Miao Xie 2014-11-06 3098
80cd926eefca522 Qu Wenruo 2022-02-18 3099 if (key.objectid + extent_size <= logical)
5a6ac9eacb49143 Miao Xie 2014-11-06 3100 goto next;
5a6ac9eacb49143 Miao Xie 2014-11-06 3101
80cd926eefca522 Qu Wenruo 2022-02-18 3102 /* Beyond this data stripe */
80cd926eefca522 Qu Wenruo 2022-02-18 3103 if (key.objectid >= logical + map->stripe_len)
5a6ac9eacb49143 Miao Xie 2014-11-06 3104 break;
5a6ac9eacb49143 Miao Xie 2014-11-06 3105
80cd926eefca522 Qu Wenruo 2022-02-18 3106 ei = btrfs_item_ptr(l, slot, struct btrfs_extent_item);
80cd926eefca522 Qu Wenruo 2022-02-18 3107 extent_flags = btrfs_extent_flags(l, ei);
80cd926eefca522 Qu Wenruo 2022-02-18 3108 extent_gen = btrfs_extent_generation(l, ei);
5a6ac9eacb49143 Miao Xie 2014-11-06 3109
80cd926eefca522 Qu Wenruo 2022-02-18 3110 if ((extent_flags & BTRFS_EXTENT_FLAG_TREE_BLOCK) &&
80cd926eefca522 Qu Wenruo 2022-02-18 3111 (key.objectid < logical || key.objectid + extent_size >
80cd926eefca522 Qu Wenruo 2022-02-18 3112 logical + map->stripe_len)) {
5d163e0e68ce743 Jeff Mahoney 2016-09-20 3113 btrfs_err(fs_info,
5d163e0e68ce743 Jeff Mahoney 2016-09-20 3114 "scrub: tree block %llu spanning stripes, ignored. logical=%llu",
80cd926eefca522 Qu Wenruo 2022-02-18 3115 key.objectid, logical);
9799d2c32bef6fb Zhao Lei 2015-08-25 3116 spin_lock(&sctx->stat_lock);
9799d2c32bef6fb Zhao Lei 2015-08-25 3117 sctx->stat.uncorrectable_errors++;
9799d2c32bef6fb Zhao Lei 2015-08-25 3118 spin_unlock(&sctx->stat_lock);
5a6ac9eacb49143 Miao Xie 2014-11-06 3119 goto next;

This goto next is what triggers the uninitialized variable warnings for
extent_start and extent_size.

But for that "goto next" we just go next slot, not relying on @extent_start.


5a6ac9eacb49143 Miao Xie 2014-11-06 3120 }
5a6ac9eacb49143 Miao Xie 2014-11-06 3121
80cd926eefca522 Qu Wenruo 2022-02-18 3122 extent_start = key.objectid;
80cd926eefca522 Qu Wenruo 2022-02-18 3123 ASSERT(extent_size <= U32_MAX);
5a6ac9eacb49143 Miao Xie 2014-11-06 3124
80cd926eefca522 Qu Wenruo 2022-02-18 3125 /* Truncate the range inside the data stripe */
80cd926eefca522 Qu Wenruo 2022-02-18 3126 if (extent_start < logical) {
80cd926eefca522 Qu Wenruo 2022-02-18 3127 extent_size -= logical - extent_start;
80cd926eefca522 Qu Wenruo 2022-02-18 3128 extent_start = logical;
80cd926eefca522 Qu Wenruo 2022-02-18 3129 }
80cd926eefca522 Qu Wenruo 2022-02-18 3130 if (extent_start + extent_size > logical + map->stripe_len)
80cd926eefca522 Qu Wenruo 2022-02-18 3131 extent_size = logical + map->stripe_len - extent_start;

And from now on, both @extent_start and @extent_size are initialized, we're safe to break/goto next.

Or did I miss something between?

Thanks,
Qu

5a6ac9eacb49143 Miao Xie 2014-11-06 3132
80cd926eefca522 Qu Wenruo 2022-02-18 3133 scrub_parity_mark_sectors_data(sparity, extent_start, extent_size);
5a6ac9eacb49143 Miao Xie 2014-11-06 3134
80cd926eefca522 Qu Wenruo 2022-02-18 3135 mapped_length = extent_size;
80cd926eefca522 Qu Wenruo 2022-02-18 3136 ret = btrfs_map_block(fs_info, BTRFS_MAP_READ, extent_start,
80cd926eefca522 Qu Wenruo 2022-02-18 3137 &mapped_length, &bioc, 0);
80cd926eefca522 Qu Wenruo 2022-02-18 3138 if (!ret && (!bioc || mapped_length < extent_size))
4a770891d9ddf94 Omar Sandoval 2015-06-19 3139 ret = -EIO;
4a770891d9ddf94 Omar Sandoval 2015-06-19 3140 if (ret) {
4c6646117912397 Qu Wenruo 2021-09-15 3141 btrfs_put_bioc(bioc);
80cd926eefca522 Qu Wenruo 2022-02-18 3142 scrub_parity_mark_sectors_error(sparity, extent_start,
80cd926eefca522 Qu Wenruo 2022-02-18 3143 extent_size);
80cd926eefca522 Qu Wenruo 2022-02-18 3144 break;
4a770891d9ddf94 Omar Sandoval 2015-06-19 3145 }
4c6646117912397 Qu Wenruo 2021-09-15 3146 extent_physical = bioc->stripes[0].physical;
4c6646117912397 Qu Wenruo 2021-09-15 3147 extent_mirror_num = bioc->mirror_num;
4c6646117912397 Qu Wenruo 2021-09-15 3148 extent_dev = bioc->stripes[0].dev;
4c6646117912397 Qu Wenruo 2021-09-15 3149 btrfs_put_bioc(bioc);
5a6ac9eacb49143 Miao Xie 2014-11-06 3150
80cd926eefca522 Qu Wenruo 2022-02-18 3151 ret = btrfs_lookup_csums_range(csum_root, extent_start,
80cd926eefca522 Qu Wenruo 2022-02-18 3152 extent_start + extent_size - 1,
5a6ac9eacb49143 Miao Xie 2014-11-06 3153 &sctx->csum_list, 1);
5a6ac9eacb49143 Miao Xie 2014-11-06 3154 if (ret)
80cd926eefca522 Qu Wenruo 2022-02-18 3155 break;
6fa96d72f79a155 Zhao Lei 2015-07-21 3156
80cd926eefca522 Qu Wenruo 2022-02-18 3157 ret = scrub_extent_for_parity(sparity, extent_start,
80cd926eefca522 Qu Wenruo 2022-02-18 3158 extent_size, extent_physical,
80cd926eefca522 Qu Wenruo 2022-02-18 3159 extent_dev, extent_flags,
80cd926eefca522 Qu Wenruo 2022-02-18 3160 extent_gen, extent_mirror_num);
6fa96d72f79a155 Zhao Lei 2015-07-21 3161 scrub_free_csums(sctx);
6fa96d72f79a155 Zhao Lei 2015-07-21 3162
5a6ac9eacb49143 Miao Xie 2014-11-06 3163 if (ret)
5a6ac9eacb49143 Miao Xie 2014-11-06 3164 break;
5a6ac9eacb49143 Miao Xie 2014-11-06 3165
5a6ac9eacb49143 Miao Xie 2014-11-06 3166 cond_resched();
5a6ac9eacb49143 Miao Xie 2014-11-06 3167 next:
5a6ac9eacb49143 Miao Xie 2014-11-06 3168 path->slots[0]++;
5a6ac9eacb49143 Miao Xie 2014-11-06 3169 }
80cd926eefca522 Qu Wenruo 2022-02-18 3170 if (ret < 0)
80cd926eefca522 Qu Wenruo 2022-02-18 @3171 scrub_parity_mark_sectors_error(sparity, extent_start,
80cd926eefca522 Qu Wenruo 2022-02-18 @3172 extent_size);
5a6ac9eacb49143 Miao Xie 2014-11-06 3173 btrfs_release_path(path);
80cd926eefca522 Qu Wenruo 2022-02-18 3174 return ret;
80cd926eefca522 Qu Wenruo 2022-02-18 3175 }
5a6ac9eacb49143 Miao Xie 2014-11-06 3176

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx