From: Darrick J. Wong Date: Thu, 29 Jan 2015 16:09:07 +0000 (-0500) Subject: e2fsck: salvage under-sized dirents by removing them X-Git-Tag: v1.43-WIP-2015-05-18~76 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4a3dc1f0b670960acd570ee64acb436c254135c8;p=thirdparty%2Fe2fsprogs.git 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 Signed-off-by: Theodore Ts'o --- diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c index 26db2e634..78bc24e39 100644 --- 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 953cbc4a6..10cb65067 100644 --- 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 index 000000000..33ce473df --- /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 index 000000000..df81c9d7f --- /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 index 000000000..9c59bf4a9 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 index 000000000..e244dbf74 --- /dev/null +++ b/tests/f_trunc_dirent_header/name @@ -0,0 +1 @@ +no space for dirent header at end of buf