@@ -846,6 +846,20 @@ static noinline int btrfs_mksubvol(struct path *parent,
if (btrfs_root_refs(&BTRFS_I(dir)->root->root_item) == 0)
goto out_up_read;
+ /*
+ * FIXME: The check here is not perfect, as for create_snapshot(),
+ * we should ensure the valid inherit is not changed during
+ * qgroup_check_inherit() to create_pending_snapshots().
+ *
+ * Unforunately, we can't use qgroup_ioctl_lock, as
+ * create_pending_snapshots() can be called async in
+ * commit_transaction_async().
+ */
+ error = btrfs_qgroup_check_inherit(BTRFS_I(dir)->root->fs_info,
+ inherit, 0);
+ if (error)
+ goto out_up_read;
+
if (snap_src) {
error = create_snapshot(snap_src, dir, dentry, name, namelen,
async_transid, readonly, inherit);
@@ -2184,6 +2184,47 @@ out:
}
/*
+ * Check if the qgroup_inherit is valid.
+ *
+ * If no_ioctl_lock is set, this will not lock qgroup_ioctl_lock and caller
+ * should lock it correctly
+ */
+int btrfs_qgroup_check_inherit(struct btrfs_fs_info *fs_info,
+ struct btrfs_qgroup_inherit *inherit,
+ int no_ioctl_lock)
+{
+ struct btrfs_qgroup *qg;
+ int i;
+ int num;
+ int ret = 0;
+
+ if (!inherit || !fs_info->quota_enabled)
+ goto out_nolock;
+
+ if (!fs_info->quota_root) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!no_ioctl_lock)
+ mutex_lock(&fs_info->qgroup_ioctl_lock);
+ num = inherit->num_qgroups + 2 * inherit->num_ref_copies +
+ 2 * inherit->num_excl_copies;
+ for (i = 0; i < num; i++) {
+ qg = find_qgroup_rb(fs_info, inherit->qgroups[i]);
+ if (!qg) {
+ ret = -ENOENT;
+ goto out;
+ }
+ }
+out:
+ if (!no_ioctl_lock)
+ mutex_unlock(&fs_info->qgroup_ioctl_lock);
+out_nolock:
+ return ret;
+}
+
+/*
* copy the acounting information between qgroups. This is necessary when a
* snapshot or a subvolume is created
*/
@@ -2210,17 +2251,9 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
}
if (inherit) {
- i_qgroups = (u64 *)(inherit + 1);
- nums = inherit->num_qgroups + 2 * inherit->num_ref_copies +
- 2 * inherit->num_excl_copies;
- for (i = 0; i < nums; ++i) {
- srcgroup = find_qgroup_rb(fs_info, *i_qgroups);
- if (!srcgroup) {
- ret = -EINVAL;
- goto out;
- }
- ++i_qgroups;
- }
+ ret = btrfs_qgroup_check_inherit(fs_info, inherit, 1);
+ if (ret < 0)
+ goto out;
}
/*
@@ -92,6 +92,9 @@ void btrfs_remove_qgroup_operation(struct btrfs_trans_handle *trans,
struct btrfs_qgroup_operation *oper);
int btrfs_run_qgroups(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info);
+int btrfs_qgroup_check_inherit(struct btrfs_fs_info *fs_info,
+ struct btrfs_qgroup_inherit *inherit,
+ int no_ioctl_lock);
int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid,
struct btrfs_qgroup_inherit *inherit);
[BUG] When quota is enabled, and we pass invalid inherit to create a snapshot, it will cause abort_transaction() and the filesystem is mounted as RO. [CAUSE] Unlike subvolume creation, snapshot creation is delayed until commit_transaction(), making it too late to detect invalid qgroup_inherit but only to abort transaction. [FIX] The fix is to check the qgroup_inherit() early before going to create_snapshot(). Although this fix is not perfect as it doesn't completely kill the possible concurrency to trigger abort_transaction(). Reported-by: Remi Rampin <remirampin@gmail.com> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> --- fs/btrfs/ioctl.c | 14 ++++++++++++++ fs/btrfs/qgroup.c | 55 ++++++++++++++++++++++++++++++++++++++++++++----------- fs/btrfs/qgroup.h | 3 +++ 3 files changed, 61 insertions(+), 11 deletions(-)