Commit 92876eec authored by Filipe Manana's avatar Filipe Manana Committed by David Sterba

btrfs: fix inode list leak during backref walking at find_parent_nodes()

During backref walking, at find_parent_nodes(), if we are dealing with a
data extent and we get an error while resolving the indirect backrefs, at
resolve_indirect_refs(), or in the while loop that iterates over the refs
in the direct refs rbtree, we end up leaking the inode lists attached to
the direct refs we have in the direct refs rbtree that were not yet added
to the refs ulist passed as argument to find_parent_nodes(). Since they
were not yet added to the refs ulist and prelim_release() does not free
the lists, on error the caller can only free the lists attached to the
refs that were added to the refs ulist, all the remaining refs get their
inode lists never freed, therefore leaking their memory.

Fix this by having prelim_release() always free any attached inode list
to each ref found in the rbtree, and have find_parent_nodes() set the
ref's inode list to NULL once it transfers ownership of the inode list
to a ref added to the refs ulist passed to find_parent_nodes().

Fixes: 86d5f994 ("btrfs: convert prelimary reference tracking to use rbtrees")
Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent 5614dc3a
...@@ -289,8 +289,10 @@ static void prelim_release(struct preftree *preftree) ...@@ -289,8 +289,10 @@ static void prelim_release(struct preftree *preftree)
struct prelim_ref *ref, *next_ref; struct prelim_ref *ref, *next_ref;
rbtree_postorder_for_each_entry_safe(ref, next_ref, rbtree_postorder_for_each_entry_safe(ref, next_ref,
&preftree->root.rb_root, rbnode) &preftree->root.rb_root, rbnode) {
free_inode_elem_list(ref->inode_list);
free_pref(ref); free_pref(ref);
}
preftree->root = RB_ROOT_CACHED; preftree->root = RB_ROOT_CACHED;
preftree->count = 0; preftree->count = 0;
...@@ -1384,6 +1386,12 @@ static int find_parent_nodes(struct btrfs_trans_handle *trans, ...@@ -1384,6 +1386,12 @@ static int find_parent_nodes(struct btrfs_trans_handle *trans,
if (ret < 0) if (ret < 0)
goto out; goto out;
ref->inode_list = eie; ref->inode_list = eie;
/*
* We transferred the list ownership to the ref,
* so set to NULL to avoid a double free in case
* an error happens after this.
*/
eie = NULL;
} }
ret = ulist_add_merge_ptr(refs, ref->parent, ret = ulist_add_merge_ptr(refs, ref->parent,
ref->inode_list, ref->inode_list,
...@@ -1409,6 +1417,14 @@ static int find_parent_nodes(struct btrfs_trans_handle *trans, ...@@ -1409,6 +1417,14 @@ static int find_parent_nodes(struct btrfs_trans_handle *trans,
eie->next = ref->inode_list; eie->next = ref->inode_list;
} }
eie = NULL; eie = NULL;
/*
* We have transferred the inode list ownership from
* this ref to the ref we added to the 'refs' ulist.
* So set this ref's inode list to NULL to avoid
* use-after-free when our caller uses it or double
* frees in case an error happens before we return.
*/
ref->inode_list = NULL;
} }
cond_resched(); cond_resched();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment