History

btrfs: qgroup: validate btrfs_qgroup_inherit parameter

[BUG] Currently btrfs can create subvolume with an invalid qgroup inherit without triggering any error: # mkfs.btrfs -O quota -f $dev # mount $dev $mnt # btrfs subvolume create -i 2/0 $mnt/subv1 # btrfs qgroup show -prce --sync $mnt Qgroupid Referenced Exclusive Path -------- ---------- --------- ---- 0/5 16.00KiB 16.00KiB <toplevel> 0/256 16.00KiB 16.00KiB subv1 [CAUSE] We only do a very basic size check for btrfs_qgroup_inherit structure, but never really verify if the values are correct. Thus in btrfs_qgroup_inherit() function, we have to skip non-existing qgroups, and never return any error. [FIX] Fix the behavior and introduce extra checks: - Introduce early check for btrfs_qgroup_inherit structure Not only the size, but also all the qgroup ids would be verified. And the timing is very early, so we can return error early. This early check is very important for snapshot creation, as snapshot is delayed to transaction commit. - Drop support for btrfs_qgroup_inherit::num_ref_copies and num_excl_copies Those two members are used to specify to copy refr/excl numbers from other qgroups. This would definitely mark qgroup inconsistent, and btrfs-progs has dropped the support for them for a long time. It's time to drop the support for kernel. - Verify the supported btrfs_qgroup_inherit::flags Just in case we want to add extra flags for btrfs_qgroup_inherit. Now above subvolume creation would fail with -ENOENT other than silently ignore the non-existing qgroup.

Qu Wenruo committed 2 years ago

Commit 86211ee1 parent 07823034 files · +58 −13Browse files

Unified · Split

4 files
  1. fs/btrfs/ioctl.c +3 −13
  2. fs/btrfs/qgroup.c +51 −0
  3. fs/btrfs/qgroup.h +3 −0
  4. include/uapi/linux/btrfs.h +1 −0
fs/btrfs/ioctl.c+3−13
@@ -1382,7 +1382,7 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file, · 1381 unmodified lines
1382 if (vol_args->flags & BTRFS_SUBVOL_RDONLY)1382 if (vol_args->flags & BTRFS_SUBVOL_RDONLY)
1383 readonly = true;1383 readonly = true;
1384 if (vol_args->flags & BTRFS_SUBVOL_QGROUP_INHERIT) {1384 if (vol_args->flags & BTRFS_SUBVOL_QGROUP_INHERIT) {
1385 u64 nums;1385 struct btrfs_fs_info *fs_info = inode_to_fs_info(file_inode(file));
1386 1386
1387 if (vol_args->size < sizeof(*inherit) ||1387 if (vol_args->size < sizeof(*inherit) ||
1388 vol_args->size > PAGE_SIZE) {1388 vol_args->size > PAGE_SIZE) {
@@ -1395,19 +1395,9 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file, · 6 unmodified lines
1395 goto free_args;1395 goto free_args;
1396 }1396 }
1397 1397
1398 if (inherit->num_qgroups > PAGE_SIZE ||1398 ret = btrfs_qgroup_check_inherit(fs_info, inherit, vol_args->size);
1399 inherit->num_ref_copies > PAGE_SIZE ||1399 if (ret < 0)
1400 inherit->num_excl_copies > PAGE_SIZE) {
1401 ret = -EINVAL;
1402 goto free_inherit;
1403 }
1404
1405 nums = inherit->num_qgroups + 2 * inherit->num_ref_copies +
1406 2 * inherit->num_excl_copies;
1407 if (vol_args->size != struct_size(inherit, qgroups, nums)) {
1408 ret = -EINVAL;
1409 goto free_inherit;1400 goto free_inherit;
1410 }
1411 }1401 }
1412 1402
1413 ret = __btrfs_ioctl_snap_create(file, file_mnt_idmap(file),1403 ret = __btrfs_ioctl_snap_create(file, file_mnt_idmap(file),
fs/btrfs/qgroup.c+51−0
@@ -3046,6 +3046,57 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans) · 3045 unmodified lines
3046 return ret;3046 return ret;
3047}3047}
3048 3048
3049int btrfs_qgroup_check_inherit(struct btrfs_fs_info *fs_info,
3050 struct btrfs_qgroup_inherit *inherit,
3051 size_t size)
3052{
3053 if (inherit->flags & ~BTRFS_QGROUP_INHERIT_FLAGS_SUPP)
3054 return -EOPNOTSUPP;
3055 if (size < sizeof(*inherit) || size > PAGE_SIZE)
3056 return -EINVAL;
3057
3058 /*
3059 * In the past we allowed btrfs_qgroup_inherit to specify to copy
3060 * rfer/excl numbers directly from other qgroups. This behavior has
3061 * been disabled in userspace for a very long time, but here we should
3062 * also disable it in kernel, as this behavior is known to mark qgroup
3063 * inconsistent, and a rescan would wipe out the changes anyway.
3064 *
3065 * Reject any btrfs_qgroup_inherit with num_ref_copies or num_excl_copies.
3066 */
3067 if (inherit->num_ref_copies > 0 || inherit->num_excl_copies > 0)
3068 return -EINVAL;
3069
3070 if (inherit->num_qgroups > PAGE_SIZE)
3071 return -EINVAL;
3072
3073 if (size != struct_size(inherit, qgroups, inherit->num_qgroups))
3074 return -EINVAL;
3075
3076 /*
3077 * Now check all the remaining qgroups, they should all:
3078 *
3079 * - Exist
3080 * - Be higher level qgroups.
3081 */
3082 for (int i = 0; i < inherit->num_qgroups; i++) {
3083 struct btrfs_qgroup *qgroup;
3084 u64 qgroupid = inherit->qgroups[i];
3085
3086 if (btrfs_qgroup_level(qgroupid) == 0)
3087 return -EINVAL;
3088
3089 spin_lock(&fs_info->qgroup_lock);
3090 qgroup = find_qgroup_rb(fs_info, qgroupid);
3091 if (!qgroup) {
3092 spin_unlock(&fs_info->qgroup_lock);
3093 return -ENOENT;
3094 }
3095 spin_unlock(&fs_info->qgroup_lock);
3096 }
3097 return 0;
3098}
3099
3049static int qgroup_auto_inherit(struct btrfs_fs_info *fs_info,3100static int qgroup_auto_inherit(struct btrfs_fs_info *fs_info,
3050 u64 inode_rootid,3101 u64 inode_rootid,
3051 struct btrfs_qgroup_inherit **inherit)3102 struct btrfs_qgroup_inherit **inherit)
fs/btrfs/qgroup.h+3−0
@@ -350,6 +350,9 @@ int btrfs_qgroup_account_extent(struct btrfs_trans_handle *trans, u64 bytenr, · 349 unmodified lines
350 struct ulist *new_roots);350 struct ulist *new_roots);
351int btrfs_qgroup_account_extents(struct btrfs_trans_handle *trans);351int btrfs_qgroup_account_extents(struct btrfs_trans_handle *trans);
352int btrfs_run_qgroups(struct btrfs_trans_handle *trans);352int btrfs_run_qgroups(struct btrfs_trans_handle *trans);
353int btrfs_qgroup_check_inherit(struct btrfs_fs_info *fs_info,
354 struct btrfs_qgroup_inherit *inherit,
355 size_t size);
353int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,356int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,
354 u64 objectid, u64 inode_rootid,357 u64 objectid, u64 inode_rootid,
355 struct btrfs_qgroup_inherit *inherit);358 struct btrfs_qgroup_inherit *inherit);
include/uapi/linux/btrfs.h+1−0
@@ -92,6 +92,7 @@ struct btrfs_qgroup_limit { · 91 unmodified lines
92 * struct btrfs_qgroup_inherit.flags92 * struct btrfs_qgroup_inherit.flags
93 */93 */
94#define BTRFS_QGROUP_INHERIT_SET_LIMITS (1ULL << 0)94#define BTRFS_QGROUP_INHERIT_SET_LIMITS (1ULL << 0)
95#define BTRFS_QGROUP_INHERIT_FLAGS_SUPP (BTRFS_QGROUP_INHERIT_SET_LIMITS)
95 96
96struct btrfs_qgroup_inherit {97struct btrfs_qgroup_inherit {
97 __u64 flags;98 __u64 flags;