On 7/16/13 9:24 AM, Dmitry Monakhov wrote:
> On Thu, 6 Jun 2013 23:39:34 -0400, Theodore Ts'o <tytso@xxxxxxx> wrote:
>> Thanks for the review. I took your changes, and added a bit more code
>> cleanup.
>>
>> Here's the version I ultimately checked into my tree.
> This patch cause regression
> TESTCASE:
> #!/bin/bash
>
> IMG=./IMG
> MNT=/mnt
> dd if=/dev/zero of=$IMG bs=1M count=16
> mkfs.ext4 -b4096 -F $IMG
> mount $IMG $MNT -oloop || exit 1
> fallocate -n -l $((1024*1024*2)) $MNT/f1
>
> for ((i=0;i<4;i++))
> do
> dd if=/dev/zero of=$MNT/f1 bs=4k count=1 seek=$((i*100)) conv=notrunc
> done
> sync
> filefrag -v $MNT/f1
> umount $MNT
> e2fsck -f -n $IMG
Yes,this is the case where extents past EOF aren't updating the parent
node in the extent tree:
e2fsck 1.43-WIP (20-Jun-2013)
Pass 1: Checking inodes, blocks, and sizes
Inode 12, end of extent exceeds allowed value
(logical block 301, physical block 1464, len 211)
Clear? no
Inode 12, i_blocks is 4112, should be 2424. Fix? no
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information
Block bitmap differences: -(1464--1674)
Fix? no
The file looks like this:
debugfs: ex f1
Level Entries Logical Physical Length Flags
0/ 1 1/ 1 0 - 300 1675 301
^^^ length 301
^^ last logical block at 300
1/ 1 1/ 8 0 - 0 1163 - 1163 1
1/ 1 2/ 8 1 - 99 1164 - 1262 99 Uninit
1/ 1 3/ 8 100 - 100 1263 - 1263 1
1/ 1 4/ 8 101 - 199 1264 - 1362 99 Uninit
1/ 1 5/ 8 200 - 200 1363 - 1363 1
1/ 1 6/ 8 201 - 299 1364 - 1462 99 Uninit
1/ 1 7/ 8 300 - 300 1463 - 1463 1
1/ 1 8/ 8 301 - 511 1464 - 1674 211 Uninit
^^ extent past EOF extends beyond parent
Ted thought that was OK, I guess, but that seems very strange to me.
Why would we NOT update the length/range of the parent block?
Where EOF/i_size happens to land should be orthogonal to how
the extent tree is described, I would expect.
I asked in:
Subject: fallocated blocks past EOF & past parent node range OK?
but there's been no discussion so far. I'd like to understand
why it's felt that the tree above is not considered to be corrupt.
Thanks,
-Eric
>>
>> - Ted
>>
>> commit d3f32c2db8f11c87aa7939d78e7eb4c373f7034f
>> Author: David Jeffery <djeffery@xxxxxxxxxx>
>> Date: Thu Jun 6 20:04:33 2013 -0400
>>
>> e2fsck: detect invalid extents at the end of an extent-block
>>
>> e2fsck does not detect extents which are outside their location in the
>> extent tree. This can result in a bad extent at the end of an extent-block
>> not being detected.
>>
>> From a part of a dump_extents output:
>>
>> 1/ 2 37/ 68 143960 - 146679 123826181 2720
>> 2/ 2 1/ 2 143960 - 146679 123785816 - 123788535 2720
>> 2/ 2 2/ 2 146680 - 147583 123788536 - 123789439 904 Uninit <-bad extent
>> 1/ 2 38/ 68 146680 - 149391 123826182 2712
>> 2/ 2 1/ 2 146680 - 147583 18486 - 19389 904
>> 2/ 2 2/ 2 147584 - 149391 123789440 - 123791247 1808
>>
>> e2fsck does not detect this bad extent which both overlaps another, valid
>> extent, and is invalid by being beyond the end of the extent above it in
>> the tree.
>>
>> This patch modifies e2fsck to detect this invalid extent and remove it.
>>
>> Signed-off-by: David Jeffery <djeffery@xxxxxxxxxx>
>> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
>> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>>
>> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
>> index de00638..af9afe3 100644
>> --- a/e2fsck/pass1.c
>> +++ b/e2fsck/pass1.c
>> @@ -1760,11 +1760,11 @@ void e2fsck_clear_inode(e2fsck_t ctx, ext2_ino_t ino,
>>
>> static void scan_extent_node(e2fsck_t ctx, struct problem_context *pctx,
>> struct process_block_struct *pb,
>> - blk64_t start_block,
>> + blk64_t start_block, blk64_t end_block,
>> ext2_extent_handle_t ehandle)
>> {
>> struct ext2fs_extent extent;
>> - blk64_t blk;
>> + blk64_t blk, last_lblk;
>> e2_blkcnt_t blockcnt;
>> unsigned int i;
>> int is_dir, is_leaf;
>> @@ -1780,6 +1780,7 @@ static void scan_extent_node(e2fsck_t ctx, struct problem_context *pctx,
>> while (!pctx->errcode && info.num_entries-- > 0) {
>> is_leaf = extent.e_flags & EXT2_EXTENT_FLAGS_LEAF;
>> is_dir = LINUX_S_ISDIR(pctx->inode->i_mode);
>> + last_lblk = extent.e_lblk + extent.e_len - 1;
>>
>> problem = 0;
>> if (extent.e_pblk == 0 ||
>> @@ -1788,6 +1789,8 @@ 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)
>> + problem = PR_1_EXTENT_END_OUT_OF_BOUNDS;
>> else if (is_leaf && extent.e_len == 0)
>> problem = PR_1_EXTENT_LENGTH_ZERO;
>> else if (is_leaf &&
>> @@ -1822,10 +1825,9 @@ report_problem:
>> }
>>
>> if (!is_leaf) {
>> - blk64_t lblk;
>> + blk64_t lblk = extent.e_lblk;
>>
>> blk = extent.e_pblk;
>> - lblk = extent.e_lblk;
>> pctx->errcode = ext2fs_extent_get(ehandle,
>> EXT2_EXTENT_DOWN, &extent);
>> if (pctx->errcode) {
>> @@ -1847,7 +1849,8 @@ report_problem:
>> if (fix_problem(ctx, problem, pctx))
>> ext2fs_extent_fix_parents(ehandle);
>> }
>> - scan_extent_node(ctx, pctx, pb, extent.e_lblk, ehandle);
>> + scan_extent_node(ctx, pctx, pb, extent.e_lblk,
>> + last_lblk, ehandle);
>> if (pctx->errcode)
>> return;
>> pctx->errcode = ext2fs_extent_get(ehandle,
>> @@ -1928,10 +1931,10 @@ report_problem:
>> if (is_dir && extent.e_len > 0)
>> pb->last_db_block = blockcnt - 1;
>> pb->previous_block = extent.e_pblk + extent.e_len - 1;
>> - start_block = pb->last_block = extent.e_lblk + extent.e_len - 1;
>> + start_block = pb->last_block = last_lblk;
>> if (is_leaf && !is_dir &&
>> !(extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT))
>> - pb->last_init_lblock = extent.e_lblk + extent.e_len - 1;
>> + pb->last_init_lblock = last_lblk;
>> next:
>> pctx->errcode = ext2fs_extent_get(ehandle,
>> EXT2_EXTENT_NEXT_SIB,
>> @@ -1967,7 +1970,7 @@ static void check_blocks_extents(e2fsck_t ctx, struct problem_context *pctx,
>> ctx->extent_depth_count[info.max_depth]++;
>> }
>>
>> - scan_extent_node(ctx, pctx, pb, 0, ehandle);
>> + scan_extent_node(ctx, pctx, pb, 0, 0, ehandle);
>> if (pctx->errcode &&
>> fix_problem(ctx, PR_1_EXTENT_ITERATE_FAILURE, pctx)) {
>> pb->num_blocks = 0;
>> diff --git a/e2fsck/problem.c b/e2fsck/problem.c
>> index 05ef626..6d03765 100644
>> --- a/e2fsck/problem.c
>> +++ b/e2fsck/problem.c
>> @@ -954,6 +954,12 @@ static struct e2fsck_problem problem_table[] = {
>> "Logical start %b does not match logical start %c at next level. "),
>> PROMPT_FIX, 0 },
>>
>> + /* Extent end is out of bounds for the tree */
>> + { PR_1_EXTENT_END_OUT_OF_BOUNDS,
>> + N_("@i %i, end of extent exceeds allowed value\n\t(logical @b %c, physical @b %b, len %N)\n"),
>> + PROMPT_CLEAR, 0 },
>> +
>> +
>> /* Pass 1b errors */
>>
>> /* Pass 1B: Rescan for duplicate/bad blocks */
>> diff --git a/e2fsck/problem.h b/e2fsck/problem.h
>> index aed524d..b578678 100644
>> --- a/e2fsck/problem.h
>> +++ b/e2fsck/problem.h
>> @@ -561,6 +561,7 @@ struct problem_context {
>> /* Index start doesn't match start of next extent down */
>> #define PR_1_EXTENT_INDEX_START_INVALID 0x01006D
>>
>> +#define PR_1_EXTENT_END_OUT_OF_BOUNDS 0x01006E
>> /*
>> * Pass 1b errors
>> */
>> --
>> 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
> --
> 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
>
--
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
