From c8ec2bad18fdaa842f786f3b37c9320a3411aea3 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Sun, 28 Jul 2013 21:49:38 -0400 Subject: [PATCH] e2fsck: correctly deallocate invalid extent-mapped symlinks The function deallocate_inode() in e2fsck/pass2.c was buggy in that it would clear out the inode's mode and flags fields before trying to deallocate any blocks which might belong to the inode. The good news is that deallocate_inode() is mostly used to free inodes which do not have blocks: device inodes, FIFO's, Unix-domain sockets. The bad news is that if deallocate_inode() tried to free an invalid extent-mapped inode, it would try to interpret the root of the extent node as block numbers, and would therefore mark various file system metadata blocks (the superblock, block group descriptors, the root directory, etc.) as free and available for allocation. This was unfortunate. (Try running an older e2fsck against the test file system image in the new test f_invalid_extent_symlink, and then run e2fsck a second time on the fs image, and weep.) Fortunately, this kind of file system image corruption appears to be fairly rare in actual practice, since it would require a very unlucky set of bits to be flipped, or a buggy file system implementation. Signed-off-by: "Theodore Ts'o" --- e2fsck/pass2.c | 7 +++++-- tests/f_invalid_extent_symlink/expect.1 | 12 ++++++++++++ tests/f_invalid_extent_symlink/expect.2 | 7 +++++++ tests/f_invalid_extent_symlink/image.gz | Bin 0 -> 1115 bytes tests/f_invalid_extent_symlink/name | 1 + 5 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 tests/f_invalid_extent_symlink/expect.1 create mode 100644 tests/f_invalid_extent_symlink/expect.2 create mode 100644 tests/f_invalid_extent_symlink/image.gz create mode 100644 tests/f_invalid_extent_symlink/name diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c index 749d264b..bceadfe6 100644 --- a/e2fsck/pass2.c +++ b/e2fsck/pass2.c @@ -1189,7 +1189,6 @@ static void deallocate_inode(e2fsck_t ctx, ext2_ino_t ino, char* block_buf) struct del_block del_block; e2fsck_read_inode(ctx, ino, &inode, "deallocate_inode"); - e2fsck_clear_inode(ctx, ino, &inode, 0, "deallocate_inode"); clear_problem_context(&pctx); pctx.ino = ino; @@ -1224,7 +1223,7 @@ static void deallocate_inode(e2fsck_t ctx, ext2_ino_t ino, char* block_buf) } if (!ext2fs_inode_has_valid_blocks2(fs, &inode)) - return; + goto clear_inode; if (LINUX_S_ISREG(inode.i_mode) && EXT2_I_SIZE(&inode) >= 0x80000000UL) ctx->large_files--; @@ -1239,6 +1238,10 @@ static void deallocate_inode(e2fsck_t ctx, ext2_ino_t ino, char* block_buf) ctx->flags |= E2F_FLAG_ABORT; return; } +clear_inode: + /* Inode may have changed by block_iterate, so reread it */ + e2fsck_read_inode(ctx, ino, &inode, "deallocate_inode"); + e2fsck_clear_inode(ctx, ino, &inode, 0, "deallocate_inode"); } /* diff --git a/tests/f_invalid_extent_symlink/expect.1 b/tests/f_invalid_extent_symlink/expect.1 new file mode 100644 index 00000000..7bda0b73 --- /dev/null +++ b/tests/f_invalid_extent_symlink/expect.1 @@ -0,0 +1,12 @@ +Pass 1: Checking inodes, blocks, and sizes +Pass 2: Checking directory structure +Symlink /a (inode #12) is invalid. +Clear? yes + +Pass 3: Checking directory connectivity +Pass 4: Checking reference counts +Pass 5: Checking group summary information + +test_filesys: ***** FILE SYSTEM WAS MODIFIED ***** +test_filesys: 11/16 files (9.1% non-contiguous), 21/100 blocks +Exit status is 1 diff --git a/tests/f_invalid_extent_symlink/expect.2 b/tests/f_invalid_extent_symlink/expect.2 new file mode 100644 index 00000000..41ceefb4 --- /dev/null +++ b/tests/f_invalid_extent_symlink/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: 11/16 files (0.0% non-contiguous), 21/100 blocks +Exit status is 0 diff --git a/tests/f_invalid_extent_symlink/image.gz b/tests/f_invalid_extent_symlink/image.gz new file mode 100644 index 0000000000000000000000000000000000000000..d4a6eef7e21483cb945052c6603173761702a89a GIT binary patch literal 1115 zcmb2|=3sbw@M|Cw^V_>;^KYlf94J&jEAUY(x5u-nZf8WlNY_aD5#O~9o z$2+&&%bfad)t6;n;h)+J1K#WV?Ei9=|NFK0-@h);pI7-b=Kbmak7M@#{MtM}I{JOh zmhZJcw^wJ!uX+8^ukVwZ@$=%_k<-bVvVrhAEnnA$VlB=uh@&$Ih^;1b(H|^+A@9FmhtYu$X*C`zJi}e6{uio_XL_(B%nN^-fR9<-3?X!(HrYcS7lXh#|eY<~&@~?SX65pjxJ}8^P zmzesvts=jkCCRnQD5LCXTK)Fdx3rE=i_>`ijeTFr(Ye=c)GvQ+%Jkbg@9tacpqLG{ z>z{60dyZ8!yYuSP*;|kG_HO-kZtJmEjw`qSnkO|Yx%8gd@+HrAto4~Dx9NJ{W+TJn zUynPNnY{3nJ@sYn^UW8zCZF4UP-Plt)U~Exp6e%{<6fM$ouPg zJn!VT#hct7Zu&Vh@=V&zy{jUx ziQm)GPdk}LhfL2{wun1cOm+2?Baa@`6|Y*-CtA*Y^UsD=_NVeR9#4=8c~Vuoa&fs^ z@7_0UeYeY$?`B!9+Zq2cVTbX{<6S#;M2KxZDe=rTb7M%DYG}0MrZl&wtDeag=lr^5 zl+|<1*Qzsl^YoefEN$5)N@y=yA5_-$^~L7RaXM*Hfz9Xc@LpZ=C4zg^4lB8N5Azo^ zPWI}1FJ9Gtq55Nnb@i2J^KZtTs@nfJl8XTwu+W+e%mSCbsQmt0d$VJI{hNaS!XN)D zoAsnW4*z`q&W-C+p7QM5#?$})-Sbze8v`tA{y%m5e!u>^%;oAge^-2;f3*FredXh$ z?r-cTYt6N4`nUi0HG^xRhgYw0014(AW8rL literal 0 HcmV?d00001 diff --git a/tests/f_invalid_extent_symlink/name b/tests/f_invalid_extent_symlink/name new file mode 100644 index 00000000..3792aac7 --- /dev/null +++ b/tests/f_invalid_extent_symlink/name @@ -0,0 +1 @@ +extent-mapped symlink with two blocks