From: Eric Sandeen <sandeen@redhat.com>
Date: Thu, 10 Dec 2009 17:36:19 -0500
Subject: [fs] ext4: fix insufficient checks in EXT4_IOC_MOVE_EXT
Message-id: <4B213193.3080702@redhat.com>
Patchwork-id: 21868
O-Subject: [PATCH RHEL5.5] ext4: Fix insufficient checks in EXT4_IOC_MOVE_EXT
Bugzilla: 546105
CVE: CVE-2009-4131
RH-Acked-by: Josef Bacik <josef@redhat.com>

This addresses:

Bug 546105 - CVE-2009-4131 kernel: ext4: Fix insufficient checks in EXT4_IOC_MOVE_EXT

Insufficient checks in the EXT4_IOC_MOVE_EXT ioctl,
used for the still-pending defrag tool, could allow modifications
to files not owned by the calling user.

This was only introduced by the big RHEL5.5 ext4 rebase,
so is not an issue for older RHEL5 releases.  It's a fairly
straightforward backport of this upstream commit:

From: Akira Fujita <a-fujita@rs.jp.nec.com>
Date: Mon, 7 Dec 2009 04:38:31 +0000 (-0500)
Subject: ext4: Fix insufficient checks in EXT4_IOC_MOVE_EXT
X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Ftytso%2Fext4.git;a=commitdiff_plain;h=4a58579b9e4e2a35d57e6c9c8483e52f6f1b7fd6

ext4: Fix insufficient checks in EXT4_IOC_MOVE_EXT

This patch fixes three problems in the handling of the
EXT4_IOC_MOVE_EXT ioctl:

1. In current EXT4_IOC_MOVE_EXT, there are read access mode checks for
original and donor files, but they allow the illegal write access to
donor file, since donor file is overwritten by original file data.  To
fix this problem, change access mode checks of original (r->r/w) and
donor (r->w) files.

2.  Disallow the use of donor files that have a setuid or setgid bits.

3.  Call mnt_want_write() and mnt_drop_write() before and after
ext4_move_extents() calling to get write access to a mount.

Signed-off-by: Akira Fujita <a-fujita@rs.jp.nec.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 2f0ee0c..e48a230 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -211,31 +211,33 @@ flags_out:
 		struct file *donor_filp;
 		int err;
 
+		if (!(filp->f_mode & FMODE_READ) ||
+		    !(filp->f_mode & FMODE_WRITE))
+			return -EBADF;
+
 		if (copy_from_user(&me,
 			(struct move_extent __user *)arg, sizeof(me)))
 			return -EFAULT;
+		me.moved_len = 0;
 
 		donor_filp = fget(me.donor_fd);
 		if (!donor_filp)
 			return -EBADF;
 
-		if (!capable(CAP_DAC_OVERRIDE)) {
-			if ((current->fsuid != inode->i_uid) ||
-				!(inode->i_mode & S_IRUSR) ||
-				!(donor_filp->f_dentry->d_inode->i_mode &
-				S_IRUSR)) {
-				fput(donor_filp);
-				return -EACCES;
-			}
+		if (!(donor_filp->f_mode & FMODE_WRITE)) {
+			err = -EBADF;
+			goto mext_out;
 		}
 
 		err = ext4_move_extents(filp, donor_filp, me.orig_start,
 					me.donor_start, me.len, &me.moved_len);
-		fput(donor_filp);
+		if (me.moved_len > 0)
+			remove_suid(donor_filp->f_dentry);
 
 		if (copy_to_user((struct move_extent *)arg, &me, sizeof(me)))
-			return -EFAULT;
-
+			err = -EFAULT;
+mext_out:
+		fput(donor_filp);
 		return err;
 	}
 
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 1487b82..da0506b 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -986,6 +986,13 @@ mext_check_arguments(struct inode *orig_inode,
 		return -EINVAL;
 	}
 
+	if (donor_inode->i_mode & (S_ISUID|S_ISGID)) {
+		ext4_debug("ext4 move extent: suid or sgid is set"
+			   " to donor file [ino:orig %lu, donor %lu]\n",
+			   orig_inode->i_ino, donor_inode->i_ino);
+		return -EINVAL;
+	}
+
 	/* Ext4 move extent does not support swapfile */
 	if (IS_SWAPFILE(orig_inode) || IS_SWAPFILE(donor_inode)) {
 		ext4_debug("ext4 move extent: The argument files should "