Message ID | 20191110121510.GH23325@mit.edu |
---|---|
State | Accepted, archived |
Headers | show |
Series | [-v2] ext4: add more paranoia checking in ext4_expand_extra_isize handling | expand |
On Sun, Nov 10, 2019 at 07:15:10AM -0500, Theodore Y. Ts'o wrote: > I hadn't gotten around to resending the patch. The original version > had a number of last-minute typos that had crept in... > > - Ted > > From a67ad537964d10f94a4b990c084365e75316cde8 Mon Sep 17 00:00:00 2001 > From: Theodore Ts'o <tytso@mit.edu> > Date: Thu, 7 Nov 2019 21:43:41 -0500 > Subject: [PATCH] ext4: add more paranoia checking in ext4_expand_extra_isize > handling > > > It's possible to specify a non-zero s_want_extra_isize via debugging > option, and this can cause bad things(tm) to happen when using a file > system with an inode size of 128 bytes. > > Add better checking when the file system is mounted, as well as when > we are actually doing the trying to do the inode expansion. > > Reported-by: syzbot+f8d6f8386ceacdbfff57@syzkaller.appspotmail.com > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > Cc: stable@kernel.org Is this patch intended to address https://lore.kernel.org/linux-ext4/000000000000950f21059564e4c7@google.com as well? If so, you can add the second Reported-by line so that both syzbot reports get closed. - Eric
On Mon, Nov 18, 2019 at 06:15:26PM -0800, Eric Biggers wrote: > Is this patch intended to address > https://lore.kernel.org/linux-ext4/000000000000950f21059564e4c7@google.com > as well? If so, you can add the second Reported-by line so that both syzbot > reports get closed. Yes, it appears to be the same issue. Thanks for pointing this out! - Ted
On Mon, Nov 18, 2019 at 11:21:20PM -0500, Theodore Y. Ts'o wrote: > On Mon, Nov 18, 2019 at 06:15:26PM -0800, Eric Biggers wrote: > > Is this patch intended to address > > https://lore.kernel.org/linux-ext4/000000000000950f21059564e4c7@google.com > > as well? If so, you can add the second Reported-by line so that both syzbot > > reports get closed. > > Yes, it appears to be the same issue. Thanks for pointing this out! > > - Ted There's actually a third that looks very similar too: "KASAN: use-after-free Write in __ext4_expand_extra_isize (2)" https://lkml.kernel.org/linux-ext4/0000000000000d74c7059564e17f@google.com/ If these are all fixed by this patch, you can use: Reported-by: syzbot+f8d6f8386ceacdbfff57@syzkaller.appspotmail.com Reported-by: syzbot+33d7ea72e47de3bdf4e1@syzkaller.appspotmail.com Reported-by: syzbot+44b6763edfc17144296f@syzkaller.appspotmail.com
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 381813205f99..c6e3fe287b50 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -5569,8 +5569,23 @@ static int __ext4_expand_extra_isize(struct inode *inode, { struct ext4_inode *raw_inode; struct ext4_xattr_ibody_header *header; + unsigned int inode_size = EXT4_INODE_SIZE(inode->i_sb); + struct ext4_inode_info *ei = EXT4_I(inode); int error; + /* this was checked at iget time, but double check for good measure */ + if ((EXT4_GOOD_OLD_INODE_SIZE + ei->i_extra_isize > inode_size) || + (ei->i_extra_isize & 3)) { + EXT4_ERROR_INODE(inode, "bad extra_isize %u (inode size %u)", + ei->i_extra_isize, + EXT4_INODE_SIZE(inode->i_sb)); + return -EFSCORRUPTED; + } + if ((new_extra_isize < ei->i_extra_isize) || + (new_extra_isize < 4) || + (new_extra_isize > inode_size - EXT4_GOOD_OLD_INODE_SIZE)) + return -EINVAL; /* Should never happen */ + raw_inode = ext4_raw_inode(iloc); header = IHDR(inode, raw_inode); diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 7796e2ffc294..71af8780d4ee 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -3545,12 +3545,15 @@ static void ext4_clamp_want_extra_isize(struct super_block *sb) { struct ext4_sb_info *sbi = EXT4_SB(sb); struct ext4_super_block *es = sbi->s_es; + unsigned def_extra_isize = sizeof(struct ext4_inode) - + EXT4_GOOD_OLD_INODE_SIZE; - /* determine the minimum size of new large inodes, if present */ - if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE && - sbi->s_want_extra_isize == 0) { - sbi->s_want_extra_isize = sizeof(struct ext4_inode) - - EXT4_GOOD_OLD_INODE_SIZE; + if (sbi->s_inode_size == EXT4_GOOD_OLD_INODE_SIZE) { + sbi->s_want_extra_isize = 0; + return; + } + if (sbi->s_want_extra_isize < 4) { + sbi->s_want_extra_isize = def_extra_isize; if (ext4_has_feature_extra_isize(sb)) { if (sbi->s_want_extra_isize < le16_to_cpu(es->s_want_extra_isize)) @@ -3563,10 +3566,10 @@ static void ext4_clamp_want_extra_isize(struct super_block *sb) } } /* Check if enough inode space is available */ - if (EXT4_GOOD_OLD_INODE_SIZE + sbi->s_want_extra_isize > - sbi->s_inode_size) { - sbi->s_want_extra_isize = sizeof(struct ext4_inode) - - EXT4_GOOD_OLD_INODE_SIZE; + if ((sbi->s_want_extra_isize > sbi->s_inode_size) || + (EXT4_GOOD_OLD_INODE_SIZE + sbi->s_want_extra_isize > + sbi->s_inode_size)) { + sbi->s_want_extra_isize = def_extra_isize; ext4_msg(sb, KERN_INFO, "required extra inode space not available"); }