Commit 2c2ed5aa authored by Mark Fasheh's avatar Mark Fasheh Committed by Chris Mason
Browse files

btrfs: clear 'ret' in btrfs_check_shared() loop 



btrfs_check_shared() is leaking a return value of '1' from
find_parent_nodes(). As a result, callers (in this case, extent_fiemap())
are told extents are shared when they are not. This in turn broke fiemap on
btrfs for kernels v3.18 and up.

The fix is simple - we just have to clear 'ret' after we are done processing
the results of find_parent_nodes().

It wasn't clear to me at first what was happening with return values in
btrfs_check_shared() and find_parent_nodes() - thanks to Josef for the help
on irc. I added documentation to both functions to make things more clear
for the next hacker who might come across them.

If we could queue this up for -stable too that would be great.

Signed-off-by: default avatarMark Fasheh <mfasheh@suse.de>
Reviewed-by: default avatarJosef Bacik <jbacik@fb.com>
Signed-off-by: default avatarChris Mason <clm@fb.com>
parent 062c19e9

fs/btrfs/backref.c

+17 −0
Original line number Original line Diff line number Diff line
@@ -880,6 +880,8 @@ static int __add_keyed_refs(struct btrfs_fs_info *fs_info, 
 * indirect refs to their parent bytenr.

 * indirect refs to their parent bytenr.

 * When roots are found, they're added to the roots list

 * When roots are found, they're added to the roots list

 *

 *

 * NOTE: This can return values > 0

 *

 * FIXME some caching might speed things up

 * FIXME some caching might speed things up

 */

 */

static int find_parent_nodes(struct btrfs_trans_handle *trans,

static int find_parent_nodes(struct btrfs_trans_handle *trans,
@@ -1198,6 +1200,19 @@ int btrfs_find_all_roots(struct btrfs_trans_handle *trans, 
	return ret;

	return ret;

}

}





/**

 * btrfs_check_shared - tell us whether an extent is shared

 *

 * @trans: optional trans handle

 *

 * btrfs_check_shared uses the backref walking code but will short

 * circuit as soon as it finds a root or inode that doesn't match the

 * one passed in. This provides a significant performance benefit for

 * callers (such as fiemap) which want to know whether the extent is

 * shared but do not need a ref count.

 *

 * Return: 0 if extent is not shared, 1 if it is shared, < 0 on error.

 */

int btrfs_check_shared(struct btrfs_trans_handle *trans,

int btrfs_check_shared(struct btrfs_trans_handle *trans,

		       struct btrfs_fs_info *fs_info, u64 root_objectid,

		       struct btrfs_fs_info *fs_info, u64 root_objectid,

		       u64 inum, u64 bytenr)

		       u64 inum, u64 bytenr)
@@ -1226,11 +1241,13 @@ int btrfs_check_shared(struct btrfs_trans_handle *trans, 
		ret = find_parent_nodes(trans, fs_info, bytenr, elem.seq, tmp,

		ret = find_parent_nodes(trans, fs_info, bytenr, elem.seq, tmp,

					roots, NULL, root_objectid, inum);

					roots, NULL, root_objectid, inum);

		if (ret == BACKREF_FOUND_SHARED) {

		if (ret == BACKREF_FOUND_SHARED) {

			/* this is the only condition under which we return 1 */

			ret = 1;

			ret = 1;

			break;

			break;

		}

		}

		if (ret < 0 && ret != -ENOENT)

		if (ret < 0 && ret != -ENOENT)

			break;

			break;

		ret = 0;

		node = ulist_next(tmp, &uiter);

		node = ulist_next(tmp, &uiter);

		if (!node)

		if (!node)

			break;

			break;

CRA Git | Maintained and supported by SUSTech CRA and CCSE