linux-ext4 - Re: [PATCH 1/1 v4] ext4: fix xfstests 75, 112, 127 punch hole failure

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: <20110811031712.GF3625@thunk.org>
Date:	Wed, 10 Aug 2011 23:17:12 -0400
From:	Ted Ts'o <tytso@....edu>
To:	Allison Henderson <achender@...ux.vnet.ibm.com>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 1/1 v4] ext4: fix xfstests 75, 112, 127 punch hole
 failure

On Tue, Aug 09, 2011 at 02:10:44PM -0700, Allison Henderson wrote:
> >	/*
> >	 * Now we need to zero out the non-block-aligned data.
> >	 * If the file space being truncated is smaller than
> >	 * than a block, just zero out the middle
> >	 */
> >
> Hmm, for this piece here, Im not sure I quite follow you.  I was
> pretty sure that ext4_block_zero_page() only deals with ranges that
> appear with in one block.  

Yes, that's true.  What I was objecting to in the comment is the
phrase "smaller than a block".  That's not right, or it's only right
if blocksize == page size.  That comment should really read, "if the
file space is smaller than a ***page***" zero out the middle.

That's what happens in ext4_block_zero_page(), and so it works
correctly; but the comment is confusing, and it makes the reader think
that all we only need to zero is based on block boundaries, when in
fact when we look at zeroing memory, we have to base it on page
boundaries.

Basically for either punch or truncate what we must do is:

   *) Zero partial pages
   *) Unmap full pages
   *) We take buffer_heads which have been freed and either
      (a) detach them, or 
      (b) clear the mapped flag, to indicate that the block number
           in the bh is invalid 

Using "unmap" for both pages and blocks can be confusing, since for
pages, unmapping means that we remove the page completely from the
page cache, so any future attempt to read from that virtual memory
address will result in a zero page getting mapped in.

However, for buffers, "unmapping" merely means that we are removing
the mapping to the disk block which has been deallocated by the punch
operation.  It does not result in anything getting zero'ed out in the
_page_ cache, which is why we need to zero out partial pages.

Does that make sense?

						- 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