linux-ext4 - Re: resize2fs problem with stride calc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140929205929.GA27728@birch.djwong.org>
Date:	Mon, 29 Sep 2014 13:59:30 -0700
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	TR Reardon <thomas_reardon@...mail.com>
Cc:	Eric Sandeen <sandeen@...hat.com>,
	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: Re: resize2fs problem with stride calc

On Sun, Sep 21, 2014 at 06:32:53PM -0400, TR Reardon wrote:
> > Date: Sun, 21 Sep 2014 11:13:06 -0500
> > From: sandeen@...hat.com
> >
> > On 9/20/14 3:46 PM, TR Reardon wrote:
> >> resize2fs seems to come up with some crazy default stride numbers.
> >> This occurs with and without bigalloc.
> >>
> >>
> >> I was testing enabling/disabling 64bit using latest patches from DJW,
> >> and noticed that s_raid_stride was being written with nonsensical
> >> values, in particular determine_fs_stride() is coming up with overly
> >> large values. The code is old (2006) and lacks comment so I'm not
> >> sure what the intended operation is. Does this just need to be
> >> updated for flex_bg? Should s_raid_stride ever be auto-changed on
> >> resize? If it should change, should stripe also change?
> >
> > That old commit says:
> >
> > + In addition, add code so that resize2fs can automatically
> > + determine the RAID stride parameter that had been
> > + previously used on the filesystem.
> >
> > but a year later, in 2007, this:
> >
> > commit 96c6a3acd377698cb99ffd9925bec9b20ca4f6f9
> > Author: Theodore Ts'o <tytso@....edu>
> > Date: Fri May 18 22:06:53 2007 -0400
> >
> > Store the RAID stride value in the superblock and take advantage of it
> >
> > stored it properly in the superblock (this hit e2fsprogs-1.40).
> >
> > So maybe the whole heuristic could just be removed now, but from a simple
> > test, it's working here.
> 
> Have you tried to test with flex_bg?  I think that's what raises the problem.

It'll end up recalculating stride for any flexbg FS with more than 12 BGs and
more than 3 flexbgs.  This piece is neither a part of nor is used for 32>64bit
conversion.

AFAICT, the point of determine_fs_stride() is to try to recover the RAID stride
by inferring it from minor variations in the block/inode bitmap locations
between successive block groups.  This explodes when flexbg is turned on
because bitmap blocks are stored in "other" bgs and there's a "big jump"
between the bitmaps in the last bg of one flexbg and the bitmaps of the first
bg of the next flexbg.  Between bgs in a single flexbg the *_stride values are
"negative" and don't contribute to the stride calculation.

I /think/ the solution is to ignore first blockgroup when crossing a flexbg
boundary when there are flexbgs.  Can you give the following patch a spin?
It shouldn't spit out "group XXX has stride..." messages after that.  I'm not
sure that "negative" stride ought to be ignored either, but....

Honestly I'd rather just kill the whole thing, but someone must've had a reason
to put it there?  Ted?

--D

diff --git a/resize/main.c b/resize/main.c
index 060e67d..b993dfb 100644
--- a/resize/main.c
+++ b/resize/main.c
@@ -105,6 +105,7 @@ static void determine_fs_stride(ext2_filsys fs)
 	unsigned long long sum;
 	unsigned int	has_sb, prev_has_sb = 0, num;
 	int		i_stride, b_stride;
+	int		flexbg_size = 1 << fs->super->s_log_groups_per_flex;
 
 	if (fs->stride)
 		return;
@@ -120,10 +121,11 @@ static void determine_fs_stride(ext2_filsys fs)
 			ext2fs_inode_bitmap_loc(fs, group - 1) -
 			fs->super->s_blocks_per_group;
 		if (b_stride != i_stride ||
-		    b_stride < 0)
+		    b_stride < 0 ||
+		    (flexbg_size > 1 && (group % flexbg_size == 0)))
 			goto next;
 
-		/* printf("group %d has stride %d\n", group, b_stride); */
+		printf("group %d has stride %d %d\n", group, b_stride, i_stride);
 		sum += b_stride;
 		num++;
 
@@ -133,7 +135,7 @@ static void determine_fs_stride(ext2_filsys fs)
 
 	if (fs->group_desc_count > 12 && num < 3)
 		sum = 0;
-
+printf("sum is %llu %u\n", sum, num);
 	if (num)
 		fs->stride = sum / num;
 	else
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html