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 31e5ee0c858fb40ad61a505cb664b0fb3c31fe23..b63d193126dbf759236a52f075af914bc921d40a 100644 (file)
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -221,32 +221,38 @@ setversion_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->real_cred->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;
                }
 
-               me.moved_len = 0;
+               err = mnt_want_write(filp->f_path.mnt);
+               if (err)
+                       goto mext_out;
+
                err = ext4_move_extents(filp, donor_filp, me.orig_start,
                                        me.donor_start, me.len, &me.moved_len);
-               fput(donor_filp);
+               mnt_drop_write(filp->f_path.mnt);
+               if (me.moved_len > 0)
+                       file_remove_suid(donor_filp);
 
                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 cad1e2edda7e2b4b613817b770d7cc6f94dc80c0..82c415be87a46be61a36dc205d24c47955937e03 100644 (file)
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -957,6 +957,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 "