e2fsck: salvage under-sized dirents by removing them

If the directory processing code ends up pointing to a directory entry
that's so close to the end of the block that there's not even space
for a rec_len/name_len, just substitute dummy values that will force
e2fsck to extend the previous entry to cover the remaining space.  We
can't use the helper methods to extract rec_len because that's reading
off the end of the buffer.

This isn't an issue with non-inline directories because the directory
check buffer is zero-extended so that fsck won't blow up.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c
index 26db2e6..78bc24e 100644 (file)
--- a/e2fsck/pass2.c
+++ b/e2fsck/pass2.c
@@ -672,18 +672,29 @@ static void salvage_directory(ext2_filsys fs,
        char    *cp = (char *) dirent;
        int left;
        unsigned int rec_len, prev_rec_len;
-       unsigned int name_len = ext2fs_dirent_name_len(dirent);
+       unsigned int name_len;
 
-       (void) ext2fs_get_rec_len(fs, dirent, &rec_len);
+       /*
+        * If the space left for the entry is too small to be an entry,
+        * we can't access dirent's fields, so plumb in the values needed
+        * so that the previous entry absorbs this one.
+        */
+       if (block_len - *offset < EXT2_DIR_ENTRY_HEADER_LEN) {
+               name_len = 0;
+               rec_len = block_len - *offset;
+       } else {
+               name_len = ext2fs_dirent_name_len(dirent);
+               (void) ext2fs_get_rec_len(fs, dirent, &rec_len);
+       }
        left = block_len - *offset - rec_len;
 
        /*
         * Special case of directory entry of size 8: copy what's left
         * of the directory block up to cover up the invalid hole.
         */
-       if ((left >= 12) && (rec_len == 8)) {
-               memmove(cp, cp+8, left);
-               memset(cp + left, 0, 8);
+       if ((left >= 12) && (rec_len == EXT2_DIR_ENTRY_HEADER_LEN)) {
+               memmove(cp, cp+EXT2_DIR_ENTRY_HEADER_LEN, left);
+               memset(cp + left, 0, EXT2_DIR_ENTRY_HEADER_LEN);
                return;
        }
        /*
@@ -692,8 +703,8 @@ static void salvage_directory(ext2_filsys fs,
         * record length.
         */
        if ((left < 0) &&
-           ((int) rec_len + left > 8) &&
-           ((int) name_len + 8 <= (int) rec_len + left) &&
+           ((int) rec_len + left > EXT2_DIR_ENTRY_HEADER_LEN) &&
+           ((int) name_len + EXT2_DIR_ENTRY_HEADER_LEN <= (int) rec_len + left) &&
            dirent->inode <= fs->super->s_inodes_count &&
            strnlen(dirent->name, name_len) == name_len) {
                (void) ext2fs_set_rec_len(fs, (int) rec_len + left, dirent);
@@ -928,6 +939,7 @@ static int check_dir_block(ext2_filsys fs,
 
        ehandler_operation(_("reading directory block"));
        if (inline_data_size) {
+               memset(buf, 0, fs->blocksize - inline_data_size);
                cd->pctx.errcode = ext2fs_inline_data_get(fs, ino, 0, buf, 0);
                if (cd->pctx.errcode)
                        goto inline_read_fail;
@@ -1081,15 +1093,20 @@ skip_checksum:
                problem = 0;
                if (!inline_data_size || dot_state > 1) {
                        dirent = (struct ext2_dir_entry *) (buf + offset);
-                       (void) ext2fs_get_rec_len(fs, dirent, &rec_len);
+                       /*
+                        * If there's not even space for the entry header,
+                        * force salvaging this dir.
+                        */
+                       if (max_block_size - offset < EXT2_DIR_ENTRY_HEADER_LEN)
+                               rec_len = EXT2_DIR_REC_LEN(1);
+                       else
+                               (void) ext2fs_get_rec_len(fs, dirent, &rec_len);
                        cd->pctx.dirent = dirent;
                        cd->pctx.num = offset;
-                       if (((offset + rec_len) > fs->blocksize) ||
-                           (inline_data_size > 0 &&
-                            (offset + rec_len) > inline_data_size) ||
+                       if ((offset + rec_len > max_block_size) ||
                            (rec_len < 12) ||
                            ((rec_len % 4) != 0) ||
-                           ((ext2fs_dirent_name_len(dirent) + 8) > rec_len)) {
+                           ((ext2fs_dirent_name_len(dirent) + EXT2_DIR_ENTRY_HEADER_LEN) > rec_len)) {
                                if (fix_problem(ctx, PR_2_DIR_CORRUPTED,
                                                &cd->pctx)) {
 #ifdef WORDS_BIGENDIAN
@@ -1098,7 +1115,7 @@ skip_checksum:
                                         * swap routine finds a rec_len that it
                                         * doesn't like, it continues
                                         * processing the block as if rec_len
-                                        * == 8.  This means that the name
+                                        * == EXT2_DIR_ENTRY_HEADER_LEN.  This means that the name
                                         * field gets byte swapped, which means
                                         * that salvage will not detect the
                                         * correct name length (unless the name
@@ -1112,11 +1129,11 @@ skip_checksum:
                                         * the salvaged dirent.
                                         */
                                        int need_reswab = 0;
-                                       if (rec_len < 8 || rec_len % 4) {
+                                       if (rec_len < EXT2_DIR_ENTRY_HEADER_LEN || rec_len % 4) {
                                                need_reswab = 1;
                                                ext2fs_dirent_swab_in2(fs,
-                                                       ((char *)dirent) + 8,
-                                                       max_block_size - offset - 8,
+                                                       ((char *)dirent) + EXT2_DIR_ENTRY_HEADER_LEN,
+                                                       max_block_size - offset - EXT2_DIR_ENTRY_HEADER_LEN,
                                                        0);
                                        }
 #endif

diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
index 953cbc4..10cb650 100644 (file)
--- a/lib/ext2fs/ext2_fs.h
+++ b/lib/ext2fs/ext2_fs.h
@@ -875,9 +875,12 @@ struct ext2_dir_entry_tail {
  *
  * NOTE: It must be a multiple of 4
  */
+#define EXT2_DIR_ENTRY_HEADER_LEN      8
 #define EXT2_DIR_PAD                   4
 #define EXT2_DIR_ROUND                 (EXT2_DIR_PAD - 1)
-#define EXT2_DIR_REC_LEN(name_len)     (((name_len) + 8 + EXT2_DIR_ROUND) & \
+#define EXT2_DIR_REC_LEN(name_len)     (((name_len) + \
+                                         EXT2_DIR_ENTRY_HEADER_LEN + \
+                                         EXT2_DIR_ROUND) & \
                                         ~EXT2_DIR_ROUND)
 
 /*

diff --git a/tests/f_trunc_dirent_header/expect.1 b/tests/f_trunc_dirent_header/expect.1
new file mode 100644 (file)
index 0000000..33ce473
--- /dev/null
+++ b/tests/f_trunc_dirent_header/expect.1
@@ -0,0 +1,15 @@
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Directory inode 15, block #0, offset 4092: directory corrupted
+Salvage? yes
+
+Directory inode 13, block #1, offset 28: directory corrupted
+Salvage? 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: 32/128 files (0.0% non-contiguous), 18/512 blocks
+Exit status is 1

diff --git a/tests/f_trunc_dirent_header/expect.2 b/tests/f_trunc_dirent_header/expect.2
new file mode 100644 (file)
index 0000000..df81c9d
--- /dev/null
+++ b/tests/f_trunc_dirent_header/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: 32/128 files (0.0% non-contiguous), 18/512 blocks
+Exit status is 0

diff --git a/tests/f_trunc_dirent_header/image.gz b/tests/f_trunc_dirent_header/image.gz
new file mode 100644 (file)
index 0000000..9c59bf4
Binary files /dev/null and b/tests/f_trunc_dirent_header/image.gz differ

diff --git a/tests/f_trunc_dirent_header/name b/tests/f_trunc_dirent_header/name
new file mode 100644 (file)
index 0000000..e244dbf
--- /dev/null
+++ b/tests/f_trunc_dirent_header/name
@@ -0,0 +1 @@
+no space for dirent header at end of buf