From f76344fb6f9439ecd5060943930e73cd3b3dd9fe Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Mon, 4 Jul 2005 12:53:36 -0500 Subject: [PATCH] [BUGFIX]: E2fsck will segfault on disconnected inode with extended attribute(s) This was actually caused by two bugs. The first bug is that if the inode has been fully fixed up, the code will attempt to remove the inode from the inode_bad_map without checking to see if this bitmap is present. Since it is cleared at the end of pass 2, if e2fsck_process_bad_inode is called in pass 4 (as it is for disconnected inodes), this would result in a core dump. The first bug was mostly hidden by a second bug, which caused e2fsck_process_bad_inode() to consider all inodes without an extended attribute to be not fixed. Note: This bug was introduced in e2fsprogs 1.36. (Addresses Debian Bug: #316736) --- e2fsck/ChangeLog | 16 ++++++++ e2fsck/pass2.c | 42 ++++++++++--------- tests/f_bad_disconnected_inode/expect.1 | 69 ++++++++++++++++++++++++++++++++ tests/f_bad_disconnected_inode/expect.2 | 7 ++++ tests/f_bad_disconnected_inode/image.gz | Bin 0 -> 1490 bytes tests/f_bad_disconnected_inode/name | 1 + 6 files changed, 115 insertions(+), 20 deletions(-) create mode 100644 tests/f_bad_disconnected_inode/expect.1 create mode 100644 tests/f_bad_disconnected_inode/expect.2 create mode 100644 tests/f_bad_disconnected_inode/image.gz create mode 100644 tests/f_bad_disconnected_inode/name diff --git a/e2fsck/ChangeLog b/e2fsck/ChangeLog index 49e3424..d13aec9 100644 --- a/e2fsck/ChangeLog +++ b/e2fsck/ChangeLog @@ -1,3 +1,19 @@ +2005-07-04 Theodore Ts'o + + * pass2.c (e2fsck_process_bad_inode): Fixed bug which could cause + e2fsck to core dump if a disconnected inode contained an + extended attribute. This was actually caused by two bugs. + The first bug is that if the inode has been fully fixed + up, the code will attempt to remove the inode from the + inode_bad_map without checking to see if this bitmap is + present. Since it is cleared at the end of pass 2, if + e2fsck_process_bad_inode is called in pass 4 (as it is for + disconnected inodes), this would result in a core dump. + This bug was mostly hidden by a second bug, which caused + e2fsck_process_bad_inode() to consider all inodes without + an extended attribute to be not fixed. (Addresses Debian + Bug: #316736) + 2006-06-30 Theodore Ts'o * Release of E2fsprogs 1.38 diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c index 52fdcba..18ea42c 100644 --- a/e2fsck/pass2.c +++ b/e2fsck/pass2.c @@ -1184,27 +1184,29 @@ extern int e2fsck_process_bad_inode(e2fsck_t ctx, ext2_ino_t dir, pctx.inode = &inode; if (inode.i_file_acl && - !(fs->super->s_feature_compat & EXT2_FEATURE_COMPAT_EXT_ATTR) && - fix_problem(ctx, PR_2_FILE_ACL_ZERO, &pctx)) { - inode.i_file_acl = 0; + !(fs->super->s_feature_compat & EXT2_FEATURE_COMPAT_EXT_ATTR)) { + if (fix_problem(ctx, PR_2_FILE_ACL_ZERO, &pctx)) { + inode.i_file_acl = 0; #ifdef EXT2FS_ENABLE_SWAPFS - /* - * This is a special kludge to deal with long symlinks - * on big endian systems. i_blocks had already been - * decremented earlier in pass 1, but since i_file_acl - * hadn't yet been cleared, ext2fs_read_inode() - * assumed that the file was short symlink and would - * not have byte swapped i_block[0]. Hence, we have - * to byte-swap it here. - */ - if (LINUX_S_ISLNK(inode.i_mode) && - (fs->flags & EXT2_FLAG_SWAP_BYTES) && - (inode.i_blocks == fs->blocksize >> 9)) - inode.i_block[0] = ext2fs_swab32(inode.i_block[0]); + /* + * This is a special kludge to deal with long + * symlinks on big endian systems. i_blocks + * had already been decremented earlier in + * pass 1, but since i_file_acl hadn't yet + * been cleared, ext2fs_read_inode() assumed + * that the file was short symlink and would + * not have byte swapped i_block[0]. Hence, + * we have to byte-swap it here. + */ + if (LINUX_S_ISLNK(inode.i_mode) && + (fs->flags & EXT2_FLAG_SWAP_BYTES) && + (inode.i_blocks == fs->blocksize >> 9)) + inode.i_block[0] = ext2fs_swab32(inode.i_block[0]); #endif - inode_modified++; - } else - not_fixed++; + inode_modified++; + } else + not_fixed++; + } if (!LINUX_S_ISDIR(inode.i_mode) && !LINUX_S_ISREG(inode.i_mode) && !LINUX_S_ISCHR(inode.i_mode) && !LINUX_S_ISBLK(inode.i_mode) && @@ -1302,7 +1304,7 @@ extern int e2fsck_process_bad_inode(e2fsck_t ctx, ext2_ino_t dir, if (inode_modified) e2fsck_write_inode(ctx, ino, &inode, "process_bad_inode"); - if (!not_fixed) + if (!not_fixed && ctx->inode_bad_map) ext2fs_unmark_inode_bitmap(ctx->inode_bad_map, ino); return 0; } diff --git a/tests/f_bad_disconnected_inode/expect.1 b/tests/f_bad_disconnected_inode/expect.1 new file mode 100644 index 0000000..deba68b --- /dev/null +++ b/tests/f_bad_disconnected_inode/expect.1 @@ -0,0 +1,69 @@ +Pass 1: Checking inodes, blocks, and sizes +Pass 2: Checking directory structure +Pass 3: Checking directory connectivity +/lost+found not found. Create? yes + +Pass 4: Checking reference counts +Inode 2 ref count is 4, should be 3. Fix? yes + +i_file_acl for inode 13 (...) is 4218798225, should be zero. +Clear? yes + +Inode 13 (...) has invalid mode (0117003). +Clear? yes + +i_file_acl for inode 14 (...) is 2892851642, should be zero. +Clear? yes + +Inode 14 (...) has invalid mode (0154247). +Clear? yes + +i_file_acl for inode 15 (...) is 1143674715, should be zero. +Clear? yes + +Inode 15 (...) has invalid mode (074044). +Clear? yes + +i_file_acl for inode 16 (...) is 2007517039, should be zero. +Clear? yes + +i_faddr for inode 16 (...) is 1003914917, should be zero. +Clear? yes + +i_frag for inode 16 (...) is 42, should be zero. +Clear? yes + +i_fsize for inode 16 (...) is 245, should be zero. +Clear? yes + +Unattached inode 16 +Connect to /lost+found? yes + +Inode 16 ref count is 5925, should be 1. Fix? yes + +Pass 5: Checking group summary information +Block bitmap differences: -(9--19) +Fix? yes + +Free blocks count wrong for group #0 (79, counted=91). +Fix? yes + +Free blocks count wrong (79, counted=91). +Fix? yes + +Inode bitmap differences: +16 +Fix? yes + +Free inodes count wrong for group #0 (7, counted=4). +Fix? yes + +Directories count wrong for group #0 (3, counted=2). +Fix? yes + +Free inodes count wrong (7, counted=4). +Fix? yes + + +test_filesys: ***** FILE SYSTEM WAS MODIFIED ***** +test_filesys: 12/16 files (0.0% non-contiguous), 9/100 blocks +Exit status is 1 diff --git a/tests/f_bad_disconnected_inode/expect.2 b/tests/f_bad_disconnected_inode/expect.2 new file mode 100644 index 0000000..1739210 --- /dev/null +++ b/tests/f_bad_disconnected_inode/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/16 files (0.0% non-contiguous), 9/100 blocks +Exit status is 0 diff --git a/tests/f_bad_disconnected_inode/image.gz b/tests/f_bad_disconnected_inode/image.gz new file mode 100644 index 0000000000000000000000000000000000000000..8f2b3d94d436791681383d9c23b6f016ab229fb9 GIT binary patch literal 1490 zcmc)8`9Bj1008je)2m26UG#CJ<_I-H_&8dkOKc%*ipjCB zs07{PI4ItL6YhKcbkX3pQIfAU)&&YgtC4*HAKvFprrh#KLtRV!mYXKet3tSlJA5D`~|PJyh$JUmn} z#Mb_1gutl+*Z+diOrX^`g{UlpwxUi(6#DS`n|_D+?M|OLm3(J7_=mLLJ5FWn<(_#Z zGRVB@&@r@<9*$@Yb6A?81E=066PUtfymM}8v2TAEbfeS25 zIHIznUK8YGQoRQijU*Ql;%;q6SbI4!__=JmBg-?&A>tG69)BL;JcCo``UV4I4q3`~_S~IX`tEM8zkG6VxyPF{J zm(x{f3B$fdwe#Lbr@dPIEHE2tGh*4SI2qyuwf0d4S2QW|$|ejvuRyUYkvKhf7j2ec zukWA`jS~6Uibhezg_?sNXwv!uF)$??ZvwqgZ>K{x_UxT+-u)uW6-F5Od`-LV?T=W* zxx)3J6DlMVS2w(g?KHJ2*d@-+H!j`ygQsK1n=vzWk&G9w_pRT`GqReC zSzMee8xLC9y@$o_h0x@KxTWD{O^(i3_u2E=lNsi|PqGIZh%K?n!)`G7nRT01dD=_w z{P?J4yXq>floYhfFE6+_D}z3Tg09^23M@9R|7N85W~Jz!%iiAE-w#a-An(O z^9cdAKS*V5yDtfc47AqbN)G7ax-7{@i4;!S^NsWi@lk7uOp!#2EdeaV0k&=e_S6A~ W3ja0W|1m|`lWD@wS_3kq0RI9{vEm5; literal 0 HcmV?d00001 diff --git a/tests/f_bad_disconnected_inode/name b/tests/f_bad_disconnected_inode/name new file mode 100644 index 0000000..d09edf1 --- /dev/null +++ b/tests/f_bad_disconnected_inode/name @@ -0,0 +1 @@ +Disconnected inode with bad fields -- 1.8.3.1