From: Tao Ma <[email protected]>
Hi all,
This is the V2 for the bug of e2fsck. patch 1 is
the fix and patch 2 is the regression test case.
Tao Ma (2):
e2fsck: Let end_blk to be the maximum value of u32.
e2fsck/tests: Add the regression test case for e2fsck.
lib/ext2fs/extent.c | 4 +---
tests/f_zero_extent_length/expect.1 | 7 +++++++
tests/f_zero_extent_length/expect.2 | 7 +++++++
tests/f_zero_extent_length/image.gz | Bin 0 -> 5102 bytes
tests/f_zero_extent_length/name | 1 +
5 files changed, 16 insertions(+), 3 deletions(-)
create mode 100644 tests/f_zero_extent_length/expect.1
create mode 100644 tests/f_zero_extent_length/expect.2
create mode 100644 tests/f_zero_extent_length/image.gz
create mode 100644 tests/f_zero_extent_length/name
From: Tao Ma <[email protected]>
Now we can use fallocate to create a large file while keep the size
to be small. It will cause the e2fsck complain about it. The test
script is simple and I have pasted it here.
DEVICE=/dev/sdb1
mount -t ext4 $DEVICE /mnt/ext4
for((i=0;i<10;i++))do fallocate -n -o $[$i*8192] -l 4096 /mnt/ext4/a;done
umount $DEVICE
e2fsck -fn $DEVICE
The error message will be like this:
e2fsck 1.42.3 (14-May-2012)
Pass 1: Checking inodes, blocks, and sizes
Inode 12 has zero length extent
(invalid logical block 0, physical block 32775)
Clear? no
Inode 12, i_blocks is 88, should be 0. 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: -(8231--8232) -(32770--32778)
Fix? no
Now actually the end_blk can be any value which is less than
u32, so make end_blk be the maximum value of u32.
Signed-off-by: Tao Ma <[email protected]>
---
lib/ext2fs/extent.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
index eb096d6..e2815c2 100644
--- a/lib/ext2fs/extent.c
+++ b/lib/ext2fs/extent.c
@@ -253,9 +253,7 @@ extern errcode_t ext2fs_extent_open2(ext2_filsys fs, ext2_ino_t ino,
ext2fs_le16_to_cpu(eh->eh_entries);
handle->path[0].max_entries = ext2fs_le16_to_cpu(eh->eh_max);
handle->path[0].curr = 0;
- handle->path[0].end_blk =
- (EXT2_I_SIZE(handle->inode) + fs->blocksize - 1) >>
- EXT2_BLOCK_SIZE_BITS(fs->super);
+ handle->path[0].end_blk = ((((unsigned long long) 1) << 32) - 1);
handle->path[0].visit_num = 1;
handle->level = 0;
handle->magic = EXT2_ET_MAGIC_EXTENT_HANDLE;
--
1.7.1
From: Tao Ma <[email protected]>
In commit "e2fsck: Let end_blk to be the maximum value of u32."
we fix a bug where a normal fallocate will cause e2fsck complain.
So add it to the regression test suite.
Signed-off-by: Tao Ma <[email protected]>
---
tests/f_zero_extent_length/expect.1 | 7 +++++++
tests/f_zero_extent_length/expect.2 | 7 +++++++
tests/f_zero_extent_length/image.gz | Bin 0 -> 5102 bytes
tests/f_zero_extent_length/name | 1 +
4 files changed, 15 insertions(+), 0 deletions(-)
create mode 100644 tests/f_zero_extent_length/expect.1
create mode 100644 tests/f_zero_extent_length/expect.2
create mode 100644 tests/f_zero_extent_length/image.gz
create mode 100644 tests/f_zero_extent_length/name
diff --git a/tests/f_zero_extent_length/expect.1 b/tests/f_zero_extent_length/expect.1
new file mode 100644
index 0000000..762159c
--- /dev/null
+++ b/tests/f_zero_extent_length/expect.1
@@ -0,0 +1,7 @@
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+test_filesys: 12/1024 files (16.7% non-contiguous), 1227/4096 blocks
+Exit status is 0
diff --git a/tests/f_zero_extent_length/expect.2 b/tests/f_zero_extent_length/expect.2
new file mode 100644
index 0000000..762159c
--- /dev/null
+++ b/tests/f_zero_extent_length/expect.2
@@ -0,0 +1,7 @@
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+test_filesys: 12/1024 files (16.7% non-contiguous), 1227/4096 blocks
+Exit status is 0
diff --git a/tests/f_zero_extent_length/image.gz b/tests/f_zero_extent_length/image.gz
new file mode 100644
index 0000000000000000000000000000000000000000..0b6915805bc7c3d67b9bf0f944f980b36012268e
GIT binary patch
literal 5102
zcmb2|=HM{n-0IK3oSB=Lp33m{u78e5s070Y^WBScwzp2RH+-<N(S>t{lY_{A6(w~8
zL)9)$^`=g#<SjBsT>RICD+?rfbW|xE5jfl^Fk47()~U$+s*edxoa}rp6SlP%t;{b=
zOY|0Bx$rI5=ijRG?`Pk<Idl8%@4o5V4(D(<WG*#ibNXRwW?khxV@=zHE%tU1E-~*;
z&fEC#d6G(E+OxCLp2a&w3wLeJcwJd&x=`@(Q;FT$iZ#>DN?U*Zbz%DYuYW#-zWv;o
zzyIfhcX>HKGoGB7R&H<m=?(MV8UHMcvvPl)kN>}Q*|pujU(VlG{qKRkzl_bqBYfXQ
z`%~91whPSNC#$>h_1Wohn%DLy9y=vr{;1mIR^e~v;%6$d(@t&sSa<6oV?@&Gq&58;
zbyL3DN3PuxdLX6rSJC!gtPH3i)#!8m7q9L2rxta*|MD^K|B9FE-rO^*Rb0u_`p3?^
zc43gD)T#FmH>+LImY4hY<HWP?eDy}3{OZ=vxxd=;Q(3sX?uLL(R{87ypUjH;@8G#Z
zTz1OzU+tfQ{s+Xpl0W7B*ST_Ky@vi*{-;a-E!=a(9wzDaSbuMU)}4FIOV*lje!rnz
z@<Z;uUwk324NKRr^I1EBT-NTkWtw)%r+)f&{*tq+BAQ<(=X4itC|?zoY5ivYs(q!a
zqkpp|d=x$Ye{J&rN=dobl6St`?!Mf8T{}T@%iRCr&UG3WJhT6A*qgCk^zExfu~WCd
z{CVS=`0n!u4i)_4e|oj<-~PqrMc01*2M5YGZiepMKUaU8za|j&rH6OBQvDL4Yi76q
zoL~60Gvykq;LW`O*L4nj+y7rFe}32PTvt1F&U};Gwr3}AuK8WAx78xPMD2C#Eslpd
zbL2EW6&uD!KM8;I@vi*C{NIKDuHN~$-(+3-*Xhr0&$s=z`Y8LPt=m4<yPM^I4Jw=B
z9;AHxf1dB<osaD~oB4OVlYhSZ?~dp1w*37Q?*IC`f9bU!=d+SGWNZ%p!D|?HY)2q(
z;urfb7ys^GxM$bjpU&?4{wI5$`SGv+(rW)JhyQIeoM`4`G5Os4Jw+Q%tUp$hJTd-t
z{yabP&#w*dM?Nu6-naV1@?V=b-?Ewi?$dqw7kysJxpgx(zJGG>*_W5y?9);oKfADW
zPi}0BUEP<8&9UJ=_ut#kj*+muR#=z(<%n(gggaOD@=xd6zZbk(_Se63-+5rDSEgN_
zf7@I?ZapYXHMBnYC4Xwgo9q9l%za(Yh?>mk%UiHHtH<P6vY}l6Mjba@H~*fho_i9t
zhSQEm2rjli=Fl_s?h76Ix@gq4(GVC7fdLMI4;NN~3(A*U1P{$LX^EORzxSuZ#QEJn
zZz=7+B!4JjVtnt<V@mtQKCgAHvwddW{U_~Ax#-W3Gv7simi%|xuO!Dd)ooVF+kKO(
zXJ1~fFBdVbceeBNPus$D%%Mf5`uE89trL@FpPRe+-zjE28T<Crwm;uK53sjKtsf16
n(GVC7fzc2c4S~@R7!85Z5Eu=CArb-&ANXYirY~S%a9{uc`yfdH
literal 0
HcmV?d00001
diff --git a/tests/f_zero_extent_length/name b/tests/f_zero_extent_length/name
new file mode 100644
index 0000000..9e0c6e0
--- /dev/null
+++ b/tests/f_zero_extent_length/name
@@ -0,0 +1 @@
+fallocated extents after i_size
--
1.7.1
Hi Tao,
Thanks for reporting the problem and submitting a test case. I've
kept the regression test case (although I ended up renaming it).
However, I think a better fix is to change e2fsck, since it wasn't
making a check that it shouldn't have been making in the first place.
The commit description describes the fix which I think is better....
- Ted
commit 9c40d14841f04811097a123d6e8555e78ce56811
Author: Theodore Ts'o <[email protected]>
Date: Mon Jun 11 00:25:45 2012 -0400
e2fsck: only check for zero-length leaf extents
The on-disk format for interior nodes in the extent tree does not
encode the length of each entry in the interior node; instead, it is
synthesized/simulated by the extent library code in libext2fs.
Unfortunately, this simulation is not perfect; in particular it does
not work for the last extent in the extent tree if there are
uninitialized blocks allocated using fallocate with
FALLOC_FL_KEEP_SIZE, and it leads to e2fsck incorrectly complaining
about an invalid zero-length extent.
We only need to worry about the extent length for the leaves of the
tree, since it is there were we are checking an on-disk value, as
opposed to a software-generated simulation. So restrict the check of
extent length to leaf nodes in the extent tree.
Reported-by: Tao Ma <[email protected]>
Signed-off-by: "Theodore Ts'o" <[email protected]>
diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index c6aae6e..72dcd97 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -1779,7 +1779,7 @@ 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 (extent.e_len == 0)
+ else if (is_leaf && extent.e_len == 0)
problem = PR_1_EXTENT_LENGTH_ZERO;
else if (is_leaf &&
(extent.e_pblk + extent.e_len) >
On Thu, May 17, 2012 at 03:16:07PM +0800, Tao Ma wrote:
> From: Tao Ma <[email protected]>
>
> In commit "e2fsck: Let end_blk to be the maximum value of u32."
> we fix a bug where a normal fallocate will cause e2fsck complain.
> So add it to the regression test suite.
>
> Signed-off-by: Tao Ma <[email protected]>
I've changed the description of this commit to reflect the change in
how I propose to fix the problem....
- Ted
commit 8d12c46a22965179cae1e3b47778fdee5efeb513
Author: Tao Ma <[email protected]>
Date: Sun Jun 10 23:56:30 2012 -0400
tests: add new test f_zero_extent_length
If all of the extents in the last extent tree block (ETB) in a
non-trivial extent tree contain uninitialized extents which are after
the end of the file as defined by i_size, the hueristics will
incorrectly estimate the last entry (and hence the node's e_len field)
in the last entry of each level of the extent tree.
As Tao Ma has noted, since e2fsck was requiring that the length
(e_len) field of interior nodes be non-zero, this was causing false
failures where e2fsck would declare that the extent tree was
corrupted.
This was fixed in commit 9c40d14841: "e2fsck: only check for
zero-length leaf extents". Add a regression test case to ensure that
this issue remains fixed.
Signed-off-by: Tao Ma <[email protected]>
Signed-off-by: Theodore Ts'o <[email protected]>