On 8/12/13 6:28 PM, Eric Sandeen wrote:

On 8/12/13 6:21 PM, Eric Sandeen wrote:

quoted

On 7/21/13 3:28 PM, Eric Whitney wrote:

quoted

Commit d3f32c2db8 caused e2fsck misbehavior during xfstests runs.
It reported that uninitialized extents created by fallocate() at
the end of file with the FALLOC_FL_KEEP_SIZE flag were invalid.
Because FALLOC_FL_KEEP_SIZE does not increase the file size when
an extent is fallocated, an uninitialized extent can legally contain
blocks past the end of file.

The information reported by ext2fs_extent_get() and used by the commit
to determine legal extent ranges is limited by the value of i_size
(determines end_blk in the root extent index), so block values greater
than that containing i_size were reported as invalid.

To fix this, filter out possible invalid extent candidates if they are
uninitialized and extend past the block containing the end of file.

Signed-off-by: Eric Whitney <redacted>
---
 e2fsck/pass1.c      |    4 +++-
 lib/ext2fs/ext2fs.h |    1 +
 lib/ext2fs/extent.c |    1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index ba6025b..b84b0d0 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c

@@ -1892,7 +1892,9 @@ static void scan_extent_node(e2fsck_t ctx, struct problem_context *pctx,
 			problem = PR_1_EXTENT_BAD_START_BLK;
 		else if (extent.e_lblk < start_block)
 			problem = PR_1_OUT_OF_ORDER_EXTENTS;
-		else if (end_block && last_lblk > end_block)
+		else if ((end_block && last_lblk > end_block) &&
+			 (!(extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT &&
+			    last_lblk > info.eof_blk - 1)))
 			problem = PR_1_EXTENT_END_OUT_OF_BOUNDS;
 		else if (is_leaf && extent.e_len == 0)
 			problem = PR_1_EXTENT_LENGTH_ZERO;

diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 311ceda..85f2ac8 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h

@@ -409,6 +409,7 @@ struct ext2_extent_info {
 	int		bytes_avail;
 	blk64_t		max_lblk;
 	blk64_t		max_pblk;
+	blk64_t         eof_blk;
 	__u32		max_len;
 	__u32		max_uninit_len;
 };

I just realized, this affects the ABI, doesn't it?  Hm.

As a hack-around, can probably just use ehandle->path[0].end_blk directly
in scan_extent_node and stash eof_blk locally?

Nope, we can't crack an extent handle, it's an opaque type.

Ned some V2 interfaces now?  :(

or maybe just:

+       eof_blk = (EXT2_I_SIZE(pctx->inode) + ctx->fs->blocksize - 1) >>
+                  EXT2_BLOCK_SIZE_BITS(ctx->fs->super);

unless that's too ugly.

-Eric (done replying to himself for now)