* Eric Sandeen <sandeen@xxxxxxxxxx>: > On 8/12/13 6:28 PM, Eric Sandeen wrote: > > On 8/12/13 6:21 PM, Eric Sandeen wrote: > >> On 7/21/13 3:28 PM, Eric Whitney wrote: > >>> 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 <enwlinux@xxxxxxxxx> > >>> --- > >>> 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. > Clearly, I wasn't thinking about the ABI at all - thanks for pointing out that misstep. So, I'd like to withdraw that patch, please, and will post a V2 in a bit. Computing the eof_blk in that manner is better than an initial patch I had that worked but which was pretty ugly. Thanks, Eric -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html