btrfs: fix anon_dev leak in create_subvol()
authorOmar Sandoval <osandov@fb.com>
Thu, 10 Mar 2022 01:31:33 +0000 (17:31 -0800)
committerDavid Sterba <dsterba@suse.com>
Mon, 16 May 2022 15:03:06 +0000 (17:03 +0200)
When btrfs_qgroup_inherit(), btrfs_alloc_tree_block, or
btrfs_insert_root() fail in create_subvol(), we return without freeing
anon_dev. Reorganize the error handling in create_subvol() to fix this.

Reviewed-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
Signed-off-by: Omar Sandoval <osandov@fb.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/ioctl.c

index be6c245..7778019 100644 (file)
@@ -561,7 +561,7 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
        struct timespec64 cur_time = current_time(dir);
        struct inode *inode;
        int ret;
-       dev_t anon_dev = 0;
+       dev_t anon_dev;
        u64 objectid;
        u64 index = 0;
 
@@ -571,11 +571,7 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
 
        ret = btrfs_get_free_objectid(fs_info->tree_root, &objectid);
        if (ret)
-               goto fail_free;
-
-       ret = get_anon_bdev(&anon_dev);
-       if (ret < 0)
-               goto fail_free;
+               goto out_root_item;
 
        /*
         * Don't create subvolume whose level is not zero. Or qgroup will be
@@ -583,9 +579,13 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
         */
        if (btrfs_qgroup_level(objectid)) {
                ret = -ENOSPC;
-               goto fail_free;
+               goto out_root_item;
        }
 
+       ret = get_anon_bdev(&anon_dev);
+       if (ret < 0)
+               goto out_root_item;
+
        btrfs_init_block_rsv(&block_rsv, BTRFS_BLOCK_RSV_TEMP);
        /*
         * The same as the snapshot creation, please see the comment
@@ -593,26 +593,26 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
         */
        ret = btrfs_subvolume_reserve_metadata(root, &block_rsv, 8, false);
        if (ret)
-               goto fail_free;
+               goto out_anon_dev;
 
        trans = btrfs_start_transaction(root, 0);
        if (IS_ERR(trans)) {
                ret = PTR_ERR(trans);
                btrfs_subvolume_release_metadata(root, &block_rsv);
-               goto fail_free;
+               goto out_anon_dev;
        }
        trans->block_rsv = &block_rsv;
        trans->bytes_reserved = block_rsv.size;
 
        ret = btrfs_qgroup_inherit(trans, 0, objectid, inherit);
        if (ret)
-               goto fail;
+               goto out;
 
        leaf = btrfs_alloc_tree_block(trans, root, 0, objectid, NULL, 0, 0, 0,
                                      BTRFS_NESTING_NORMAL);
        if (IS_ERR(leaf)) {
                ret = PTR_ERR(leaf);
-               goto fail;
+               goto out;
        }
 
        btrfs_mark_buffer_dirty(leaf);
@@ -667,7 +667,7 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
                btrfs_tree_unlock(leaf);
                btrfs_free_tree_block(trans, objectid, leaf, 0, 1);
                free_extent_buffer(leaf);
-               goto fail;
+               goto out;
        }
 
        free_extent_buffer(leaf);
@@ -676,19 +676,18 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
        key.offset = (u64)-1;
        new_root = btrfs_get_new_fs_root(fs_info, objectid, anon_dev);
        if (IS_ERR(new_root)) {
-               free_anon_bdev(anon_dev);
                ret = PTR_ERR(new_root);
                btrfs_abort_transaction(trans, ret);
-               goto fail;
+               goto out;
        }
-       /* Freeing will be done in btrfs_put_root() of new_root */
+       /* anon_dev is owned by new_root now. */
        anon_dev = 0;
 
        ret = btrfs_record_root_in_trans(trans, new_root);
        if (ret) {
                btrfs_put_root(new_root);
                btrfs_abort_transaction(trans, ret);
-               goto fail;
+               goto out;
        }
 
        ret = btrfs_create_subvol_root(trans, new_root, root, mnt_userns);
@@ -696,7 +695,7 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
        if (ret) {
                /* We potentially lose an unused inode item here */
                btrfs_abort_transaction(trans, ret);
-               goto fail;
+               goto out;
        }
 
        /*
@@ -705,28 +704,28 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
        ret = btrfs_set_inode_index(BTRFS_I(dir), &index);
        if (ret) {
                btrfs_abort_transaction(trans, ret);
-               goto fail;
+               goto out;
        }
 
        ret = btrfs_insert_dir_item(trans, name, namelen, BTRFS_I(dir), &key,
                                    BTRFS_FT_DIR, index);
        if (ret) {
                btrfs_abort_transaction(trans, ret);
-               goto fail;
+               goto out;
        }
 
        btrfs_i_size_write(BTRFS_I(dir), dir->i_size + namelen * 2);
        ret = btrfs_update_inode(trans, root, BTRFS_I(dir));
        if (ret) {
                btrfs_abort_transaction(trans, ret);
-               goto fail;
+               goto out;
        }
 
        ret = btrfs_add_root_ref(trans, objectid, root->root_key.objectid,
                                 btrfs_ino(BTRFS_I(dir)), index, name, namelen);
        if (ret) {
                btrfs_abort_transaction(trans, ret);
-               goto fail;
+               goto out;
        }
 
        ret = btrfs_uuid_tree_add(trans, root_item->uuid,
@@ -734,8 +733,7 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
        if (ret)
                btrfs_abort_transaction(trans, ret);
 
-fail:
-       kfree(root_item);
+out:
        trans->block_rsv = NULL;
        trans->bytes_reserved = 0;
        btrfs_subvolume_release_metadata(root, &block_rsv);
@@ -751,11 +749,10 @@ fail:
                        return PTR_ERR(inode);
                d_instantiate(dentry, inode);
        }
-       return ret;
-
-fail_free:
+out_anon_dev:
        if (anon_dev)
                free_anon_bdev(anon_dev);
+out_root_item:
        kfree(root_item);
        return ret;
 }