Skip to content

Commit 3324d05

Browse files
osandovkdave
authored andcommitted
btrfs: avoid copying BTRFS_ROOT_SUBVOL_DEAD flag to snapshot of subvolume being deleted
Sweet Tea spotted a race between subvolume deletion and snapshotting that can result in the root item for the snapshot having the BTRFS_ROOT_SUBVOL_DEAD flag set. The race is: Thread 1 | Thread 2 ----------------------------------------------|---------- btrfs_delete_subvolume | btrfs_set_root_flags(BTRFS_ROOT_SUBVOL_DEAD)| |btrfs_mksubvol | down_read(subvol_sem) | create_snapshot | ... | create_pending_snapshot | copy root item from source down_write(subvol_sem) | This flag is only checked in send and swap activate, which this would cause to fail mysteriously. create_snapshot() now checks the root refs to reject a deleted subvolume, so we can fix this by locking subvol_sem earlier so that the BTRFS_ROOT_SUBVOL_DEAD flag and the root refs are updated atomically. CC: stable@vger.kernel.org # 4.14+ Reported-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me> Reviewed-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me> Reviewed-by: Anand Jain <anand.jain@oracle.com> Signed-off-by: Omar Sandoval <osandov@fb.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
1 parent 7081929 commit 3324d05

File tree

1 file changed

+13
-9
lines changed

1 file changed

+13
-9
lines changed

fs/btrfs/inode.c

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4458,6 +4458,8 @@ int btrfs_delete_subvolume(struct btrfs_inode *dir, struct dentry *dentry)
44584458
u64 root_flags;
44594459
int ret;
44604460

4461+
down_write(&fs_info->subvol_sem);
4462+
44614463
/*
44624464
* Don't allow to delete a subvolume with send in progress. This is
44634465
* inside the inode lock so the error handling that has to drop the bit
@@ -4469,25 +4471,25 @@ int btrfs_delete_subvolume(struct btrfs_inode *dir, struct dentry *dentry)
44694471
btrfs_warn(fs_info,
44704472
"attempt to delete subvolume %llu during send",
44714473
dest->root_key.objectid);
4472-
return -EPERM;
4474+
ret = -EPERM;
4475+
goto out_up_write;
44734476
}
44744477
if (atomic_read(&dest->nr_swapfiles)) {
44754478
spin_unlock(&dest->root_item_lock);
44764479
btrfs_warn(fs_info,
44774480
"attempt to delete subvolume %llu with active swapfile",
44784481
root->root_key.objectid);
4479-
return -EPERM;
4482+
ret = -EPERM;
4483+
goto out_up_write;
44804484
}
44814485
root_flags = btrfs_root_flags(&dest->root_item);
44824486
btrfs_set_root_flags(&dest->root_item,
44834487
root_flags | BTRFS_ROOT_SUBVOL_DEAD);
44844488
spin_unlock(&dest->root_item_lock);
44854489

4486-
down_write(&fs_info->subvol_sem);
4487-
44884490
ret = may_destroy_subvol(dest);
44894491
if (ret)
4490-
goto out_up_write;
4492+
goto out_undead;
44914493

44924494
btrfs_init_block_rsv(&block_rsv, BTRFS_BLOCK_RSV_TEMP);
44934495
/*
@@ -4497,7 +4499,7 @@ int btrfs_delete_subvolume(struct btrfs_inode *dir, struct dentry *dentry)
44974499
*/
44984500
ret = btrfs_subvolume_reserve_metadata(root, &block_rsv, 5, true);
44994501
if (ret)
4500-
goto out_up_write;
4502+
goto out_undead;
45014503

45024504
trans = btrfs_start_transaction(root, 0);
45034505
if (IS_ERR(trans)) {
@@ -4563,15 +4565,17 @@ int btrfs_delete_subvolume(struct btrfs_inode *dir, struct dentry *dentry)
45634565
inode->i_flags |= S_DEAD;
45644566
out_release:
45654567
btrfs_subvolume_release_metadata(root, &block_rsv);
4566-
out_up_write:
4567-
up_write(&fs_info->subvol_sem);
4568+
out_undead:
45684569
if (ret) {
45694570
spin_lock(&dest->root_item_lock);
45704571
root_flags = btrfs_root_flags(&dest->root_item);
45714572
btrfs_set_root_flags(&dest->root_item,
45724573
root_flags & ~BTRFS_ROOT_SUBVOL_DEAD);
45734574
spin_unlock(&dest->root_item_lock);
4574-
} else {
4575+
}
4576+
out_up_write:
4577+
up_write(&fs_info->subvol_sem);
4578+
if (!ret) {
45754579
d_invalidate(dentry);
45764580
btrfs_prune_dentries(dest);
45774581
ASSERT(dest->send_in_progress == 0);

0 commit comments

Comments
 (0)