diff mbox

btrfs-progs: qgroup: Fix regression leads to corrupted qgroup status

Message ID 20160907025419.23284-1-quwenruo@cn.fujitsu.com (mailing list archive)
State Accepted
Headers show

Commit Message

Qu Wenruo Sept. 7, 2016, 2:54 a.m. UTC
Commit 93dabf211d74daf6e3de642bdd887a90a00f7b49
Author: Mark Fasheh <mfasheh@suse.de>
Date:   Fri Jun 17 13:37:48 2016 -0700

    btrfs-progs: check: verify qgroups above level 0

This commit introduced a new regression which corrupts
read_qgroup_status, since it iterate leaf with manually specified slot,
not correct path->slot[0].

This leads to wrong slot[0] and read_qgroup_status() will read out wrong
flags, leading to regression.

Fix read_qgroup_status() by using eb and slot instread of wrong path
strucutre.

Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
Cc: Mark Fasheh <mfasheh@suse.de>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 qgroup-verify.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

David Sterba Sept. 29, 2016, 5:19 p.m. UTC | #1
On Wed, Sep 07, 2016 at 10:54:19AM +0800, Qu Wenruo wrote:
> Commit 93dabf211d74daf6e3de642bdd887a90a00f7b49
> Author: Mark Fasheh <mfasheh@suse.de>
> Date:   Fri Jun 17 13:37:48 2016 -0700
> 
>     btrfs-progs: check: verify qgroups above level 0
> 
> This commit introduced a new regression which corrupts
> read_qgroup_status, since it iterate leaf with manually specified slot,
> not correct path->slot[0].
> 
> This leads to wrong slot[0] and read_qgroup_status() will read out wrong
> flags, leading to regression.
> 
> Fix read_qgroup_status() by using eb and slot instread of wrong path
> strucutre.
> 
> Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
> Cc: Mark Fasheh <mfasheh@suse.de>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

I'm adding this patch to devel.  Do you have a test for the regression please?
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Qu Wenruo Sept. 30, 2016, 12:44 a.m. UTC | #2
At 09/30/2016 01:19 AM, David Sterba wrote:
> On Wed, Sep 07, 2016 at 10:54:19AM +0800, Qu Wenruo wrote:
>> Commit 93dabf211d74daf6e3de642bdd887a90a00f7b49
>> Author: Mark Fasheh <mfasheh@suse.de>
>> Date:   Fri Jun 17 13:37:48 2016 -0700
>>
>>     btrfs-progs: check: verify qgroups above level 0
>>
>> This commit introduced a new regression which corrupts
>> read_qgroup_status, since it iterate leaf with manually specified slot,
>> not correct path->slot[0].
>>
>> This leads to wrong slot[0] and read_qgroup_status() will read out wrong
>> flags, leading to regression.
>>
>> Fix read_qgroup_status() by using eb and slot instread of wrong path
>> strucutre.
>>
>> Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
>> Cc: Mark Fasheh <mfasheh@suse.de>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>
> I'm adding this patch to devel.  Do you have a test for the regression please?
>
>
Xfstests btrfs/114 can produce it.

If you mean to add btrfs-progs test case, then I can try to create a 
minimal image to reproduce it.

Thanks,
Qu


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba Sept. 30, 2016, 9:53 a.m. UTC | #3
On Fri, Sep 30, 2016 at 08:44:58AM +0800, Qu Wenruo wrote:
> 
> 
> At 09/30/2016 01:19 AM, David Sterba wrote:
> > On Wed, Sep 07, 2016 at 10:54:19AM +0800, Qu Wenruo wrote:
> >> Commit 93dabf211d74daf6e3de642bdd887a90a00f7b49
> >> Author: Mark Fasheh <mfasheh@suse.de>
> >> Date:   Fri Jun 17 13:37:48 2016 -0700
> >>
> >>     btrfs-progs: check: verify qgroups above level 0
> >>
> >> This commit introduced a new regression which corrupts
> >> read_qgroup_status, since it iterate leaf with manually specified slot,
> >> not correct path->slot[0].
> >>
> >> This leads to wrong slot[0] and read_qgroup_status() will read out wrong
> >> flags, leading to regression.
> >>
> >> Fix read_qgroup_status() by using eb and slot instread of wrong path
> >> strucutre.
> >>
> >> Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
> >> Cc: Mark Fasheh <mfasheh@suse.de>
> >> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> >
> > I'm adding this patch to devel.  Do you have a test for the regression please?
> >
> >
> Xfstests btrfs/114 can produce it.

Good.

> If you mean to add btrfs-progs test case, then I can try to create a 
> minimal image to reproduce it.

Yes please.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/qgroup-verify.c b/qgroup-verify.c
index 66eb870..f6df12d 100644
--- a/qgroup-verify.c
+++ b/qgroup-verify.c
@@ -874,15 +874,14 @@  static int add_qgroup_relation(u64 memberid, u64 parentid)
 	return 0;
 }
 
-static void read_qgroup_status(struct btrfs_path *path,
+static void read_qgroup_status(struct extent_buffer *eb, int slot,
 			      struct counts_tree *counts)
 {
 	struct btrfs_qgroup_status_item *status_item;
 	u64 flags;
 
-	status_item = btrfs_item_ptr(path->nodes[0], path->slots[0],
-				     struct btrfs_qgroup_status_item);
-	flags = btrfs_qgroup_status_flags(path->nodes[0], status_item);
+	status_item = btrfs_item_ptr(eb, slot, struct btrfs_qgroup_status_item);
+	flags = btrfs_qgroup_status_flags(eb, status_item);
 	/*
 	 * Since qgroup_inconsist/rescan_running is just one bit,
 	 * assign value directly won't work.
@@ -946,7 +945,7 @@  loop:
 			}
 
 			if (key.type == BTRFS_QGROUP_STATUS_KEY) {
-				read_qgroup_status(&path, &counts);
+				read_qgroup_status(leaf, i, &counts);
 				continue;
 			}