From: David Sterba <dsterba@suse.com>
To: stable@vger.kernel.org
Subject: [PATCH 14/17] btrfs: handle non-fatal errors in btrfs_qgroup_inherit()
Date: Thu, 5 May 2016 11:44:21 +0200 [thread overview]
Message-ID: <1462441461-3127-1-git-send-email-dsterba@suse.com> (raw)
In-Reply-To: <20160505094018.GA29353@twin.jikos.cz>
From: Mark Fasheh <mfasheh@suse.de>
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: Mark Fasheh <mfasheh@suse.de>
Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/qgroup.c | 54 ++++++++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 5279fdae7142..7173360eea7a 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1842,8 +1842,10 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans,
}
/*
- * copy the acounting information between qgroups. This is necessary when a
- * snapshot or a subvolume is created
+ * Copy the acounting information between qgroups. This is necessary
+ * 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,
struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid,
@@ -1873,15 +1875,15 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
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;
- }
- if ((srcgroup->qgroupid >> 48) <= (objectid >> 48)) {
- ret = -EINVAL;
- goto out;
- }
+ /*
+ * Zero out invalid groups so we can ignore
+ * them later.
+ */
+ if (!srcgroup ||
+ ((srcgroup->qgroupid >> 48) <= (objectid >> 48)))
+ *i_qgroups = 0ULL;
+
++i_qgroups;
}
}
@@ -1916,17 +1918,19 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
*/
if (inherit) {
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,
objectid, *i_qgroups);
- if (ret)
+ if (ret && ret != -EEXIST)
goto out;
ret = add_qgroup_relation_item(trans, quota_root,
*i_qgroups, objectid);
- if (ret)
+ if (ret && ret != -EEXIST)
goto out;
- ++i_qgroups;
}
+ ret = 0;
}
@@ -1987,17 +1991,22 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
i_qgroups = (u64 *)(inherit + 1);
for (i = 0; i < inherit->num_qgroups; ++i) {
- ret = add_relation_rb(quota_root->fs_info, objectid,
- *i_qgroups);
- if (ret)
- goto unlock;
+ if (*i_qgroups) {
+ ret = add_relation_rb(quota_root->fs_info, objectid,
+ *i_qgroups);
+ if (ret)
+ goto unlock;
+ }
++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 *dst;
+ if (!i_qgroups[0] || !i_qgroups[1])
+ continue;
+
src = find_qgroup_rb(fs_info, i_qgroups[0]);
dst = find_qgroup_rb(fs_info, i_qgroups[1]);
@@ -2008,12 +2017,14 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
dst->rfer = src->rfer - 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 *dst;
+ if (!i_qgroups[0] || !i_qgroups[1])
+ continue;
+
src = find_qgroup_rb(fs_info, i_qgroups[0]);
dst = find_qgroup_rb(fs_info, i_qgroups[1]);
@@ -2024,7 +2035,6 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
dst->excl = src->excl + level_size;
dst->excl_cmpr = src->excl_cmpr + level_size;
- i_qgroups += 2;
}
unlock:
--
2.7.1
next prev parent reply other threads:[~2016-05-05 9:44 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-05 9:40 Btrfs stable fixes for 4.5.x David Sterba
2016-05-05 9:43 ` [PATCH 01/17] btrfs: reada: Fix in-segment calculation for reada David Sterba
2016-05-05 12:37 ` Greg KH
2016-05-05 9:43 ` [PATCH 02/17] Btrfs: fix truncate_space_check David Sterba
2016-05-05 9:43 ` [PATCH 03/17] btrfs: remove error message from search ioctl for nonexistent tree David Sterba
2016-05-05 9:43 ` [PATCH 04/17] btrfs: change max_inline default to 2048 David Sterba
2016-05-05 9:43 ` [PATCH 05/17] Btrfs: fix unreplayable log after snapshot delete + parent dir fsync David Sterba
2016-05-05 9:43 ` [PATCH 06/17] Btrfs: fix file loss on log replay after renaming a file and fsync David Sterba
2016-05-05 9:43 ` [PATCH 07/17] Btrfs: fix extent_same allowing destination offset beyond i_size David Sterba
2016-05-05 9:44 ` [PATCH 08/17] Btrfs: fix deadlock between direct IO reads and buffered writes David Sterba
2016-05-05 9:44 ` [PATCH 09/17] Btrfs: fix race when checking if we can skip fsync'ing an inode David Sterba
2016-05-05 9:44 ` [PATCH 10/17] Btrfs: do not collect ordered extents when logging that inode exists David Sterba
2016-05-05 9:44 ` [PATCH 11/17] btrfs: csum_tree_block: return proper errno value David Sterba
2016-05-05 9:44 ` [PATCH 12/17] btrfs: do not write corrupted metadata blocks to disk David Sterba
2016-05-05 9:44 ` [PATCH 13/17] Btrfs: fix invalid reference in replace_path David Sterba
2016-05-05 9:44 ` David Sterba [this message]
2016-05-05 9:44 ` [PATCH 15/17] btrfs: fallback to vmalloc in btrfs_compare_tree David Sterba
2016-05-05 9:44 ` [PATCH 16/17] Btrfs: don't use src fd for printk David Sterba
2016-05-05 9:44 ` [PATCH 17/17] btrfs: Reset IO error counters before start of device replacing David Sterba
-- strict thread matches above, loose matches on Subject: below --
2016-05-11 12:45 Btrfs stable fixes for 4.5.x (with added commit references) David Sterba
2016-05-11 12:47 ` [PATCH 14/17] btrfs: handle non-fatal errors in btrfs_qgroup_inherit() David Sterba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1462441461-3127-1-git-send-email-dsterba@suse.com \
--to=dsterba@suse.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox