From: Andreas Dilger Date: Sat, 14 Jul 2012 02:33:01 +0000 (-0600) Subject: LU-1540 e2fsck: add missing symlink NUL terminator X-Git-Tag: v1.42.13.wc6~26 X-Git-Url: https://git.whamcloud.com/tools/e2fsprogs.git/?a=commitdiff_plain;h=64ac23537ea19d91ecabe52129bb7fa73b1c9043;p=tools%2Fe2fsprogs.git 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 --- diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h index a3e6c63..4aa76c3 100644 --- a/e2fsck/e2fsck.h +++ b/e2fsck/e2fsck.h @@ -517,7 +517,7 @@ extern void e2fsck_setup_tdb_icount(e2fsck_t ctx, int flags, 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 9bc35ee..9584a0c 100644 --- a/e2fsck/pass1.c +++ b/e2fsck/pass1.c @@ -169,73 +169,96 @@ 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 len; - int i; - blk64_t blocks; - ext2_extent_handle_t handle; - struct ext2_extent_info info; - struct ext2fs_extent extent; + blk64_t block, num_blocks; + unsigned len, limit; if ((inode->i_size_high || inode->i_size == 0) || (inode->i_flags & EXT2_INDEX_FL)) return 0; - if (inode->i_flags & EXT4_EXTENTS_FL) { - if (inode->i_size > fs->blocksize) + num_blocks = ext2fs_inode_data_blocks2(ctx->fs, inode); + if (inode->i_flags & EXT4_EXTENTS_FL) { /* in extent-mapped block */ + struct ext2_extent_info info; + ext2_extent_handle_t handle; + struct ext2fs_extent extent; + + if (inode->i_size > ctx->fs->blocksize) return 0; - if (ext2fs_extent_open2(fs, ino, inode, &handle)) + if (ext2fs_extent_open2(ctx->fs, ino, inode, &handle)) return 0; - i = 0; if (ext2fs_extent_get_info(handle, &info) || (info.num_entries != 1) || - (info.max_depth != 0)) - goto exit_extent; - if (ext2fs_extent_get(handle, EXT2_EXTENT_ROOT, &extent) || + (info.max_depth != 0) || + + ext2fs_extent_get(handle, EXT2_EXTENT_ROOT, &extent) || (extent.e_lblk != 0) || - (extent.e_len != 1) || - (extent.e_pblk < fs->super->s_first_data_block) || - (extent.e_pblk >= ext2fs_blocks_count(fs->super))) - goto exit_extent; - i = 1; - exit_extent: + (extent.e_len != 1)) { + ext2fs_extent_free(handle); + return 0; + } + + block = extent.e_pblk; + limit = ctx->fs->blocksize; + ext2fs_extent_free(handle); - return i; - } + } else if (num_blocks > 0) { /* in block-mapped block */ + int i; - blocks = ext2fs_inode_data_blocks2(fs, inode); - if (blocks) { - if ((inode->i_size >= fs->blocksize) || - (blocks != fs->blocksize >> 9) || - (inode->i_block[0] < fs->super->s_first_data_block) || - (inode->i_block[0] >= ext2fs_blocks_count(fs->super))) - return 0; + block = inode->i_block[0]; + limit = ctx->fs->blocksize; for (i = 1; i < EXT2_N_BLOCKS; i++) if (inode->i_block[i]) return 0; + } else { /* inside inode i_block[] */ + block = 0; + limit = sizeof(inode->i_block); + } - if (io_channel_read_blk64(fs->io, inode->i_block[0], 1, buf)) + if (inode->i_size >= limit) + return 0; + + if (num_blocks) { + if ((num_blocks != ctx->fs->blocksize >> 9) || /* == 1 block */ + (block < ctx->fs->super->s_first_data_block) || + (block >= ext2fs_blocks_count(ctx->fs->super))) return 0; - len = strnlen(buf, fs->blocksize); - if (len == fs->blocksize) + if (io_channel_read_blk64(ctx->fs->io, block, 1, buf)) return 0; } else { - if (inode->i_size >= sizeof(inode->i_block)) - return 0; + buf = (char *)inode->i_block; + } - len = strnlen((char *)inode->i_block, sizeof(inode->i_block)); - if (len == sizeof(inode->i_block)) - return 0; + len = strnlen(buf, limit); + /* 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 (num_blocks != 0) { + if (io_channel_write_blk64(ctx->fs->io, block, 1, buf)) + return 0; + } else { + e2fsck_write_inode(ctx, ino, inode, "check_ext_attr"); + } + len = inode->i_size; } 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 immutable (or append-only) flag is set on the inode, offer * to clear it. @@ -1458,8 +1481,7 @@ void e2fsck_pass1(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 (ext2fs_inode_data_blocks(fs, inode) == 0) { diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c index ff02f32..1aa6a3a 100644 --- a/e2fsck/pass2.c +++ b/e2fsck/pass2.c @@ -1404,8 +1404,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 a56c126..26bbf80 100644 --- a/e2fsck/problem.c +++ b/e2fsck/problem.c @@ -999,6 +999,11 @@ static struct e2fsck_problem problem_table[] = { N_("@a in @i %i is corrupt (@n value)."), PROMPT_CLEAR, 0}, + /* Bad extended attribute value in inode */ + { PR_1_SYMLINK_NUL, + N_("@i %i symlink missing NUL terminator. "), + PROMPT_FIX, 0}, + /* expand inode */ { PR_1_EXPAND_EISIZE_WARNING, N_("\ne2fsck is being run with \"expand_extra_isize\" option or\n" diff --git a/e2fsck/problem.h b/e2fsck/problem.h index c1afb84..e96b115 100644 --- a/e2fsck/problem.h +++ b/e2fsck/problem.h @@ -599,6 +599,9 @@ struct problem_context { /* Bad extended attribute value in inode */ #define PR_1_INODE_EA_BAD_VALUE 0x010080 +/* Symlink missing NUL terminator */ +#define PR_1_SYMLINK_NUL 0x010081 + /* Warning for user that all inodes need to be expanded atleast by * s_min_extra_isize */ 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