[-v2] ext4: add more paranoia checking in ext4_expand_extra_isize handling
diff mbox series

Message ID 20191110121510.GH23325@mit.edu
State Awaiting Upstream
Headers show
Series
  • [-v2] ext4: add more paranoia checking in ext4_expand_extra_isize handling
Related show

Commit Message

Theodore Y. Ts'o Nov. 10, 2019, 12:15 p.m. UTC
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
---
 fs/ext4/inode.c | 15 +++++++++++++++
 fs/ext4/super.c | 21 ++++++++++++---------
 2 files changed, 27 insertions(+), 9 deletions(-)

Comments

Eric Biggers Nov. 19, 2019, 2:15 a.m. UTC | #1
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
Theodore Y. Ts'o Nov. 19, 2019, 4:21 a.m. UTC | #2
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
Eric Biggers Nov. 19, 2019, 4:36 a.m. UTC | #3
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

Patch
diff mbox series

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");
 	}