linux-kernel - Re: [PATCH v4] ext4: fix indirect punch hole corruption

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: <20150211033720.GA20820@mew>
Date:	Tue, 10 Feb 2015 19:37:20 -0800
From:	Omar Sandoval <osandov@...ndov.com>
To:	Chris J Arges <chris.j.arges@...onical.com>
Cc:	Theodore Ts'o <tytso@....edu>,
	Andreas Dilger <adilger.kernel@...ger.ca>,
	Lukáš Czerner <lczerner@...hat.com>,
	linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] ext4: fix indirect punch hole corruption

On Tue, Feb 10, 2015 at 08:59:23PM -0600, Chris J Arges wrote:
> On 02/10/2015 03:44 PM, Omar Sandoval wrote:
> > Commit 4f579ae7de56 (ext4: fix punch hole on files with indirect
> > mapping) rewrote FALLOC_FL_PUNCH_HOLE for ext4 files with indirect
> > mapping. However, there are bugs in several corner cases. This fixes 5
> > distinct bugs:
> > 
> > 1. When there is at least one entire level of indirection between the
> > start and end of the punch range and the end of the punch range is the
> > first block of its level, we can't return early; we have to free the
> > intervening levels.
> > 
> > 2. When the end is at a higher level of indirection than the start and
> > ext4_find_shared returns a top branch for the end, we still need to free
> > the rest of the shared branch it returns; we can't decrement partial2.
> > 
> > 3. When a punch happens within one level of indirection, we need to
> > converge on an indirect block that contains the start and end. However,
> > because the branches returned from ext4_find_shared do not necessarily
> > start at the same level (e.g., the partial2 chain will be shallower if
> > the last block occurs at the beginning of an indirect group), the walk
> > of the two chains can end up "missing" each other and freeing a bunch of
> > extra blocks in the process. This mismatch can be handled by first
> > making sure that the chains are at the same level, then walking them
> > together until they converge.
> > 
> > 4. When the punch happens within one level of indirection and
> > ext4_find_shared returns a top branch for the start, we must free it,
> > but only if the end does not occur within that branch.
> > 
> > 5. When the punch happens within one level of indirection and
> > ext4_find_shared returns a top branch for the end, then we shouldn't
> > free the block referenced by the end of the returned chain (this mirrors
> > the different levels case).
> > 
> > Signed-off-by: Omar Sandoval <osandov@...ndov.com>
> > ---
> > Okay, two more bugfixes folded in, all described in the commit message.
> > I'm finally no longer seeing xfstest generic/270 cause corruptions, even
> > after running it overnight, so hopefully this is it. Chris, would you
> > mind trying this out?
> >
> 
> Omar,
> I've completed 80 iterations of this patch so far without failure!
> Normally failures have occurred between 2-15 runs. Great job, and thanks
> for your persistence in fixing this issue!
> 
> Tested-by: Chris J Arges <chris.j.arges@...onical.com>
> 

Awesome, I was starting to run out of ideas ;) Thanks for all of your
testing.

Lukáš, would you like to take a look at this?

Also, Ted and Andreas, would you prefer this all in one patch, or should
I split out each individual fix into its own patch?

Thanks!
-- 
Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/