Skip to content

Commit

Permalink
ext4: Fix insufficient checks in EXT4_IOC_MOVE_EXT
Browse files Browse the repository at this point in the history
(cherry picked from commit 4a58579)

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
Akira Fujita authored and Greg Kroah-Hartman committed Dec 14, 2009
1 parent eebb744 commit 0fd023e
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 12 deletions.
30 changes: 18 additions & 12 deletions fs/ext4/ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,32 +221,38 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
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;
}

Expand Down
7 changes: 7 additions & 0 deletions fs/ext4/move_extent.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down

0 comments on commit 0fd023e

Please sign in to comment.