Commit 918c2ee1 authored by Mark Fasheh's avatar Mark Fasheh Committed by David Sterba

btrfs: handle non-fatal errors in btrfs_qgroup_inherit()

create_pending_snapshot() will go readonly on _any_ error return from
btrfs_qgroup_inherit(). If qgroups are enabled, a user can crash their fs by
just making a snapshot and asking it to inherit from an invalid qgroup. For
example:

$ btrfs sub snap -i 1/10 /btrfs/ /btrfs/foo

Will cause a transaction abort.

Fix this by only throwing errors in btrfs_qgroup_inherit() when we know
going readonly is acceptable.

The following xfstests test case reproduces this bug:

  seq=`basename $0`
  seqres=$RESULT_DIR/$seq
  echo "QA output created by $seq"

  here=`pwd`
  tmp=/tmp/$$
  status=1	# failure is the default!
  trap "_cleanup; exit \$status" 0 1 2 3 15

  _cleanup()
  {
  	cd /
  	rm -f $tmp.*
  }

  # get standard environment, filters and checks
  . ./common/rc
  . ./common/filter

  # remove previous $seqres.full before test
  rm -f $seqres.full

  # real QA test starts here
  _supported_fs btrfs
  _supported_os Linux
  _require_scratch

  rm -f $seqres.full

  _scratch_mkfs
  _scratch_mount
  _run_btrfs_util_prog quota enable $SCRATCH_MNT
  # The qgroup '1/10' does not exist and should be silently ignored
  _run_btrfs_util_prog subvolume snapshot -i 1/10 $SCRATCH_MNT $SCRATCH_MNT/snap1

  _scratch_unmount

  echo "Silence is golden"

  status=0
  exit
Signed-off-by: default avatarMark Fasheh <mfasheh@suse.de>
Reviewed-by: default avatarQu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent 0305bc27
...@@ -1842,8 +1842,10 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans, ...@@ -1842,8 +1842,10 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans,
} }
/* /*
* copy the acounting information between qgroups. This is necessary when a * Copy the acounting information between qgroups. This is necessary
* snapshot or a subvolume is created * when a snapshot or a subvolume is created. Throwing an error will
* cause a transaction abort so we take extra care here to only error
* when a readonly fs is a reasonable outcome.
*/ */
int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid, struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid,
...@@ -1873,15 +1875,15 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, ...@@ -1873,15 +1875,15 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
2 * inherit->num_excl_copies; 2 * inherit->num_excl_copies;
for (i = 0; i < nums; ++i) { for (i = 0; i < nums; ++i) {
srcgroup = find_qgroup_rb(fs_info, *i_qgroups); srcgroup = find_qgroup_rb(fs_info, *i_qgroups);
if (!srcgroup) {
ret = -EINVAL;
goto out;
}
if ((srcgroup->qgroupid >> 48) <= (objectid >> 48)) { /*
ret = -EINVAL; * Zero out invalid groups so we can ignore
goto out; * them later.
} */
if (!srcgroup ||
((srcgroup->qgroupid >> 48) <= (objectid >> 48)))
*i_qgroups = 0ULL;
++i_qgroups; ++i_qgroups;
} }
} }
...@@ -1916,17 +1918,19 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, ...@@ -1916,17 +1918,19 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
*/ */
if (inherit) { if (inherit) {
i_qgroups = (u64 *)(inherit + 1); i_qgroups = (u64 *)(inherit + 1);
for (i = 0; i < inherit->num_qgroups; ++i) { for (i = 0; i < inherit->num_qgroups; ++i, ++i_qgroups) {
if (*i_qgroups == 0)
continue;
ret = add_qgroup_relation_item(trans, quota_root, ret = add_qgroup_relation_item(trans, quota_root,
objectid, *i_qgroups); objectid, *i_qgroups);
if (ret) if (ret && ret != -EEXIST)
goto out; goto out;
ret = add_qgroup_relation_item(trans, quota_root, ret = add_qgroup_relation_item(trans, quota_root,
*i_qgroups, objectid); *i_qgroups, objectid);
if (ret) if (ret && ret != -EEXIST)
goto out; goto out;
++i_qgroups;
} }
ret = 0;
} }
...@@ -1987,17 +1991,22 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, ...@@ -1987,17 +1991,22 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
i_qgroups = (u64 *)(inherit + 1); i_qgroups = (u64 *)(inherit + 1);
for (i = 0; i < inherit->num_qgroups; ++i) { for (i = 0; i < inherit->num_qgroups; ++i) {
ret = add_relation_rb(quota_root->fs_info, objectid, if (*i_qgroups) {
*i_qgroups); ret = add_relation_rb(quota_root->fs_info, objectid,
if (ret) *i_qgroups);
goto unlock; if (ret)
goto unlock;
}
++i_qgroups; ++i_qgroups;
} }
for (i = 0; i < inherit->num_ref_copies; ++i) { for (i = 0; i < inherit->num_ref_copies; ++i, i_qgroups += 2) {
struct btrfs_qgroup *src; struct btrfs_qgroup *src;
struct btrfs_qgroup *dst; struct btrfs_qgroup *dst;
if (!i_qgroups[0] || !i_qgroups[1])
continue;
src = find_qgroup_rb(fs_info, i_qgroups[0]); src = find_qgroup_rb(fs_info, i_qgroups[0]);
dst = find_qgroup_rb(fs_info, i_qgroups[1]); dst = find_qgroup_rb(fs_info, i_qgroups[1]);
...@@ -2008,12 +2017,14 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, ...@@ -2008,12 +2017,14 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
dst->rfer = src->rfer - level_size; dst->rfer = src->rfer - level_size;
dst->rfer_cmpr = src->rfer_cmpr - level_size; dst->rfer_cmpr = src->rfer_cmpr - level_size;
i_qgroups += 2;
} }
for (i = 0; i < inherit->num_excl_copies; ++i) { for (i = 0; i < inherit->num_excl_copies; ++i, i_qgroups += 2) {
struct btrfs_qgroup *src; struct btrfs_qgroup *src;
struct btrfs_qgroup *dst; struct btrfs_qgroup *dst;
if (!i_qgroups[0] || !i_qgroups[1])
continue;
src = find_qgroup_rb(fs_info, i_qgroups[0]); src = find_qgroup_rb(fs_info, i_qgroups[0]);
dst = find_qgroup_rb(fs_info, i_qgroups[1]); dst = find_qgroup_rb(fs_info, i_qgroups[1]);
...@@ -2024,7 +2035,6 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, ...@@ -2024,7 +2035,6 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
dst->excl = src->excl + level_size; dst->excl = src->excl + level_size;
dst->excl_cmpr = src->excl_cmpr + level_size; dst->excl_cmpr = src->excl_cmpr + level_size;
i_qgroups += 2;
} }
unlock: unlock:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment