[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)

diff --git a/e2fsck/ChangeLog b/e2fsck/ChangeLog
index 49e3424..d13aec9 100644 (file)
--- a/e2fsck/ChangeLog
+++ b/e2fsck/ChangeLog
@@ -1,3 +1,19 @@
+2005-07-04  Theodore Ts'o  <tytso@mit.edu>
+
+       * 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  <tytso@mit.edu>
 
        * Release of E2fsprogs 1.38

diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c
index 52fdcba..18ea42c 100644 (file)
--- 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 (file)
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 (file)
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 (file)
index 0000000..8f2b3d9
Binary files /dev/null and b/tests/f_bad_disconnected_inode/image.gz differ

diff --git a/tests/f_bad_disconnected_inode/name b/tests/f_bad_disconnected_inode/name
new file mode 100644 (file)
index 0000000..d09edf1
--- /dev/null
+++ b/tests/f_bad_disconnected_inode/name
@@ -0,0 +1 @@
+Disconnected inode with bad fields