Commit b7f9945a authored by Qu Wenruo's avatar Qu Wenruo Committed by David Sterba
Browse files

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: default avatarDan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent f880fe6e

fs/btrfs/inode.c

+12 −4
Original line number Diff line number Diff line
@@ -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 };

fs/btrfs/scrub.c

+17 −11
Original line number Diff line number Diff line
@@ -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 };