Eric Lee / smarc-ti-linux-kernel

Download as
Browse Code ยป

Commit 24f1a316c4bfeed2e1d9a7803545839e2d2e4a0b

Authored by Al Viro
Committed by Greg Kroah-Hartman
1 parent 3436c0aadc

fix deadlock in cifs_ioctl_clone() 

commit 378ff1a53b5724f3ac97b0aba3c9ecac072f6fcd upstream.

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().

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Showing 1 changed file with 5 additions and 16 deletions Side-by-side Diff

... ... @@ -86,22 +86,17 @@
86 86 }
87 87  
88 88 src_inode = file_inode(src_file.file);
  89 + rc = -EINVAL;
  90 + if (S_ISDIR(src_inode->i_mode))
  91 + goto out_fput;
89 92  
90 93 /*
91 94 * Note: cifs case is easier than btrfs since server responsible for
92 95 * checks for proper open modes and file type and if it wants
93 96 * server could even support copy of range where source = target
94 97 */
  98 + lock_two_nondirectories(target_inode, src_inode);
95 99  
96   - /* so we do not deadlock racing two ioctls on same files */
97   - if (target_inode < src_inode) {
98   - mutex_lock_nested(&target_inode->i_mutex, I_MUTEX_PARENT);
99   - mutex_lock_nested(&src_inode->i_mutex, I_MUTEX_CHILD);
100   - } else {
101   - mutex_lock_nested(&src_inode->i_mutex, I_MUTEX_PARENT);
102   - mutex_lock_nested(&target_inode->i_mutex, I_MUTEX_CHILD);
103   - }
104   -
105 100 /* determine range to clone */
106 101 rc = -EINVAL;
107 102 if (off + len > src_inode->i_size || off + len < off)
... ... @@ -124,13 +119,7 @@
124 119 out_unlock:
125 120 /* although unlocking in the reverse order from locking is not
126 121 strictly necessary here it is a little cleaner to be consistent */
127   - if (target_inode < src_inode) {
128   - mutex_unlock(&src_inode->i_mutex);
129   - mutex_unlock(&target_inode->i_mutex);
130   - } else {
131   - mutex_unlock(&target_inode->i_mutex);
132   - mutex_unlock(&src_inode->i_mutex);
133   - }
  122 + unlock_two_nondirectories(src_inode, target_inode);
134 123 out_fput:
135 124 fdput(src_file);
136 125 out_drop_write: