From eb89a6287d27d90f10cf7b80da4a5dbd9d1bcd3e Mon Sep 17 00:00:00 2001 From: "Darrick J. Wong" Date: Sat, 26 Jul 2014 17:14:40 -0400 Subject: [PATCH] e2fsck: write dir blocks after new inode when reconstructing root/lost+found If we trash the root directory block, e2fsck will find inode 11 (the old lost+found) and try to attach it to l+f. The lost+found checker also fails to find l+f and tries to add one to the root dir. The root dir is not found but is recreated with incorrect checksums, so linking in the l+f dir fails and the l+f '..' entry isn't set. Since both dirs now fail checksum verification, they're both referred to rehash to have that fixed, but because l+f doesn't have a '..' entry, rehash crashes because l+f has < 2 entries. On a checksumming filesystem, the routines in e2fsck that recreate /lost+found and / must write the new directory block *after* the inode has been written to disk because the checksum depends on i_generation. Add a regression test while we're at it. Signed-off-by: Darrick J. Wong Signed-off-by: Theodore Ts'o --- e2fsck/pass3.c | 85 +++---- tests/f_rebuild_csum_rootdir/expect.1 | 311 ++++++++++++++++++++++++++ tests/f_rebuild_csum_rootdir/expect.2 | 7 + tests/f_rebuild_csum_rootdir/image.gz | Bin 0 -> 12476 bytes tests/f_rebuild_csum_rootdir/name | 1 + 5 files changed, 364 insertions(+), 40 deletions(-) create mode 100644 tests/f_rebuild_csum_rootdir/expect.1 create mode 100644 tests/f_rebuild_csum_rootdir/expect.2 create mode 100644 tests/f_rebuild_csum_rootdir/image.gz create mode 100644 tests/f_rebuild_csum_rootdir/name diff --git a/e2fsck/pass3.c b/e2fsck/pass3.c index 3fd1253d2..31131ab53 100644 --- a/e2fsck/pass3.c +++ b/e2fsck/pass3.c @@ -204,28 +204,6 @@ skip_new_block: ext2fs_mark_block_bitmap2(fs->block_map, blk); ext2fs_mark_bb_dirty(fs); - /* - * Now let's create the actual data block for the inode - */ - pctx.errcode = ext2fs_new_dir_block(fs, EXT2_ROOT_INO, EXT2_ROOT_INO, - &block); - if (pctx.errcode) { - pctx.str = "ext2fs_new_dir_block"; - fix_problem(ctx, PR_3_CREATE_ROOT_ERROR, &pctx); - ctx->flags |= E2F_FLAG_ABORT; - return; - } - - pctx.errcode = ext2fs_write_dir_block4(fs, blk, block, 0, - EXT2_ROOT_INO); - if (pctx.errcode) { - pctx.str = "ext2fs_write_dir_block4"; - fix_problem(ctx, PR_3_CREATE_ROOT_ERROR, &pctx); - ctx->flags |= E2F_FLAG_ABORT; - return; - } - ext2fs_free_mem(&block); - /* * Set up the inode structure */ @@ -248,6 +226,30 @@ skip_new_block: return; } + /* + * Now let's create the actual data block for the inode. + * Due to metadata_csum, we must write the dir blocks AFTER + * the inode has been written to disk! + */ + pctx.errcode = ext2fs_new_dir_block(fs, EXT2_ROOT_INO, EXT2_ROOT_INO, + &block); + if (pctx.errcode) { + pctx.str = "ext2fs_new_dir_block"; + fix_problem(ctx, PR_3_CREATE_ROOT_ERROR, &pctx); + ctx->flags |= E2F_FLAG_ABORT; + return; + } + + pctx.errcode = ext2fs_write_dir_block4(fs, blk, block, 0, + EXT2_ROOT_INO); + ext2fs_free_mem(&block); + if (pctx.errcode) { + pctx.str = "ext2fs_write_dir_block4"; + fix_problem(ctx, PR_3_CREATE_ROOT_ERROR, &pctx); + ctx->flags |= E2F_FLAG_ABORT; + return; + } + /* * Miscellaneous bookkeeping... */ @@ -471,24 +473,6 @@ skip_new_block: ext2fs_mark_inode_bitmap2(ctx->inode_dir_map, ino); ext2fs_inode_alloc_stats2(fs, ino, +1, 1); - /* - * Now let's create the actual data block for the inode - */ - retval = ext2fs_new_dir_block(fs, ino, EXT2_ROOT_INO, &block); - if (retval) { - pctx.errcode = retval; - fix_problem(ctx, PR_3_ERR_LPF_NEW_DIR_BLOCK, &pctx); - return 0; - } - - retval = ext2fs_write_dir_block4(fs, blk, block, 0, ino); - ext2fs_free_mem(&block); - if (retval) { - pctx.errcode = retval; - fix_problem(ctx, PR_3_ERR_LPF_WRITE_BLOCK, &pctx); - return 0; - } - /* * Set up the inode structure */ @@ -509,6 +493,27 @@ skip_new_block: fix_problem(ctx, PR_3_CREATE_LPF_ERROR, &pctx); return 0; } + + /* + * Now let's create the actual data block for the inode. + * Due to metadata_csum, the directory block MUST be written + * after the inode is written to disk! + */ + retval = ext2fs_new_dir_block(fs, ino, EXT2_ROOT_INO, &block); + if (retval) { + pctx.errcode = retval; + fix_problem(ctx, PR_3_ERR_LPF_NEW_DIR_BLOCK, &pctx); + return 0; + } + + retval = ext2fs_write_dir_block4(fs, blk, block, 0, ino); + ext2fs_free_mem(&block); + if (retval) { + pctx.errcode = retval; + fix_problem(ctx, PR_3_ERR_LPF_WRITE_BLOCK, &pctx); + return 0; + } + /* * Finally, create the directory link */ diff --git a/tests/f_rebuild_csum_rootdir/expect.1 b/tests/f_rebuild_csum_rootdir/expect.1 new file mode 100644 index 000000000..4df58f913 --- /dev/null +++ b/tests/f_rebuild_csum_rootdir/expect.1 @@ -0,0 +1,311 @@ +Pass 1: Checking inodes, blocks, and sizes +Pass 2: Checking directory structure +Directory inode 2, block #0, offset 0: directory has no checksum. +Fix? yes + +Directory inode 2, block #0, offset 0: directory corrupted +Salvage? yes + +Missing '.' in directory inode 2. +Fix? yes + +Setting filetype for entry '.' in ??? (2) to 2. +Missing '..' in directory inode 2. +Fix? yes + +Setting filetype for entry '..' in ??? (2) to 2. +Pass 3: Checking directory connectivity +'..' in / (2) is (0), should be / (2). +Fix? yes + +Unconnected directory inode 11 (/???) +Connect to /lost+found? yes + +/lost+found not found. Create? yes + +Pass 3A: Optimizing directories +Pass 4: Checking reference counts +Inode 11 ref count is 3, should be 2. Fix? yes + +Unattached inode 12 +Connect to /lost+found? yes + +Inode 12 ref count is 2, should be 1. Fix? yes + +Unattached inode 13 +Connect to /lost+found? yes + +Inode 13 ref count is 2, should be 1. Fix? yes + +Unattached inode 14 +Connect to /lost+found? yes + +Inode 14 ref count is 2, should be 1. Fix? yes + +Unattached inode 15 +Connect to /lost+found? yes + +Inode 15 ref count is 2, should be 1. Fix? yes + +Unattached inode 16 +Connect to /lost+found? yes + +Inode 16 ref count is 2, should be 1. Fix? yes + +Unattached inode 17 +Connect to /lost+found? yes + +Inode 17 ref count is 2, should be 1. Fix? yes + +Unattached inode 18 +Connect to /lost+found? yes + +Inode 18 ref count is 2, should be 1. Fix? yes + +Unattached inode 19 +Connect to /lost+found? yes + +Inode 19 ref count is 2, should be 1. Fix? yes + +Unattached inode 20 +Connect to /lost+found? yes + +Inode 20 ref count is 2, should be 1. Fix? yes + +Unattached inode 21 +Connect to /lost+found? yes + +Inode 21 ref count is 2, should be 1. Fix? yes + +Unattached inode 22 +Connect to /lost+found? yes + +Inode 22 ref count is 2, should be 1. Fix? yes + +Unattached inode 23 +Connect to /lost+found? yes + +Inode 23 ref count is 2, should be 1. Fix? yes + +Unattached inode 24 +Connect to /lost+found? yes + +Inode 24 ref count is 2, should be 1. Fix? yes + +Unattached inode 25 +Connect to /lost+found? yes + +Inode 25 ref count is 2, should be 1. Fix? yes + +Unattached inode 26 +Connect to /lost+found? yes + +Inode 26 ref count is 2, should be 1. Fix? yes + +Unattached inode 27 +Connect to /lost+found? yes + +Inode 27 ref count is 2, should be 1. Fix? yes + +Unattached inode 28 +Connect to /lost+found? yes + +Inode 28 ref count is 2, should be 1. Fix? yes + +Unattached inode 29 +Connect to /lost+found? yes + +Inode 29 ref count is 2, should be 1. Fix? yes + +Unattached inode 30 +Connect to /lost+found? yes + +Inode 30 ref count is 2, should be 1. Fix? yes + +Unattached inode 31 +Connect to /lost+found? yes + +Inode 31 ref count is 2, should be 1. Fix? yes + +Unattached inode 32 +Connect to /lost+found? yes + +Inode 32 ref count is 2, should be 1. Fix? yes + +Unattached inode 33 +Connect to /lost+found? yes + +Inode 33 ref count is 2, should be 1. Fix? yes + +Unattached inode 34 +Connect to /lost+found? yes + +Inode 34 ref count is 2, should be 1. Fix? yes + +Unattached inode 35 +Connect to /lost+found? yes + +Inode 35 ref count is 2, should be 1. Fix? yes + +Unattached inode 36 +Connect to /lost+found? yes + +Inode 36 ref count is 2, should be 1. Fix? yes + +Unattached inode 37 +Connect to /lost+found? yes + +Inode 37 ref count is 2, should be 1. Fix? yes + +Unattached inode 38 +Connect to /lost+found? yes + +Inode 38 ref count is 2, should be 1. Fix? yes + +Unattached inode 39 +Connect to /lost+found? yes + +Inode 39 ref count is 2, should be 1. Fix? yes + +Unattached inode 40 +Connect to /lost+found? yes + +Inode 40 ref count is 2, should be 1. Fix? yes + +Unattached inode 41 +Connect to /lost+found? yes + +Inode 41 ref count is 2, should be 1. Fix? yes + +Unattached inode 42 +Connect to /lost+found? yes + +Inode 42 ref count is 2, should be 1. Fix? yes + +Unattached inode 43 +Connect to /lost+found? yes + +Inode 43 ref count is 2, should be 1. Fix? yes + +Unattached inode 44 +Connect to /lost+found? yes + +Inode 44 ref count is 2, should be 1. Fix? yes + +Unattached inode 45 +Connect to /lost+found? yes + +Inode 45 ref count is 2, should be 1. Fix? yes + +Unattached inode 46 +Connect to /lost+found? yes + +Inode 46 ref count is 2, should be 1. Fix? yes + +Unattached inode 47 +Connect to /lost+found? yes + +Inode 47 ref count is 2, should be 1. Fix? yes + +Unattached inode 48 +Connect to /lost+found? yes + +Inode 48 ref count is 2, should be 1. Fix? yes + +Unattached inode 49 +Connect to /lost+found? yes + +Inode 49 ref count is 2, should be 1. Fix? yes + +Unattached inode 50 +Connect to /lost+found? yes + +Inode 50 ref count is 2, should be 1. Fix? yes + +Unattached inode 51 +Connect to /lost+found? yes + +Inode 51 ref count is 2, should be 1. Fix? yes + +Unattached inode 52 +Connect to /lost+found? yes + +Inode 52 ref count is 2, should be 1. Fix? yes + +Unattached inode 53 +Connect to /lost+found? yes + +Inode 53 ref count is 2, should be 1. Fix? yes + +Unattached inode 54 +Connect to /lost+found? yes + +Inode 54 ref count is 2, should be 1. Fix? yes + +Unattached inode 55 +Connect to /lost+found? yes + +Inode 55 ref count is 2, should be 1. Fix? yes + +Unattached inode 56 +Connect to /lost+found? yes + +Inode 56 ref count is 2, should be 1. Fix? yes + +Unattached inode 57 +Connect to /lost+found? yes + +Inode 57 ref count is 2, should be 1. Fix? yes + +Unattached inode 58 +Connect to /lost+found? yes + +Inode 58 ref count is 2, should be 1. Fix? yes + +Unattached inode 59 +Connect to /lost+found? yes + +Inode 59 ref count is 2, should be 1. Fix? yes + +Unattached inode 60 +Connect to /lost+found? yes + +Inode 60 ref count is 2, should be 1. Fix? yes + +Unattached inode 61 +Connect to /lost+found? yes + +Inode 61 ref count is 2, should be 1. Fix? yes + +Unattached inode 62 +Connect to /lost+found? yes + +Inode 62 ref count is 2, should be 1. Fix? yes + +Unattached inode 63 +Connect to /lost+found? yes + +Inode 63 ref count is 2, should be 1. Fix? yes + +Unattached inode 64 +Connect to /lost+found? yes + +Inode 64 ref count is 2, should be 1. Fix? yes + +Unattached zero-length inode 65. Clear? yes + +Unattached inode 66 +Connect to /lost+found? yes + +Inode 66 ref count is 2, should be 1. Fix? yes + +Unattached inode 67 +Connect to /lost+found? yes + +Inode 67 ref count is 2, should be 1. Fix? yes + +Pass 5: Checking group summary information + +test_filesys: ***** FILE SYSTEM WAS MODIFIED ***** +test_filesys: 67/512 files (1.5% non-contiguous), 1127/2048 blocks +Exit status is 1 diff --git a/tests/f_rebuild_csum_rootdir/expect.2 b/tests/f_rebuild_csum_rootdir/expect.2 new file mode 100644 index 000000000..033f1bf7c --- /dev/null +++ b/tests/f_rebuild_csum_rootdir/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: 67/512 files (1.5% non-contiguous), 1127/2048 blocks +Exit status is 0 diff --git a/tests/f_rebuild_csum_rootdir/image.gz b/tests/f_rebuild_csum_rootdir/image.gz new file mode 100644 index 0000000000000000000000000000000000000000..a32fd4431a44560b20033d43836000ef22ce977f GIT binary patch literal 12476 zc-rlkc~ld3yT`G<*6menRVcE=zG_hs5KuHCq?HOUDq;ae7Ks8H6)`A~usW>^xS&`R zQ)NjhA}R<1VvqnMZiryQQubxE6bK{`l!U+xGtWEr-gEA~=bm%Vy?@?w^7&_e=b7Kk zFVFm*-}n23acP#$5z5%(v+0WN*;kDYWj*|KrQKTa?I_UeR*+VkR<_aFVbsBl}~ z1@*>9Z_WqIE~gH0zOD+n&mMa{Q=t?L)zytiJ}5^92M2FvxO}<)>HX~qRW8=1_NDh7 zTufG1O!lr_J~!AgI-ou{EA^W@j;o?b^&OTVc{B%NBf>qft<92K$NcTA*4s}csbc4T zzM{s`!fq(uyx*v>HbQ*aXsn|b$w%aG+j1XN5Z$j0%X>bgI6rWRe3fk@s+4yv?)?Ux zN+q^s57t~Q>%q=liAP&Z&$JLke8=ST_YcFr`gwk5_Vcj82dvvzaB5}My&ncqcNVh# z?(Z?o1s=Vbn$1Vo=sbPuM`EjudH3>x<#|8%!8Aeg?pl-8#%pcyI}LN zq|n&zwY}5npu=4$SW-$CP89X&g&FWEsDuFs&e`FADHifgjLknjGZ$ zvF+zz;62M*o3$!#*K-IwJfKIo0`a?T#}gO@uckI9iMT zG5=KUpy<@Dy5DV{edNCRL*y_&L`B^ODk!I?4{_f|wQ?U$aqp7f7{w<^jsJZo&pi8@ zSwJMub%fu{1IMdXs_D}S3T-c&5FHp!98zG?DR!$Jfm#8#l(JblsnI=;qaE_>wIKYs zqizz}8V#tGtI-%&j%06WfoUhY(NqOPZKnQhr}khA@>xd5 z>!=m;UgFjL3YKLSPOi*;?4ojS6=<|9*}u>){T$%?862;AK&$t{MuOF-!gKb zLf~;UBslMSrtmpG=`6UtIA>Q_6rn^^I?ciXuCziB!bNeGtX0iL= z@sa#|-*c$)Fn5(`y-0LypM8bPz1-{ycNghH*I`@FLtg!Se)yyKPWAf^wZ%$)iNM!x z)21eQ_hTM2hZi0$;?;#1c-wV%H#c~!;_rT(o1P;oFe7u9w^!oDZ@29m4l~RZq-XSG z^2lmQ+J_6vIPpi?VqymPkKbD6%*YBxxvA;?V_o4)wm5Odn6Iy$c+;VT+KGP-b9W9O z9UgHg=I^#iJhLqo>D_GraoPIfOSx%uf z8h1Q+GO*-vMt=AU{-YH!p1!u0Z-eMV#S}{pnU5N)WxtEHGvuONBQb z42q5ovMygIme)x}ZcL`kNO^TK%u9ara!Dt9aE*W?4c>t?wRhRa_^Zd03Nko{?=`k` zHK+T3hecw!wsD(ce9Zd#Gj2>8yA<)UN4lbl>=0g9w86srt!$w`;!)%_T-4Do-B}&$ zygh14G18*qEDT1Pdb)2ad<|PGUb+;qhpp{4)ebjzb{FoasFGI_iR8ws@XAzAJ0#n> zpe$}>SN`}(je+sLxsBUhl53hPzsSxQ?;jZnzm?*;oRIbO{4gq#)&&D#@#OV->z3Knv0aT^Y!Cm(>Czsx75SmME^>p^X zi*k$m>boO9YG_e@DqgOD(6A~}I4*et&$k*7EWe57(IxE%7X~Wf#OrTf;--Hp(2ZF@ z;q{2Ts*n)R8G&Lq3ANRLbi?#<2=!G^?p1>c(JB(NHE%)L5{et7!b;bubUQ3~@WK?73XNFP_Wt>S_A3?(q6?MRdp zoY?LNbxprn!dwT+wWb}}>IAJ8x}^O0#cpIm?OO(QXh3>oaJ$G(Uom4Pz^$jm+kx;hI)ef2DPUK`12T`U)7 z6eCBR!0o;eRBzt2k*J}+VvKRd>WWt{c_KgN)V5C`%^%W9) zi&8T1kc7&$!A0H+aB18+9(c@XMReb`Bd$(x@zNG>F?~4#+pE;*^ONJ2QW@80A)bVO z!vlje7uAS02TBach5T8hC^4xUdQXqRdpe6+nJ#10YBlqOkAe!EvlEt`9A^s~;Mtpc zk+&4bpYmBFwTu!1sZZ@h3_*guOW`o1_~daY zT^~p|OUDin)z64PE->@njTiKu9LH`npdlO?q8@J}JN0A9-ib6)#8F}4Tp2o&+(rsl zO;Cb`C&v-XEVQ@-m1VS%ZhEn#@M#UM$f%%r-cRLBq` z-&ZNoN%|LyiykAaFK`I|4$}zZzxG2mcHQlSxs}ixMrVCU1-UqtW9Cd3&Yn>UCg~!P z%FsJyQ+6DbUTje;U^PraNf(ZKXox_312t@RC^>LFjby!+u?owe*K0|D#?ov8XNO6s z7;9WOwN-}VkJ`wM9C}Yl#C;^EvqvRYpWO!5@}qsieG1-K^|) z6eBF`%2mP!M`L|um`c`0UYlb9{e22#X%>=8AG+r^sueSJbiP{Ivh!y7I&Y=C^5fwp zWJg+aa9s5=e@e)dLbMpr3pwAJ1EaK%{4edL2AZ|eKx#BC;aCx@|} zzbuX)JCx^BWL1qEyXG`cL3G*Z&R0-o5`#9<#v}1C1pDLx z8T-+}I_j5-yByo|iFH)x}d(-M6H&7UPfbfoUETuwAtq8%oMRG=J zF_b4#!uh&%0S0Q^CUzB4Czym(Dr1MAkRi91@sKB#dyzm1HP;&7Fc~Nl+HT@OnK>;7 zlr0%?1nASHe@JENy$JWRUm}M7)3s5oo8-Fp69Xso6}zQX)f92Nf0_ zs-W^|8IPkC>x~S-E9)VlGe9GtW#`N zkW@W6W=VeqYnwyeemjJXw>)WNqAmJ zbZ)S0LB6Fe=AepYw@QUf+-oDHwBEQ$+7WLjm{EO>vg6SPu!nxMnoc330qJV2XN^6Q z)xzl{#DiAJpBtd0))C5512l}vt%cM*CLst?F+OO9aH&Nhk)Rh#vCKJ;Vn7C;GQk-= zHaz%wh7gSDcDU1kw5%|~?S3nOxV(2CmA|HuUI$h1*a+vOFBXHMEP!kZ8@GHb0~fVg z=(Xq72y~AIbRz%7COTQI$9Ctz)iko!8dr8zlu_c%Jdn%~A$4`_2z`oe=Wc-~4I;4L z9ZG8I)5z8!RYB=Wl`!00L7|KMz-|o(J|qJ65QajN` zvMe?NBcm9~5~(N;N4U^L=SpiKY@zfG}R-lymwQd{+e`JCEtf;IZ*mt)Yy|W(#!^j-Q84F-}yH3 z`J5hL(ftfzK(=}ckO76h7%rUd{Hp-@PLS*6#fIR^-AG)OT1ZzLEX{%!Lj(hDTp&ig ztJKh+MrwjotjSCDi26t&QLX1k32zlcPa-AGRU-L+nSts9;`h@amRt-8-b$qXKdIaMG8hi{!=PqPMeeNBoPI$E@7(V|6*7A;z|XwjlYixw?fv}n