From 378ff1a53b5724f3ac97b0aba3c9ecac072f6fcd Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 18 Jan 2015 23:37:32 -0500 Subject: fix deadlock in cifs_ioctl_clone() It really needs to check that src is non-directory *and* use {un,}lock_two_nodirectories(). As it is, it's trivial to cause double-lock (ioctl(fd, CIFS_IOC_COPYCHUNK_FILE, fd)) and if the last argument is an fd of directory, we are asking for trouble by violating the locking order - all directories go before all non-directories. If the last argument is an fd of parent directory, it has 50% odds of locking child before parent, which will cause AB-BA deadlock if we race with unlink(). Cc: stable@vger.kernel.org @ 3.13+ Signed-off-by: Al Viro --- fs/cifs/ioctl.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) (limited to 'fs/cifs') diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c index 45cb59bcc791..8b7898b7670f 100644 --- a/fs/cifs/ioctl.c +++ b/fs/cifs/ioctl.c @@ -86,21 +86,16 @@ static long cifs_ioctl_clone(unsigned int xid, struct file *dst_file, } src_inode = file_inode(src_file.file); + rc = -EINVAL; + if (S_ISDIR(src_inode->i_mode)) + goto out_fput; /* * Note: cifs case is easier than btrfs since server responsible for * checks for proper open modes and file type and if it wants * server could even support copy of range where source = target */ - - /* so we do not deadlock racing two ioctls on same files */ - if (target_inode < src_inode) { - mutex_lock_nested(&target_inode->i_mutex, I_MUTEX_PARENT); - mutex_lock_nested(&src_inode->i_mutex, I_MUTEX_CHILD); - } else { - mutex_lock_nested(&src_inode->i_mutex, I_MUTEX_PARENT); - mutex_lock_nested(&target_inode->i_mutex, I_MUTEX_CHILD); - } + lock_two_nondirectories(target_inode, src_inode); /* determine range to clone */ rc = -EINVAL; @@ -124,13 +119,7 @@ static long cifs_ioctl_clone(unsigned int xid, struct file *dst_file, out_unlock: /* although unlocking in the reverse order from locking is not strictly necessary here it is a little cleaner to be consistent */ - if (target_inode < src_inode) { - mutex_unlock(&src_inode->i_mutex); - mutex_unlock(&target_inode->i_mutex); - } else { - mutex_unlock(&target_inode->i_mutex); - mutex_unlock(&src_inode->i_mutex); - } + unlock_two_nondirectories(src_inode, target_inode); out_fput: fdput(src_file); out_drop_write: -- cgit v1.2.3-58-ga151