linux-ext4 - Re: [PATCH] ext4: Prevent massive fs corruption if verifying the block bitmap fails

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: <20130717194356.GB5790@blackbox.djwong.org>
Date:	Wed, 17 Jul 2013 12:43:56 -0700
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	"Theodore Ts'o" <tytso@....edu>
Cc:	linux-ext4 <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] ext4: Prevent massive fs corruption if verifying the
 block bitmap fails

On Wed, Jul 17, 2013 at 09:19:51AM -0400, Theodore Ts'o wrote:
> On Wed, Jul 17, 2013 at 12:09:00AM -0700, Darrick J. Wong wrote:
> > 
> > I have a question though -- ext4_init_inode_bitmap (and the block bitmap
> > equivalent) contain code to detect a corrupt block group descriptor and "seal"
> > it off by setting the inode/block's free counts to zero and writing 1s to the
> > bitmap.  Does it make more sense to keep doing that, or to hook that up to this
> > EXT4_MB_GRP_CORRUPT mechanism?
> 
> The only thing a I worry a bit about this what
> ext4_init_inode_bitmap() is doing is if the block group descriptor
> checksum is wrong, who's to say that the location of the inode bitmap
> is correct?  Maybe it has been set to overlap with some valid data
> block belonging to a directory, and by memset'ing the bh to zero and
> then marking it up to date, when you try to read the directory, it
> will get all zero's instead of the valid directory information.
> (Fortunately the code in question isn't marking the bh dirty; if it
> did, then it would guarantee the overwritting the directory or data
> block in question, where as if it is just in the buffer cache marked
> uptodate, the user might get lucky and the bh might get pushed out of
> memory.)
> 
> What I would think is a better approach is to change the patch so that
> we have bits indicating an invalid block bitmap and an invalid
> inode/table bitmap, which disables block and inode allocations to that
> block group, respectively.
> 
> We could just also set the inode/block's free counts to zero, but then
> we would need to audit all of the codepaths where we decrement the
> free count to make sure it never goes negative.  (We shold be doing
> that already, though.)

Hmm, ok, I guess we could have separate flags to forbid allocating inodes and
blocks from a block group, and if we find the group descriptor to be faulty we
can forbid both.  I'll go poke on that this after lunch.

I also wrote a script that fills a fs, maliciously marks all the fs metadata
blocks as free, and writes more files to the fs, with the result that you
corrupt the metadata.  I wonder if it's feasible to modify mballoc to check
that it's not handing out well known metadata locations to files?

(metadata_csum will catch this fairly quickly, but you still end up with a
trashed fs.)

--D
> 
> 					- Ted
--
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