Re: [PATCH v2] btrfs: prevent subvol with swapfile from being deleted

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2022/3/23 15:10, Kaiwen Hu wrote:
> This patch prevent subvol being deleted when the subvol contains
> any active swapfile.
>
> Since the subvolume is deleted, we cannot swapoff the swapfile in
> this deleted subvolume.  However, the swapfile is still active,
> we unable to unmount this volume.  Let it into some deadlock
> situation.
>
> The test looks like this:
> 	mkfs.btrfs -f $dev > /dev/null
> 	mount $dev $mnt
>
> 	btrfs sub create $mnt/subvol
> 	touch $mnt/subvol/swapfile
> 	chmod 600 $mnt/subvol/swapfile
> 	chattr +C $mnt/subvol/swapfile
> 	dd if=/dev/zero of=$mnt/subvol/swapfile bs=1K count=4096
> 	mkswap $mnt/subvol/swapfile
> 	swapon $mnt/subvol/swapfile
>
> 	btrfs sub delete $mnt/subvol
> 	swapoff $mnt/subvol/swapfile  // failed: No such file or directory
> 	swapoff --all
>
> 	unmount $mnt  // target is busy.
>
> To prevent above issue, we simply check that whether the subvolume
> contains any active swapfile, and stop the deleting process.  This
> behavior is like snapshot ioctl dealing with a swapfile.
>
> Reviewed-by: Robbie Ko <robbieko@synology.com>
> Signed-off-by: Kaiwen Hu <kevinhu@synology.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>
> Add comment on it.
>
>   fs/btrfs/inode.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 5bbea5ec31fc..b32def311f44 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4460,6 +4460,12 @@ int btrfs_delete_subvolume(struct inode *dir, struct dentry *dentry)
>   			   dest->root_key.objectid);
>   		return -EPERM;
>   	}
> +	if (atomic_read(&dest->nr_swapfiles)) {
> +		spin_unlock(&dest->root_item_lock);
> +		btrfs_warn(fs_info,
> +			   "attempt to delete subvolume with active swapfile");
> +		return -ETXTBSY;
> +	}
>   	root_flags = btrfs_root_flags(&dest->root_item);
>   	btrfs_set_root_flags(&dest->root_item,
>   			     root_flags | BTRFS_ROOT_SUBVOL_DEAD);
> @@ -10418,8 +10424,22 @@ static int btrfs_swap_activate(struct swap_info_struct *sis, struct file *file,
>   	 * set. We use this counter to prevent snapshots. We must increment it
>   	 * before walking the extents because we don't want a concurrent
>   	 * snapshot to run after we've already checked the extents.
> +	 *
> +	 * It is possible that subvolume is marked for deletion but still not
> +	 * remove yet. To prevent this race, we check the root's mark before
> +	 * activating swapfile.
>   	 */
> +	spin_lock(&root->root_item_lock);
> +	if (btrfs_root_dead(root)) {
> +		spin_unlock(&root->root_item_lock);
> +		btrfs_exclop_finish(fs_info);
> +		btrfs_warn(fs_info,
> +	   "cannot activate swapfile because subvolume is marked for deletion");
> +		return -EINVAL;
> +	}
>   	atomic_inc(&root->nr_swapfiles);
> +	spin_unlock(&root->root_item_lock);
> +
>
>   	isize = ALIGN_DOWN(inode->i_size, fs_info->sectorsize);
>