From 52f81b328fb655e220d14fe9b712dacc2a8fa17b Mon Sep 17 00:00:00 2001 From: Andreas Dilger Date: Fri, 13 Jul 2012 20:33:01 -0600 Subject: [PATCH] LU-1540 e2fsck: add missing symlink NUL terminator If a long symbolic link target is written into an external block without a NUL terminator, its length is decided by the inode's size. Make symlink check add a NUL termination in such cases if needed. Such faulty symlinks were generated by osd-ldiskfs on the MDS until Lustre 2.1.3 and Lustre 2.3. The in-kernel code would handle such unterminated symlinks correctly, since it used the inode size to determine the symlink length, but e2fsck would assume the symlink is broken if there wasn't a trailing NUL. LU-2627 e2fsck: check_symlink() SIGSEGV Since e2fsck_pass1_check_symlink() calls into check_symlink() with pctx == NULL, we should use 'ino' instead of 'pctx->ino' in check_symlink(). Signed-off-by: Bobi Jam Change-Id: If9c16f96d0655d5a886ef607f1f47ced6176f8d8 Signed-off-by: Andreas Dilger Change-Id: I4419b30f1adb4a7d273796a936427aa351510213 --- e2fsck/e2fsck.h | 2 +- e2fsck/pass1.c | 47 ++++++++++++++++++++++++++++++++----------- e2fsck/pass2.c | 4 ++-- e2fsck/problem.c | 5 +++++ e2fsck/problem.h | 3 +++ tests/f_badsymlinks/expect.1 | 16 +++++++-------- tests/f_badsymlinks/expect.2 | 2 +- tests/f_badsymlinks2/expect.1 | 27 ++++++++++--------------- tests/f_badsymlinks2/expect.2 | 2 +- 9 files changed, 66 insertions(+), 42 deletions(-) diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h index ed2410d..f7c3b60 100644 --- a/e2fsck/e2fsck.h +++ b/e2fsck/e2fsck.h @@ -713,7 +713,7 @@ extern errcode_t e2fsck_setup_icount(e2fsck_t ctx, const char *icount_name, extern void e2fsck_use_inode_shortcuts(e2fsck_t ctx, int use_shortcuts); extern int e2fsck_pass1_check_device_inode(ext2_filsys fs, struct ext2_inode *inode); -extern int e2fsck_pass1_check_symlink(ext2_filsys fs, ext2_ino_t ino, +extern int e2fsck_pass1_check_symlink(e2fsck_t ctx, ext2_ino_t ino, struct ext2_inode *inode, char *buf); extern void e2fsck_clear_inode(e2fsck_t ctx, ext2_ino_t ino, struct ext2_inode *inode, int restart_flag, diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c index 904cc07..e4f3aa1 100644 --- a/e2fsck/pass1.c +++ b/e2fsck/pass1.c @@ -179,11 +179,12 @@ int e2fsck_pass1_check_device_inode(ext2_filsys fs EXT2FS_ATTR((unused)), * Check to make sure a symlink inode is real. Returns 1 if the symlink * checks out, 0 if not. */ -int e2fsck_pass1_check_symlink(ext2_filsys fs, ext2_ino_t ino, - struct ext2_inode *inode, char *buf) +static int check_symlink(e2fsck_t ctx, struct problem_context *pctx, + ext2_ino_t ino, struct ext2_inode *inode, char *buf) { unsigned int buflen; unsigned int len; + blk64_t blk; if ((inode->i_size_high || inode->i_size == 0) || (inode->i_flags & EXT2_INDEX_FL)) @@ -194,7 +195,7 @@ int e2fsck_pass1_check_symlink(ext2_filsys fs, ext2_ino_t ino, if (inode->i_flags & EXT4_EXTENTS_FL) return 0; - if (ext2fs_inline_data_size(fs, ino, &inline_size)) + if (ext2fs_inline_data_size(ctx->fs, ino, &inline_size)) return 0; if (inode->i_size != inline_size) return 0; @@ -211,11 +212,10 @@ int e2fsck_pass1_check_symlink(ext2_filsys fs, ext2_ino_t ino, ext2_extent_handle_t handle; struct ext2_extent_info info; struct ext2fs_extent extent; - blk64_t blk; int i; if (inode->i_flags & EXT4_EXTENTS_FL) { - if (ext2fs_extent_open2(fs, ino, inode, &handle)) + if (ext2fs_extent_open2(ctx->fs, ino, inode, &handle)) return 0; if (ext2fs_extent_get_info(handle, &info) || (info.num_entries != 1) || @@ -240,29 +240,53 @@ int e2fsck_pass1_check_symlink(ext2_filsys fs, ext2_ino_t ino, return 0; } - if (blk < fs->super->s_first_data_block || - blk >= ext2fs_blocks_count(fs->super)) + if (blk < ctx->fs->super->s_first_data_block || + blk >= ext2fs_blocks_count(ctx->fs->super)) return 0; - if (io_channel_read_blk64(fs->io, blk, 1, buf)) + if (io_channel_read_blk64(ctx->fs->io, blk, 1, buf)) return 0; - buflen = fs->blocksize; + buflen = ctx->fs->blocksize; } if (inode->i_flags & EXT4_ENCRYPT_FL) len = ext2fs_le16_to_cpu(*(__u16 *)buf) + 2; - else + else { len = strnlen(buf, buflen); + /* Add missing NUL terminator at end of symlink (LU-1540), + * but only offer to fix this in pass1, not from pass2. */ + if (len > inode->i_size && pctx != NULL && + fix_problem(ctx, PR_1_SYMLINK_NUL, pctx)) { + buf[inode->i_size] = '\0'; + if (ext2fs_is_fast_symlink(inode)) { + e2fsck_write_inode(ctx, ino, + inode, "check_ext_attr"); + } else { + if (io_channel_write_blk64(ctx->fs->io, + blk, 1, buf)) + return 0; + } + len = inode->i_size; + } + } + if (len >= buflen) return 0; if (len != inode->i_size) return 0; + return 1; } +int e2fsck_pass1_check_symlink(e2fsck_t ctx, ext2_ino_t ino, + struct ext2_inode *inode, char *buf) +{ + return check_symlink(ctx, NULL, ino, inode, buf); +} + /* * If the extents or inlinedata flags are set on the inode, offer to clear 'em. */ @@ -2486,8 +2510,7 @@ void e2fsck_pass1_run(e2fsck_t ctx) check_size(ctx, &pctx); ctx->fs_blockdev_count++; } else if (LINUX_S_ISLNK (inode->i_mode) && - e2fsck_pass1_check_symlink(fs, ino, inode, - block_buf)) { + check_symlink(ctx, &pctx, ino, inode, block_buf)) { check_immutable(ctx, &pctx); ctx->fs_symlinks_count++; if (inode->i_flags & EXT4_INLINE_DATA_FL) { diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c index 8b5ea57..68e81d1 100644 --- a/e2fsck/pass2.c +++ b/e2fsck/pass2.c @@ -2054,8 +2054,8 @@ int e2fsck_process_bad_inode(e2fsck_t ctx, ext2_ino_t dir, else if (LINUX_S_ISSOCK(inode.i_mode) && !e2fsck_pass1_check_device_inode(fs, &inode)) problem = PR_2_BAD_SOCKET; - else if (LINUX_S_ISLNK(inode.i_mode) - && !e2fsck_pass1_check_symlink(fs, ino, &inode, buf)) { + else if (LINUX_S_ISLNK(inode.i_mode) && + !e2fsck_pass1_check_symlink(ctx, ino, &inode, buf)) { problem = PR_2_INVALID_SYMLINK; } diff --git a/e2fsck/problem.c b/e2fsck/problem.c index 75dfeec..16c858e 100644 --- a/e2fsck/problem.c +++ b/e2fsck/problem.c @@ -1323,6 +1323,11 @@ static struct e2fsck_problem problem_table[] = { N_("@i %i creation time (%t) invalid.\n"), PROMPT_CLEAR, PR_PREEN_OK | PR_NO_OK }, + /* Bad extended attribute value in inode */ + { PR_1_SYMLINK_NUL, + N_("@i %i symlink missing NUL terminator. "), + PROMPT_FIX, 0}, + /* Failed to goto block group */ { PR_1_SCAN_GOTO, N_("failed to goto block group"), diff --git a/e2fsck/problem.h b/e2fsck/problem.h index 1ee9f5f..a379c8c 100644 --- a/e2fsck/problem.h +++ b/e2fsck/problem.h @@ -750,6 +750,9 @@ struct problem_context { /* invalid inode creation time */ #define PR_1_CRTIME_BAD 0x010094 +/* Symlink missing NUL terminator */ +#define PR_1_SYMLINK_NUL 0x010095 + /* Failed to goto block group */ #define PR_1_SCAN_GOTO 0x0100A0 diff --git a/tests/f_badsymlinks/expect.1 b/tests/f_badsymlinks/expect.1 index b28b57d..f300fca 100644 --- a/tests/f_badsymlinks/expect.1 +++ b/tests/f_badsymlinks/expect.1 @@ -1,4 +1,8 @@ Pass 1: Checking inodes, blocks, and sizes +Inode 13 symlink missing NUL terminator. Fix? yes + +Inode 14 symlink missing NUL terminator. Fix? yes + Special (device/socket/fifo/symlink) file (inode 18) has immutable or append-only flag set. Clear? yes @@ -19,12 +23,6 @@ Pass 2: Checking directory structure Symlink /empty_link (inode #17) is invalid. Clear? yes -Symlink /long_fastlink (inode #13) is invalid. -Clear? yes - -Symlink /long_link (inode #14) is invalid. -Clear? yes - Symlink /high_fastlink (inode #15) is invalid. Clear? yes @@ -49,13 +47,13 @@ Clear? yes Pass 3: Checking directory connectivity Pass 4: Checking reference counts Pass 5: Checking group summary information -Free blocks count wrong for group #0 (1000, counted=1001). +Free blocks count wrong for group #0 (999, counted=1000). Fix? yes -Free blocks count wrong (1000, counted=1001). +Free blocks count wrong (999, counted=1000). Fix? yes test_filesys: ***** FILE SYSTEM WAS MODIFIED ***** -test_filesys: 13/32 files (7.7% non-contiguous), 23/1024 blocks +test_filesys: 15/32 files (6.7% non-contiguous), 24/1024 blocks Exit status is 1 diff --git a/tests/f_badsymlinks/expect.2 b/tests/f_badsymlinks/expect.2 index 1499551..f361d8d 100644 --- a/tests/f_badsymlinks/expect.2 +++ b/tests/f_badsymlinks/expect.2 @@ -3,5 +3,5 @@ Pass 2: Checking directory structure Pass 3: Checking directory connectivity Pass 4: Checking reference counts Pass 5: Checking group summary information -test_filesys: 13/32 files (0.0% non-contiguous), 23/1024 blocks +test_filesys: 15/32 files (0.0% non-contiguous), 24/1024 blocks Exit status is 0 diff --git a/tests/f_badsymlinks2/expect.1 b/tests/f_badsymlinks2/expect.1 index 939edce..3e059ea 100644 --- a/tests/f_badsymlinks2/expect.1 +++ b/tests/f_badsymlinks2/expect.1 @@ -1,17 +1,21 @@ Pass 1: Checking inodes, blocks, and sizes +Inode 15 symlink missing NUL terminator. Fix? yes + +Inode 19 symlink missing NUL terminator. Fix? yes + +Inode 40 symlink missing NUL terminator. Fix? yes + +Inode 44 symlink missing NUL terminator. Fix? yes + +Inode 65 symlink missing NUL terminator. Fix? yes + Pass 2: Checking directory structure Symlink /default/empty (inode #13) is invalid. Clear? yes -Symlink /default/fast_isize_too_small (inode #15) is invalid. -Clear? yes - Symlink /default/fast_isize_too_large (inode #16) is invalid. Clear? yes -Symlink /default/slow_isize_too_small (inode #19) is invalid. -Clear? yes - Symlink /default/slow_isize_too_large (inode #20) is invalid. Clear? yes @@ -45,15 +49,9 @@ Clear? yes Symlink /extents/empty (inode #38) is invalid. Clear? yes -Symlink /extents/fast_isize_too_small (inode #40) is invalid. -Clear? yes - Symlink /extents/fast_isize_too_large (inode #41) is invalid. Clear? yes -Symlink /extents/slow_isize_too_small (inode #44) is invalid. -Clear? yes - Symlink /extents/slow_isize_too_large (inode #45) is invalid. Clear? yes @@ -87,9 +85,6 @@ Clear? yes Symlink /inline_data/empty (inode #63) is invalid. Clear? yes -Symlink /inline_data/fast_isize_too_small (inode #65) is invalid. -Clear? yes - Symlink /inline_data/fast_isize_too_large (inode #66) is invalid. Clear? yes @@ -110,5 +105,5 @@ Pass 4: Checking reference counts Pass 5: Checking group summary information test_filesys: ***** FILE SYSTEM WAS MODIFIED ***** -test_filesys: 36/80 files (0.0% non-contiguous), 31/150 blocks +test_filesys: 41/80 files (0.0% non-contiguous), 33/150 blocks Exit status is 1 diff --git a/tests/f_badsymlinks2/expect.2 b/tests/f_badsymlinks2/expect.2 index 3da98a8..dfd000a 100644 --- a/tests/f_badsymlinks2/expect.2 +++ b/tests/f_badsymlinks2/expect.2 @@ -3,5 +3,5 @@ Pass 2: Checking directory structure Pass 3: Checking directory connectivity Pass 4: Checking reference counts Pass 5: Checking group summary information -test_filesys: 36/80 files (0.0% non-contiguous), 31/150 blocks +test_filesys: 41/80 files (0.0% non-contiguous), 33/150 blocks Exit status is 0 -- 1.8.3.1