Skip to content

Commit 86211ee

Browse files
adam900710kdave
authored andcommitted
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. CC: [email protected] # 6.7+ Signed-off-by: Qu Wenruo <[email protected]> Reviewed-by: David Sterba <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 0782303 commit 86211ee

File tree

4 files changed

+58
-13
lines changed

4 files changed

+58
-13
lines changed

fs/btrfs/ioctl.c

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,7 +1382,7 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
13821382
if (vol_args->flags & BTRFS_SUBVOL_RDONLY)
13831383
readonly = true;
13841384
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));
13861386

13871387
if (vol_args->size < sizeof(*inherit) ||
13881388
vol_args->size > PAGE_SIZE) {
@@ -1395,19 +1395,9 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
13951395
goto free_args;
13961396
}
13971397

1398-
if (inherit->num_qgroups > PAGE_SIZE ||
1399-
inherit->num_ref_copies > PAGE_SIZE ||
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;
1398+
ret = btrfs_qgroup_check_inherit(fs_info, inherit, vol_args->size);
1399+
if (ret < 0)
14091400
goto free_inherit;
1410-
}
14111401
}
14121402

14131403
ret = __btrfs_ioctl_snap_create(file, file_mnt_idmap(file),

fs/btrfs/qgroup.c

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3046,6 +3046,57 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans)
30463046
return ret;
30473047
}
30483048

3049+
int 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+
30493100
static int qgroup_auto_inherit(struct btrfs_fs_info *fs_info,
30503101
u64 inode_rootid,
30513102
struct btrfs_qgroup_inherit **inherit)

fs/btrfs/qgroup.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,9 @@ int btrfs_qgroup_account_extent(struct btrfs_trans_handle *trans, u64 bytenr,
350350
struct ulist *new_roots);
351351
int btrfs_qgroup_account_extents(struct btrfs_trans_handle *trans);
352352
int btrfs_run_qgroups(struct btrfs_trans_handle *trans);
353+
int btrfs_qgroup_check_inherit(struct btrfs_fs_info *fs_info,
354+
struct btrfs_qgroup_inherit *inherit,
355+
size_t size);
353356
int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,
354357
u64 objectid, u64 inode_rootid,
355358
struct btrfs_qgroup_inherit *inherit);

include/uapi/linux/btrfs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ struct btrfs_qgroup_limit {
9292
* struct btrfs_qgroup_inherit.flags
9393
*/
9494
#define BTRFS_QGROUP_INHERIT_SET_LIMITS (1ULL << 0)
95+
#define BTRFS_QGROUP_INHERIT_FLAGS_SUPP (BTRFS_QGROUP_INHERIT_SET_LIMITS)
9596

9697
struct btrfs_qgroup_inherit {
9798
__u64 flags;

0 commit comments

Comments
 (0)