btrfs: handle tree backref walk error properly

[BUG]
Smatch reports the following errors related to commit ("btrfs: output
affected files when relocation fails"):

	fs/btrfs/inode.c:283 print_data_reloc_error()
	error: uninitialized symbol 'ref_level'.

[CAUSE]
That part of code is mostly copied from scrub, but unfortunately scrub
code from the beginning is not doing the error handling properly.

The offending code looks like this:

	do {
		ret = tree_backref_for_extent();
		btrfs_warn_rl();
	} while (ret != 1);

There are several problems involved:

- No error handling
  If that tree_backref_for_extent() failed, we would output the same
  error again and again, never really exit as it requires ret == 1 to
  exit.

- Always do one extra output
  As tree_backref_for_extent() only return > 0 if there is no more
  backref item.
  This means after the last item we hit, we would output an invalid
  error message for ret > 0 case.

[FIX]
Fix the old code by:

- Move @ref_root and @ref_level into the if branch
  And do not initialize them, so we can catch such uninitialized values
  just like what we do in the inode.c

- Explicitly check the return value of tree_backref_for_extent()
  And handle ret < 0 and ret > 0 cases properly.

- No more do {} while () loop
  Instead go while (true) {} loop since we will handle @ret manually.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index da7e5bf2f6db65096f88a7601ee10c9bb3d46226..333736712bd9ab4f1cfdaaae7a9aee48d31eddd5 100644 (file)
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -276,17 +276,25 @@ static void print_data_reloc_error(const struct btrfs_inode *inode, u64 file_off
                u64 ref_root;
                u8 ref_level;
 
-               do {
+               while (true) {
                        ret = tree_backref_for_extent(&ptr, eb, &found_key, ei,
                                                      item_size, &ref_root,
                                                      &ref_level);
+                       if (ret < 0) {
+                               btrfs_warn_rl(fs_info,
+                               "failed to resolve tree backref for logical %llu: %d",
+                                             logical, ret);
+                               break;
+                       }
+                       if (ret > 0)
+                               break;
+
                        btrfs_warn_rl(fs_info,
 "csum error at logical %llu mirror %u: metadata %s (level %d) in tree %llu",
                                logical, mirror_num,
                                (ref_level ? "node" : "leaf"),
-                               (ret < 0 ? -1 : ref_level),
-                               (ret < 0 ? -1 : ref_root));
-               } while (ret != 1);
+                               ref_level, ref_root);
+               }
                btrfs_release_path(&path);
        } else {
                struct btrfs_backref_walk_ctx ctx = { 0 };

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index f231883f504a548f3d349237eab130293253ab8b..265db0c15a4c9cecdc5be26062bfa1b88471659a 100644 (file)
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -479,11 +479,8 @@ static void scrub_print_common_warning(const char *errstr, struct btrfs_device *
        struct extent_buffer *eb;
        struct btrfs_extent_item *ei;
        struct scrub_warning swarn;
-       unsigned long ptr = 0;
        u64 flags = 0;
-       u64 ref_root;
        u32 item_size;
-       u8 ref_level = 0;
        int ret;
 
        /* Super block error, no need to search extent tree. */
@@ -513,19 +510,28 @@ static void scrub_print_common_warning(const char *errstr, struct btrfs_device *
        item_size = btrfs_item_size(eb, path->slots[0]);
 
        if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK) {
-               do {
+               unsigned long ptr = 0;
+               u8 ref_level;
+               u64 ref_root;
+
+               while (true) {
                        ret = tree_backref_for_extent(&ptr, eb, &found_key, ei,
                                                      item_size, &ref_root,
                                                      &ref_level);
+                       if (ret < 0) {
+                               btrfs_warn(fs_info,
+                               "failed to resolve tree backref for logical %llu: %d",
+                                                 swarn.logical, ret);
+                               break;
+                       }
+                       if (ret > 0)
+                               break;
                        btrfs_warn_in_rcu(fs_info,
 "%s at logical %llu on dev %s, physical %llu: metadata %s (level %d) in tree %llu",
-                               errstr, swarn.logical,
-                               btrfs_dev_name(dev),
-                               swarn.physical,
-                               ref_level ? "node" : "leaf",
-                               ret < 0 ? -1 : ref_level,
-                               ret < 0 ? -1 : ref_root);
-               } while (ret != 1);
+                               errstr, swarn.logical, btrfs_dev_name(dev),
+                               swarn.physical, (ref_level ? "node" : "leaf"),
+                               ref_level, ref_root);
+               }
                btrfs_release_path(path);
        } else {
                struct btrfs_backref_walk_ctx ctx = { 0 };