diff mbox series

btrfs: fix qgroup id collision across mounts

Message ID f7e7c99aded1f18703dd19ec6b92cf3c7d635060.1715296042.git.boris@bur.io (mailing list archive)
State New
Headers show
Series btrfs: fix qgroup id collision across mounts | expand

Commit Message

Boris Burkov May 9, 2024, 11:07 p.m. UTC
If we delete subvolumes whose ID is the largest in the filesystem, then
unmount and mount again, then btrfs_init_root_free_objectid on the
tree_root will select a subvolid smaller than that one and thus allow
reusing it.

If we are also using qgroups (and particularly squotas) it is possible
to delete the subvol without deleting the qgroup. In that case, we will
be able to create a new subvol whose id already has a level 0 qgroup.
This will result in re-using that qgroup which would then lead to
incorrect accounting.

Fixes: 6ed05643ddb1 ("btrfs: create qgroup earlier in snapshot creation")
Signed-off-by: Boris Burkov <boris@bur.io>
---
 fs/btrfs/qgroup.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Qu Wenruo May 9, 2024, 11:13 p.m. UTC | #1
在 2024/5/10 08:37, Boris Burkov 写道:
> If we delete subvolumes whose ID is the largest in the filesystem, then
> unmount and mount again, then btrfs_init_root_free_objectid on the
> tree_root will select a subvolid smaller than that one and thus allow
> reusing it.
>
> If we are also using qgroups (and particularly squotas) it is possible
> to delete the subvol without deleting the qgroup. In that case, we will
> be able to create a new subvol whose id already has a level 0 qgroup.
> This will result in re-using that qgroup which would then lead to
> incorrect accounting.
>
> Fixes: 6ed05643ddb1 ("btrfs: create qgroup earlier in snapshot creation")
> Signed-off-by: Boris Burkov <boris@bur.io>

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

This also affects regular qgroup, e.g. the qgroup is already
inconsistent, then the delete subvolume would leave a non-empty qgroup.

Although it's less severe, since we need a rescan anyway.

But for squota it's really severe since there is no inconsistent status
at all.

Thanks,
Qu
> ---
>   fs/btrfs/qgroup.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 2cba6451d164..243995b95e63 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -468,6 +468,7 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
>   		}
>   		if (!qgroup) {
>   			struct btrfs_qgroup *prealloc;
> +			struct btrfs_root *tree_root = fs_info->tree_root;
>
>   			prealloc = kzalloc(sizeof(*prealloc), GFP_KERNEL);
>   			if (!prealloc) {
> @@ -475,6 +476,25 @@ int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
>   				goto out;
>   			}
>   			qgroup = add_qgroup_rb(fs_info, prealloc, found_key.offset);
> +			/*
> +			 * If a qgroup exists for a subvolume ID, it is possible
> +			 * that subvolume has been deleted, in which case
> +			 * re-using that ID would lead to incorrect accounting.
> +			 *
> +			 * Ensure that we skip any such subvol ids.
> +			 *
> +			 * We don't need to lock because this is only called
> +			 * during mount before we start doing things like creating
> +			 * subvolumes.
> +			 */
> +			if (is_fstree(qgroup->qgroupid) &&
> +			    qgroup->qgroupid > tree_root->free_objectid)
> +				/*
> +				 * Don't need to check against BTRFS_LAST_FREE_OBJECTID,
> +				 * as it will get checked on the next call to
> +				 * btrfs_get_free_objectid.
> +				 */
> +				tree_root->free_objectid = qgroup->qgroupid + 1;
>   		}
>   		ret = btrfs_sysfs_add_one_qgroup(fs_info, qgroup);
>   		if (ret < 0)
diff mbox series

Patch

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 2cba6451d164..243995b95e63 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -468,6 +468,7 @@  int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
 		}
 		if (!qgroup) {
 			struct btrfs_qgroup *prealloc;
+			struct btrfs_root *tree_root = fs_info->tree_root;
 
 			prealloc = kzalloc(sizeof(*prealloc), GFP_KERNEL);
 			if (!prealloc) {
@@ -475,6 +476,25 @@  int btrfs_read_qgroup_config(struct btrfs_fs_info *fs_info)
 				goto out;
 			}
 			qgroup = add_qgroup_rb(fs_info, prealloc, found_key.offset);
+			/*
+			 * If a qgroup exists for a subvolume ID, it is possible
+			 * that subvolume has been deleted, in which case
+			 * re-using that ID would lead to incorrect accounting.
+			 *
+			 * Ensure that we skip any such subvol ids.
+			 *
+			 * We don't need to lock because this is only called
+			 * during mount before we start doing things like creating
+			 * subvolumes.
+			 */
+			if (is_fstree(qgroup->qgroupid) &&
+			    qgroup->qgroupid > tree_root->free_objectid)
+				/*
+				 * Don't need to check against BTRFS_LAST_FREE_OBJECTID,
+				 * as it will get checked on the next call to
+				 * btrfs_get_free_objectid.
+				 */
+				tree_root->free_objectid = qgroup->qgroupid + 1;
 		}
 		ret = btrfs_sysfs_add_one_qgroup(fs_info, qgroup);
 		if (ret < 0)