linux-ext4 - Re: [PATCH 1/3] ext4: Fix insertion point of extent in mext_insert_across

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:	Wed, 3 Mar 2010 20:25:09 -0500
From:	tytso@....edu
To:	Akira Fujita <a-fujita@...jp.nec.com>
Cc:	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH 1/3] ext4: Fix insertion point of extent in
 mext_insert_across_blocks()

On Wed, Mar 03, 2010 at 03:49:29PM +0900, Akira Fujita wrote:
> ext4: Fix insertion point of extent in mext_insert_across_blocks()
> 
> From: Akira Fujita <a-fujita@...jp.nec.com>
> 
> If the leaf node has 2 extent space or fewer and
> EXT4_IOC_MOVE_EXT ioctl is called
> with the file offset where after the 2nd extent covers,
> mext_insert_across_blocks() always tries to insert extent into the first extent.
> As a result, the file gets corrupted because of
> wrong extent order.  The patch fixes this problem.

Do you have test cases that we can use as part of a regression test
suite to test the EXT4_IOC_MOVE_EXT ioctl?  I'm very glad you found
these problems (although timing --- right before the merge window is
about to close --- wasn't exactly ideal), but what's more important to
me is how we get better regression testing.

The other two two patches are obviously correct, but this one is going
to require me to spend a long time staring at the verious corner cases
in order for me to convince myself that it is totally safe.  If we had
a set of test cases where we could easily verify the "before" and
"after" file system images as being correct, and then combined it with
a code coverage tool, it would make it a lot easier to validate future
patches in fs/ext4/move_extent.c.

It would be useful for other parts of the kernel as well, but at least
for the standard extents function we have some fairly aggressive
generic file system tests, combined with the fact that
fs/ext4/extents.c gets exercised much more frequently than
fs/ext4/move_extents.c.

So the question is how can get we get to the point where we can
comfortable tell people that e2defrag is totally safe, and has no
chance of corrupting their data?

						- Ted

P.S.  Here's another random idea for how we might aggressively test
the EXT4_IOC_MOVE_EXT ioctl: (1) create an empty filesystem, (2)
create a tool which randomly sets 50% of the bits in the block
allocation bitmap, marking them as in use, and making the free space
look very badly fragmented.  (3) write a large number of files into
the filesystem.  (4) calculate the checksums for all of the files.
(5) run e2fsck on the filesystem to fix up the block allocation
bitmap.  (6) defrag all of the files on the filesystem.  (7) use
e2fsck to make sure the filesystem is still consistent.  (8) calculate
the checksums for all of the files to make sure they still contain
their original data.

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