[PATCH] btrfs: fix metadata extent leak after failure to create subvolume

[PATCH] btrfs: fix metadata extent leak after failure to create subvolume

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

When creating a subvolume we allocate an extent buffer for its root node
after starting a transaction. We setup a root item for the subvolume that
points to that extent buffer and then attempt to insert the root item into
the root tree - however if that fails, due to -ENOMEM for example, we do
not free the extent buffer previously allocated and we do not abort the
transaction (as at that point we did nothing that can not be undone).

This means that we effectively do not return the metadata extent back to
the free space cache/tree and we leave a delayed reference for it which
causes a metadata extent item to be added to the extent tree, in the next
transaction commit, without having backreferences. When this happens
'btrfs check' reports the following:

  $ btrfs check /dev/sdi
  Opening filesystem to check...
  Checking filesystem on /dev/sdi
  UUID: dce2cb9d-025f-4b05-a4bf-cee0ad3785eb
  [1/7] checking root items
  [2/7] checking extents
  ref mismatch on [30425088 16384] extent item 1, found 0
  backref 30425088 root 256 not referenced back 0x564a91c23d70
  incorrect global backref count on 30425088 found 1 wanted 0
  backpointer mismatch on [30425088 16384]
  owner ref check failed [30425088 16384]
  ERROR: errors found in extent allocation tree or chunk allocation
  [3/7] checking free space cache
  [4/7] checking fs roots
  [5/7] checking only csums items (without verifying data)
  [6/7] checking root refs
  [7/7] checking quota groups skipped (not enabled on this FS)
  found 212992 bytes used, error(s) found
  total csum bytes: 0
  total tree bytes: 131072
  total fs tree bytes: 32768
  total extent tree bytes: 16384
  btree space waste bytes: 124669
  file data blocks allocated: 65536
   referenced 65536

So fix this by freeing the metadata extent if btrfs_insert_root() returns
an error.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ioctl.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 3415a9f06c81..e3927bc114b0 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -697,8 +697,6 @@ static noinline int create_subvol(struct inode *dir,
 	btrfs_set_root_otransid(root_item, trans->transid);
 
 	btrfs_tree_unlock(leaf);
-	free_extent_buffer(leaf);
-	leaf = NULL;
 
 	btrfs_set_root_dirid(root_item, BTRFS_FIRST_FREE_OBJECTID);
 
@@ -707,8 +705,22 @@ static noinline int create_subvol(struct inode *dir,
 	key.type = BTRFS_ROOT_ITEM_KEY;
 	ret = btrfs_insert_root(trans, fs_info->tree_root, &key,
 				root_item);
-	if (ret)
+	if (ret) {
+		/*
+		 * Since we don't abort the transaction in this case, free the
+		 * tree block so that we don't leak space and leave the filesytem
+		 * in an inconsistent state (an extent item in the extent tree
+		 * without backreferences). Also no need to have the tree block
+		 * locked since it is not in any tree at this point, so no other
+		 * task can find it and use it.
+		 */
+		btrfs_free_tree_block(trans, root, leaf, 0, 1);
+		free_extent_buffer(leaf);
 		goto fail;
+	}
+
+	free_extent_buffer(leaf);
+	leaf = NULL;
 
 	key.offset = (u64)-1;
 	new_root = btrfs_get_new_fs_root(fs_info, objectid, anon_dev);
-- 
2.28.0

Re: [PATCH] btrfs: fix metadata extent leak after failure to create subvolume

[David Sterba]

On Tue, Apr 20, 2021 at 10:55:12AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When creating a subvolume we allocate an extent buffer for its root node
> after starting a transaction. We setup a root item for the subvolume that
> points to that extent buffer and then attempt to insert the root item into
> the root tree - however if that fails, due to -ENOMEM for example, we do
> not free the extent buffer previously allocated and we do not abort the
> transaction (as at that point we did nothing that can not be undone).
> 
> This means that we effectively do not return the metadata extent back to
> the free space cache/tree and we leave a delayed reference for it which
> causes a metadata extent item to be added to the extent tree, in the next
> transaction commit, without having backreferences. When this happens
> 'btrfs check' reports the following:
> 
>   $ btrfs check /dev/sdi
>   Opening filesystem to check...
>   Checking filesystem on /dev/sdi
>   UUID: dce2cb9d-025f-4b05-a4bf-cee0ad3785eb
>   [1/7] checking root items
>   [2/7] checking extents
>   ref mismatch on [30425088 16384] extent item 1, found 0
>   backref 30425088 root 256 not referenced back 0x564a91c23d70
>   incorrect global backref count on 30425088 found 1 wanted 0
>   backpointer mismatch on [30425088 16384]
>   owner ref check failed [30425088 16384]
>   ERROR: errors found in extent allocation tree or chunk allocation
>   [3/7] checking free space cache
>   [4/7] checking fs roots
>   [5/7] checking only csums items (without verifying data)
>   [6/7] checking root refs
>   [7/7] checking quota groups skipped (not enabled on this FS)
>   found 212992 bytes used, error(s) found
>   total csum bytes: 0
>   total tree bytes: 131072
>   total fs tree bytes: 32768
>   total extent tree bytes: 16384
>   btree space waste bytes: 124669
>   file data blocks allocated: 65536
>    referenced 65536
> 
> So fix this by freeing the metadata extent if btrfs_insert_root() returns
> an error.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Added to misc-next, thanks.