linux-ext4 - Re: [PATCH 06/10] e2fsck: Handle s_inodes

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] [day] [month] [year] [list]

Message-ID: <20180612094154.zvvfpyeimbtg4ytu@quack2.suse.cz>
Date:   Tue, 12 Jun 2018 11:41:54 +0200
From:   Jan Kara <jack@...e.cz>
To:     Andreas Dilger <adilger@...ger.ca>
Cc:     Jan Kara <jack@...e.cz>, Ted Tso <tytso@....edu>,
        linux-ext4@...r.kernel.org
Subject: Re: [PATCH 06/10] e2fsck: Handle s_inodes_count corruption properly

On Wed 06-06-18 11:17:39, Jan Kara wrote:
> On Sun 03-06-18 22:50:39, Andreas Dilger wrote:
> > On May 30, 2018, at 6:51 AM, Jan Kara <jack@...e.cz> wrote:
> > > +	if (should_be > UINT_MAX) {
> > > +		pctx.num = should_be;
> > > +		fix_problem(ctx, PR_0_INODE_COUNT_BIG, &pctx);
> > > +		ctx->flags |= E2F_FLAG_ABORT;
> > > +		return;
> > > +	}
> > 
> > I think this should also handle the case hit by the original bug reporter,
> > "should_be == 2^32".  That would likely be the common case hit here, since
> > the number of inodes per group is typically a power of two value and
> > resizing the filesystem too much would hit exactly 2^32.  Something like:
> > 
> > 	if (should_be == (unsigned long long)UINT_MAX + 1) {
> > 		should_be = UINT_MAX;
> > 	} else if (should_be > UINT_MAX) {
> > 		:
> > 		:
> > 
> > This matches the behaviour in mke2fs that converts 2^32 inodes to 2^32-1.
> > Otherwise, it is possible for a valid filesystem formatted by mke2fs with
> > 2^32-1 inodes cannot be checked by e2fsck.
> 
> Originally I wanted to do something like this as well. The problem with
> setting number of inodes to 2^32-1 when it should really be 2^32 is that
> the kernel is not really able to handle such filesystems safely. In
> particular the code in fs/ext4/ialloc.c will happily allocate the inode
> with number 2^32 which immediately overflows to 0 and bad things happen
> after that. We could fix up the code there but older kernels won't be able
> to handle it anyway. We could mark the last inode as allocated but that
> would require more fixups in other places so in the end I've decided the
> problems simply are not worth the additional possibility of having
> filesystem larger by 1 block group.
> 
> I'll check mke2fs code and fix it up if the code there still trims inode
> count of 2^32 to 2^32-1.

So I've checked what mke2fs (lib/ext2fs/initialize.c) does and although we
do trim the inode count to 2^32-1 there, this is just an initial estimate
which is later rounded to have equal number of inodes per group and to have
full inode blocks and in the end we check that the number of inodes fits in
32 bits (and reduce number of inodes per group if not) so overall mke2fs
cannot generate filesystem where number of usable inodes in the last group
would be different from the number of usable inodes in other groups.

I've fixed all the other issues you've mentioned.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR