[PATCH v2 0/3] btrfs: qgroup: Prepare to deprecate unused features for btrfs_qgroup_inherit()

[PATCH v2 0/3] btrfs: qgroup: Prepare to deprecate unused features for btrfs_qgroup_inherit()

[Qu Wenruo]

This patchset can be fetched from github:
https://github.com/adam900710/linux/tree/qgroup_inherit_check
Which is based on v4.19-rc1 tag.

This patchset will firstly set btrfs_qgroup_inherit structure size limit
from PAGE_SIZE to fixed SZ_4K.
I understand this normally will cause compatibility problem, but
considering how minor this feature is and no sane guy should use it for
over 100 qgroups, it should be fine in real world.

The theoretical difference can be found in the commit message of the 1st
patch.

The 2nd patch introduce check function for btrfs_qgroup_inherit
structure and deprecates the following features:
1) flags
   No unsupported flags should sneak in.

2) num_qgroups limit
   It should not exceed the maximum size of btrfs_qgroup_inherit nor the
   size passed from btrfs_ioctl_vol_args_v2.

3) qgroupid
   No valid qgroupid should be 0/0.

The 3rd patch prepare to deprecate the following features:

1) flags
2) num_excl|ref_copies
3) btrfs_qgroup_inherit::lim

Details can be found in the 3rd patch.

changelog:
v2:
  Show more details in the 1st patch about the incompatibility for 64K
  page size arch.
  Don't deprecate unused features right now, only mark then deprecated
  and output warning message for now.
  Better split validation check and deprecation patches.

Qu Wenruo (3):
  btrfs: Set qgroup inherit size limit to SZ_4K instead of page size
  btrfs: qgroup: Validate btrfs_qgroup_inherit structure before passing
    it to qgroup
  btrfs: qgroup: Prepare to deprecate some btrfs_qgroup_inherit features

 fs/btrfs/ioctl.c           |  7 +++++-
 fs/btrfs/qgroup.c          | 44 ++++++++++++++++++++++++++++++++++++++
 fs/btrfs/qgroup.h          |  3 +++
 include/uapi/linux/btrfs.h | 17 +++++++--------
 4 files changed, 61 insertions(+), 10 deletions(-)

-- 
2.18.0

[PATCH v2 1/3] btrfs: Set qgroup inherit size limit to SZ_4K instead of page size

[Qu Wenruo]

Change btrfs_qgroup_inherit maximum size from PAGE_SIZE to SZ_4K to make
it consistent across different architectures.

This change will only affect architectures whose page size is not
(larger than) 4K, and will only affect how many qgroups can exist in
btrfs_qgroup_inherit structure:

Before: (64K page size)
8183

After: (fixed to 4K btrfs_qgroup_inherit size)
503

Although in theory this could lead to incompatibility, but considering
how rare btrfs_qgroup_inherit is used, it's still not too late to change
it without impacting a large user base.

And passing over 100 qgroups in one btrfs_qgroup_inherit structure is
already insane.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/ioctl.c           | 2 +-
 include/uapi/linux/btrfs.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 63600dc2ac4c..5db8680b40a9 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1811,7 +1811,7 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
 	if (vol_args->flags & BTRFS_SUBVOL_RDONLY)
 		readonly = true;
 	if (vol_args->flags & BTRFS_SUBVOL_QGROUP_INHERIT) {
-		if (vol_args->size > PAGE_SIZE) {
+		if (vol_args->size > BTRFS_QGROUP_INHERIT_MAX_SIZE) {
 			ret = -EINVAL;
 			goto free_args;
 		}
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 5ca1d21fc4a7..311edb65567c 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -82,6 +82,7 @@ struct btrfs_qgroup_limit {
  */
 #define BTRFS_QGROUP_INHERIT_SET_LIMITS	(1ULL << 0)
 
+#define BTRFS_QGROUP_INHERIT_MAX_SIZE	(SZ_4K)
 struct btrfs_qgroup_inherit {
 	__u64	flags;
 	__u64	num_qgroups;
-- 
2.18.0

[PATCH v2 2/3] btrfs: qgroup: Validate btrfs_qgroup_inherit structure before passing it to qgroup

[Qu Wenruo]

btrfs_qgroup_inherit structure doesn't goes through much validation
check.

Now do a comprehensive check for it, including:
1) Inherit size
   Should not exceeding SZ_4K and its num_qgroups should not
   exceed its size passed in btrfs_ioctl_vol_args_v2.

2) Flags
   Should not include any unknown flags

3) Qgroupid
   Comprehensive check is already in btrfs_qgroup_inherit(), here we
   only check if there is any obviously invalid qgroupid (0).

Coverity-id: 1021055
Reported-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/ioctl.c           |  5 +++++
 fs/btrfs/qgroup.c          | 33 +++++++++++++++++++++++++++++++++
 fs/btrfs/qgroup.h          |  3 +++
 include/uapi/linux/btrfs.h | 16 +++++++---------
 4 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 5db8680b40a9..ce88d6518076 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1820,6 +1820,11 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
 			ret = PTR_ERR(inherit);
 			goto free_args;
 		}
+		ret = btrfs_validate_inherit(
+				BTRFS_I(file_inode(file))->root->fs_info,
+				inherit, vol_args->size);
+		if (ret < 0)
+			goto free_args;
 	}
 
 	ret = btrfs_ioctl_snap_create_transid(file, vol_args->name,
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 4353bb69bb86..75e1dccf8ed3 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2232,6 +2232,39 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans)
 	return ret;
 }
 
+/*
+ * To make sure the inherit passed in is valid
+ *
+ * Here we only check flags and rule out some no-longer supported features.
+ * And we only do very basis qgroupid check to ensure there is no obviously
+ * invalid qgroupid (0). Detailed qgroupid check will be done in
+ * btrfs_qgroup_inherit().
+ */
+int btrfs_validate_inherit(struct btrfs_fs_info *fs_info,
+			   struct btrfs_qgroup_inherit *inherit,
+			   u64 inherit_size)
+{
+	u64 i;
+
+	if (inherit->flags & ~BTRFS_QGROUP_INHERIT_FLAGS_SUPP)
+		return -ENOTTY;
+
+	/* Size check */
+	if (sizeof(u64) * inherit->num_qgroups +
+	    sizeof(u64) * inherit->num_ref_copies * 2 +
+	    sizeof(u64) * inherit->num_excl_copies * 2 +
+	    sizeof(*inherit) >
+	    min_t(u64, BTRFS_QGROUP_INHERIT_MAX_SIZE, inherit_size))
+		return -EINVAL;
+
+	/* Qgroup 0/0 is not allowed */
+	for (i = 0; i < inherit->num_qgroups; i++) {
+		if (inherit->qgroups[i] == 0)
+			return -EINVAL;
+	}
+	return 0;
+}
+
 /*
  * Copy the accounting information between qgroups. This is necessary
  * when a snapshot or a subvolume is created. Throwing an error will
diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
index 54b8bb282c0e..6031c0beb798 100644
--- a/fs/btrfs/qgroup.h
+++ b/fs/btrfs/qgroup.h
@@ -241,6 +241,9 @@ int btrfs_qgroup_account_extent(struct btrfs_trans_handle *trans, u64 bytenr,
 				struct ulist *new_roots);
 int btrfs_qgroup_account_extents(struct btrfs_trans_handle *trans);
 int btrfs_run_qgroups(struct btrfs_trans_handle *trans);
+int btrfs_validate_inherit(struct btrfs_fs_info *fs_info,
+			   struct btrfs_qgroup_inherit *inherit,
+			   u64 inherit_size);
 int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,
 			 u64 objectid, struct btrfs_qgroup_inherit *inherit);
 void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info,
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 311edb65567c..3398f35a5528 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -74,21 +74,19 @@ struct btrfs_qgroup_limit {
 	__u64	rsv_excl;
 };
 
-/*
- * flags definition for qgroup inheritance
- *
- * Used by:
- * struct btrfs_qgroup_inherit.flags
- */
+/* flags definition for qgroup inheritance (will be deprecated) */
 #define BTRFS_QGROUP_INHERIT_SET_LIMITS	(1ULL << 0)
 
+#define BTRFS_QGROUP_INHERIT_FLAGS_SUPP (BTRFS_QGROUP_INHERIT_SET_LIMITS)
+
 #define BTRFS_QGROUP_INHERIT_MAX_SIZE	(SZ_4K)
+
 struct btrfs_qgroup_inherit {
 	__u64	flags;
 	__u64	num_qgroups;
-	__u64	num_ref_copies;
-	__u64	num_excl_copies;
-	struct btrfs_qgroup_limit lim;
+	__u64	num_ref_copies;		/* will be deprecated */
+	__u64	num_excl_copies;	/* will be deprecated */
+	struct btrfs_qgroup_limit lim;	/* will be deprecated */
 	__u64	qgroups[0];
 };
 
-- 
2.18.0

[PATCH v2 3/3] btrfs: qgroup: Prepare to deprecate some btrfs_qgroup_inherit features

[Qu Wenruo]

Prepare to deprecate the following btrfs_qgroup_inherit features:

1) Flags
   There is only one flag supported, BTRFS_QGROUP_INHERIT_SET_LIMITS,
   however it's never used by btrfs-progs.

2) num_excl|ref_copies
   These two features are going to copy excl/rfer numbers from one
   qgroup to another.
   There is no easier way to screw up qgroup numbers like this.

3) btrfs_qgroup_inherit::lim
   It's used along with BTRFS_QGROUP_INHERIT_SET_LIMITS, never supported
   by btrfs-progs either.

Just output warnings for such features for now.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/qgroup.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 75e1dccf8ed3..aecd3cfc7c78 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2248,6 +2248,9 @@ int btrfs_validate_inherit(struct btrfs_fs_info *fs_info,
 
 	if (inherit->flags & ~BTRFS_QGROUP_INHERIT_FLAGS_SUPP)
 		return -ENOTTY;
+	if (inherit->flags & BTRFS_QGROUP_INHERIT_SET_LIMITS)
+		btrfs_warn(fs_info,
+"btrfs_qgroup_inherit::flags BTRFS_QGROUP_INHERIT_SET_LIMITS will be deprecated");
 
 	/* Size check */
 	if (sizeof(u64) * inherit->num_qgroups +
@@ -2256,12 +2259,20 @@ int btrfs_validate_inherit(struct btrfs_fs_info *fs_info,
 	    sizeof(*inherit) >
 	    min_t(u64, BTRFS_QGROUP_INHERIT_MAX_SIZE, inherit_size))
 		return -EINVAL;
+	if (inherit->num_excl_copies || inherit->num_ref_copies)
+		btrfs_warn(fs_info,
+	"btrfs_qgroup_inherit::num_excl/ref_copies will be deprecated");
 
 	/* Qgroup 0/0 is not allowed */
 	for (i = 0; i < inherit->num_qgroups; i++) {
 		if (inherit->qgroups[i] == 0)
 			return -EINVAL;
 	}
+	if (inherit->lim.max_rfer || inherit->lim.max_excl ||
+	    inherit->lim.rsv_rfer || inherit->lim.rsv_excl ||
+	    inherit->lim.flags)
+		btrfs_warn(fs_info,
+			"btrfs_qgroup_inherit::lim will be deprecated");
 	return 0;
 }
 
-- 
2.18.0