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 <bobijam.xu@intel.com>
  Change-Id: If9c16f96d0655d5a886ef607f1f47ced6176f8d8

Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
Change-Id: I4419b30f1adb4a7d273796a936427aa351510213

diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h
index ed2410d..f7c3b60 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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