linux-ext4 - Re: [PATCH 29/47] resize2fs: don't mark unallocated bg metadata blocks when fixing bg flags

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]

Date:	Sun, 14 Dec 2014 23:09:00 -0800
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	"Theodore Ts'o" <tytso@....edu>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 29/47] resize2fs: don't mark unallocated bg metadata
 blocks when fixing bg flags

On Sun, Dec 14, 2014 at 09:17:48PM -0500, Theodore Ts'o wrote:
> On Fri, Nov 07, 2014 at 01:53:54PM -0800, Darrick J. Wong wrote:
> > When fixing up block group bitmaps on the new fs, don't try to mark
> > unallocated group metadata blocks; these will be allocated (and
> > marked) later.
> 
> The commit description doesn't match the diff.  It looks like what you
> are doing is adding a check in case ext2fs_{block,inode}_bitmap_loc is
> zero (which is not bad, but when can that happen; and if it's
> something that _can_ happen, should we be triggering an assert
> failure), and using ext2fs_mark_block_bitmap_range2() instead of a for
> loop.
> 
> Is there a back story to this patch?

Looking through my notes, I think the reason I wrote this patch was that if
we're running resize2fs to enlarge inodes on a nearly full filesystem, we can
end up setting those bg fields to zero to force a later reallocation because
the only free space big enough to hold the larger inode table is somewhere else
in the FS. In the meantime, trying to mark block zero results in a lot of
unnecessary stderr spew about the invalid block number.  This issue doesn't
affect resize2fs functionality, it just looks scary.

Yeah, probably we should use mark...range().

--D

> 
> Thanks,
> 
> 						- Ted
> 
> > diff --git a/resize/resize2fs.c b/resize/resize2fs.c
> > index 94c0643..a760593 100644
> > --- a/resize/resize2fs.c
> > +++ b/resize/resize2fs.c
> > @@ -242,7 +242,6 @@ static void fix_uninit_block_bitmaps(ext2_filsys fs)
> >  {
> >  	blk64_t		blk, lblk;
> >  	dgrp_t		g;
> > -	int		i;
> >  
> >  	if (!ext2fs_has_group_desc_csum(fs))
> >  		return;
> > @@ -257,14 +256,16 @@ static void fix_uninit_block_bitmaps(ext2_filsys fs)
> >  						  lblk - blk + 1);
> >  
> >  		ext2fs_reserve_super_and_bgd(fs, g, fs->block_map);
> > -		ext2fs_mark_block_bitmap2(fs->block_map,
> > -					  ext2fs_block_bitmap_loc(fs, g));
> > -		ext2fs_mark_block_bitmap2(fs->block_map,
> > -					  ext2fs_inode_bitmap_loc(fs, g));
> > -		for (i = 0, blk = ext2fs_inode_table_loc(fs, g);
> > -		     i < (unsigned int) fs->inode_blocks_per_group;
> > -		     i++, blk++)
> > +		blk = ext2fs_block_bitmap_loc(fs, g);
> > +		if (blk)
> > +			ext2fs_mark_block_bitmap2(fs->block_map, blk);
> > +		blk = ext2fs_inode_bitmap_loc(fs, g);
> > +		if (blk)
> >  			ext2fs_mark_block_bitmap2(fs->block_map, blk);
> > +		blk = ext2fs_inode_table_loc(fs, g);
> > +		if (blk)
> > +			ext2fs_mark_block_bitmap_range2(fs->block_map, blk,
> > +						fs->inode_blocks_per_group);
> >  	}
> >  }
> >  
> > 
> --
> 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
--
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