Commit 2dc8d9e1 authored by Eric Biggers's avatar Eric Biggers Committed by Theodore Ts'o

ext4: forbid i_extra_isize not divisible by 4

i_extra_isize not divisible by 4 is problematic for several reasons:

- It causes the in-inode xattr space to be misaligned, but the xattr
  header and entries are not declared __packed to express this
  possibility.  This may cause poor performance or incorrect code
  generation on some platforms.
- When validating the xattr entries we can read past the end of the
  inode if the size available for xattrs is not a multiple of 4.
- It allows the nonsensical i_extra_isize=1, which doesn't even leave
  enough room for i_extra_isize itself.

Therefore, update ext4_iget() to consider i_extra_isize not divisible by
4 to be an error, like the case where i_extra_isize is too large.

This also matches the rule recently added to e2fsck for determining
whether an inode has valid i_extra_isize.

This patch shouldn't have any noticeable effect on
non-corrupted/non-malicious filesystems, since the size of ext4_inode
has always been a multiple of 4.
Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
Reviewed-by: default avatarAndreas Dilger <adilger@dilger.ca>
parent ba679017
...@@ -4572,10 +4572,12 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino) ...@@ -4572,10 +4572,12 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) { if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) {
ei->i_extra_isize = le16_to_cpu(raw_inode->i_extra_isize); ei->i_extra_isize = le16_to_cpu(raw_inode->i_extra_isize);
if (EXT4_GOOD_OLD_INODE_SIZE + ei->i_extra_isize > if (EXT4_GOOD_OLD_INODE_SIZE + ei->i_extra_isize >
EXT4_INODE_SIZE(inode->i_sb)) { EXT4_INODE_SIZE(inode->i_sb) ||
EXT4_ERROR_INODE(inode, "bad extra_isize (%u != %u)", (ei->i_extra_isize & 3)) {
EXT4_GOOD_OLD_INODE_SIZE + ei->i_extra_isize, EXT4_ERROR_INODE(inode,
EXT4_INODE_SIZE(inode->i_sb)); "bad extra_isize %u (inode size %u)",
ei->i_extra_isize,
EXT4_INODE_SIZE(inode->i_sb));
ret = -EFSCORRUPTED; ret = -EFSCORRUPTED;
goto bad_inode; goto bad_inode;
} }
...@@ -4693,6 +4695,7 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino) ...@@ -4693,6 +4695,7 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) { if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) {
if (ei->i_extra_isize == 0) { if (ei->i_extra_isize == 0) {
/* The extra space is currently unused. Use it. */ /* The extra space is currently unused. Use it. */
BUILD_BUG_ON(sizeof(struct ext4_inode) & 3);
ei->i_extra_isize = sizeof(struct ext4_inode) - ei->i_extra_isize = sizeof(struct ext4_inode) -
EXT4_GOOD_OLD_INODE_SIZE; EXT4_GOOD_OLD_INODE_SIZE;
} else { } else {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment