diff mbox

ext4: handle symlink properly with inline_data

Message ID 1401709168-27403-1-git-send-email-wenqing.lz@taobao.com
State Accepted, archived
Headers show

Commit Message

Zheng Liu June 2, 2014, 11:39 a.m. UTC 
From: Zheng Liu <wenqing.lz@taobao.com>

This commit tries to fix a bug that we can't read symlink properly with
inline data feature when the length of symlink is greater than 60 bytes
but less than extra space.

The key issue is in ext4_inode_is_fast_symlink() that it doesn't check
whether or not an inode has inline data.  When the user creates a new
symlink, an inode will be allocated with MAY_INLINE_DATA flag.  Then
symlink will be stored in ->i_block and extended attribute space.  In
the mean time, this inode is with inline data flag.  After remounting
it, ext4_inode_is_fast_symlink() function thinks that this inode is a
fast symlink so that the data in ->i_block is copied to the user, and
the data in extra space is trimmed.  In fact this inode should be as a
normal symlink.

The following script can hit this bug.

  #!/bin/bash

  cd ${MNT}
  filename=ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789
  rm -rf test
  mkdir test
  cd test
  echo "hello" >$filename
  ln -s $filename symlinkfile
  cd
  sudo umount /mnt/sda1
  sudo mount -t ext4 /dev/sda1 /mnt/sda1
  readlink /mnt/sda1/test/symlinkfile

After applying this patch, it will break the assumption in e2fsck
because the original implementation doesn't want to support symlink
with inline data.

Reported-by: "Darrick J. Wong" <darrick.wong@oracle.com>
Reported-by: Ian Nartowicz <claws@nartowicz.co.uk>
Cc: Ian Nartowicz <claws@nartowicz.co.uk>
Cc: Tao Ma <tm@tao.ma>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
---
 fs/ext4/inode.c |    3 +++
 1 file changed, 3 insertions(+)

Comments

Theodore Ts'o June 2, 2014, 2:56 p.m. UTC | #1 
On Mon, Jun 02, 2014 at 07:39:28PM +0800, Zheng Liu wrote:
> From: Zheng Liu <wenqing.lz@taobao.com>
> 
> This commit tries to fix a bug that we can't read symlink properly with
> inline data feature when the length of symlink is greater than 60 bytes
> but less than extra space....

Applied, thanks.

> +	if (ext4_has_inline_data(inode))
> +		return 0;
> +

I am wondering though, as we add ext4_has_inline_data() to more and
more codepaths, whether we might be better off making this an inline
function in ext4.h.  We should see how much such a change bloats the
text sizes, and whether there is any measurable CPU difference.

Cheers,

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 55f999a..bc5e4c1 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -148,6 +148,9 @@  static int ext4_inode_is_fast_symlink(struct inode *inode)
         int ea_blocks = EXT4_I(inode)->i_file_acl ?
 		EXT4_CLUSTER_SIZE(inode->i_sb) >> 9 : 0;
 
+	if (ext4_has_inline_data(inode))
+		return 0;
+
 	return (S_ISLNK(inode->i_mode) && inode->i_blocks - ea_blocks == 0);
 }