Commit dd0734f2 authored by Filipe Manana's avatar Filipe Manana Committed by David Sterba

btrfs: fix race between swap file activation and snapshot creation

When creating a snapshot we check if the current number of swap files, in
the root, is non-zero, and if it is, we error out and warn that we can not
create the snapshot because there are active swap files.

However this is racy because when a task started activation of a swap
file, another task might have started already snapshot creation and might
have seen the counter for the number of swap files as zero. This means
that after the swap file is activated we may end up with a snapshot of the
same root successfully created, and therefore when the first write to the
swap file happens it has to fall back into COW mode, which should never
happen for active swap files.

Basically what can happen is:

1) Task A starts snapshot creation and enters ioctl.c:create_snapshot().
   There it sees that root->nr_swapfiles has a value of 0 so it continues;

2) Task B enters btrfs_swap_activate(). It is not aware that another task
   started snapshot creation but it did not finish yet. It increments
   root->nr_swapfiles from 0 to 1;

3) Task B checks that the file meets all requirements to be an active
   swap file - it has NOCOW set, there are no snapshots for the inode's
   root at the moment, no file holes, no reflinked extents, etc;

4) Task B returns success and now the file is an active swap file;

5) Task A commits the transaction to create the snapshot and finishes.
   The swap file's extents are now shared between the original root and
   the snapshot;

6) A write into an extent of the swap file is attempted - there is a
   snapshot of the file's root, so we fall back to COW mode and therefore
   the physical location of the extent changes on disk.

So fix this by taking the snapshot lock during swap file activation before
locking the extent range, as that is the order in which we lock these
during buffered writes.

Fixes: ed46ff3d ("Btrfs: support swap files")
CC: stable@vger.kernel.org # 5.4+
Reviewed-by: default avatarAnand Jain <anand.jain@oracle.com>
Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent 195a49ea
...@@ -10298,7 +10298,8 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file, ...@@ -10298,7 +10298,8 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
sector_t *span) sector_t *span)
{ {
struct inode *inode = file_inode(file); struct inode *inode = file_inode(file);
struct btrfs_fs_info *fs_info = BTRFS_I(inode)->root->fs_info; struct btrfs_root *root = BTRFS_I(inode)->root;
struct btrfs_fs_info *fs_info = root->fs_info;
struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree; struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
struct extent_state *cached_state = NULL; struct extent_state *cached_state = NULL;
struct extent_map *em = NULL; struct extent_map *em = NULL;
...@@ -10349,13 +10350,27 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file, ...@@ -10349,13 +10350,27 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
"cannot activate swapfile while exclusive operation is running"); "cannot activate swapfile while exclusive operation is running");
return -EBUSY; return -EBUSY;
} }
/*
* Prevent snapshot creation while we are activating the swap file.
* We do not want to race with snapshot creation. If snapshot creation
* already started before we bumped nr_swapfiles from 0 to 1 and
* completes before the first write into the swap file after it is
* activated, than that write would fallback to COW.
*/
if (!btrfs_drew_try_write_lock(&root->snapshot_lock)) {
btrfs_exclop_finish(fs_info);
btrfs_warn(fs_info,
"cannot activate swapfile because snapshot creation is in progress");
return -EINVAL;
}
/* /*
* Snapshots can create extents which require COW even if NODATACOW is * Snapshots can create extents which require COW even if NODATACOW is
* set. We use this counter to prevent snapshots. We must increment it * set. We use this counter to prevent snapshots. We must increment it
* before walking the extents because we don't want a concurrent * before walking the extents because we don't want a concurrent
* snapshot to run after we've already checked the extents. * snapshot to run after we've already checked the extents.
*/ */
atomic_inc(&BTRFS_I(inode)->root->nr_swapfiles); atomic_inc(&root->nr_swapfiles);
isize = ALIGN_DOWN(inode->i_size, fs_info->sectorsize); isize = ALIGN_DOWN(inode->i_size, fs_info->sectorsize);
...@@ -10501,6 +10516,8 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file, ...@@ -10501,6 +10516,8 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
if (ret) if (ret)
btrfs_swap_deactivate(file); btrfs_swap_deactivate(file);
btrfs_drew_write_unlock(&root->snapshot_lock);
btrfs_exclop_finish(fs_info); btrfs_exclop_finish(fs_info);
if (ret) if (ret)
......
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