summaryrefslogtreecommitdiffstats
path: root/fs/cifs
diff options
context:
space:
mode:
authorRabin Vincent <rabinv@axis.com>2017-06-29 16:01:42 +0200
committerSteve French <smfrench@gmail.com>2017-07-05 20:08:39 -0500
commit966681c9f029afd5decee069b7658bb58ad0a863 (patch)
treea55d91f196580ea1a744c5b120bd4b40b4fdcc92 /fs/cifs
parent709340a00ad67aea081916582846248e3b18b463 (diff)
downloadlinux-966681c9f029afd5decee069b7658bb58ad0a863.tar.gz
linux-966681c9f029afd5decee069b7658bb58ad0a863.tar.bz2
linux-966681c9f029afd5decee069b7658bb58ad0a863.zip
CIFS: fix circular locking dependency
When a CIFS filesystem is mounted with the forcemand option and the following command is run on it, lockdep warns about a circular locking dependency between CifsInodeInfo::lock_sem and the inode lock. while echo foo > hello; do :; done & while touch -c hello; do :; done cifs_writev() takes the locks in the wrong order, but note that we can't only flip the order around because it releases the inode lock before the call to generic_write_sync() while it holds the lock_sem across that call. But, AFAICS, there is no need to hold the CifsInodeInfo::lock_sem across the generic_write_sync() call either, so we can release both the locks before generic_write_sync(), and change the order. ====================================================== WARNING: possible circular locking dependency detected 4.12.0-rc7+ #9 Not tainted ------------------------------------------------------ touch/487 is trying to acquire lock: (&cifsi->lock_sem){++++..}, at: cifsFileInfo_put+0x88f/0x16a0 but task is already holding lock: (&sb->s_type->i_mutex_key#11){+.+.+.}, at: utimes_common+0x3ad/0x870 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&sb->s_type->i_mutex_key#11){+.+.+.}: __lock_acquire+0x1f74/0x38f0 lock_acquire+0x1cc/0x600 down_write+0x74/0x110 cifs_strict_writev+0x3cb/0x8c0 __vfs_write+0x4c1/0x930 vfs_write+0x14c/0x2d0 SyS_write+0xf7/0x240 entry_SYSCALL_64_fastpath+0x1f/0xbe -> #0 (&cifsi->lock_sem){++++..}: check_prevs_add+0xfa0/0x1d10 __lock_acquire+0x1f74/0x38f0 lock_acquire+0x1cc/0x600 down_write+0x74/0x110 cifsFileInfo_put+0x88f/0x16a0 cifs_setattr+0x992/0x1680 notify_change+0x61a/0xa80 utimes_common+0x3d4/0x870 do_utimes+0x1c1/0x220 SyS_utimensat+0x84/0x1a0 entry_SYSCALL_64_fastpath+0x1f/0xbe other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&sb->s_type->i_mutex_key#11); lock(&cifsi->lock_sem); lock(&sb->s_type->i_mutex_key#11); lock(&cifsi->lock_sem); *** DEADLOCK *** 2 locks held by touch/487: #0: (sb_writers#10){.+.+.+}, at: mnt_want_write+0x41/0xb0 #1: (&sb->s_type->i_mutex_key#11){+.+.+.}, at: utimes_common+0x3ad/0x870 stack backtrace: CPU: 0 PID: 487 Comm: touch Not tainted 4.12.0-rc7+ #9 Call Trace: dump_stack+0xdb/0x185 print_circular_bug+0x45b/0x790 __lock_acquire+0x1f74/0x38f0 lock_acquire+0x1cc/0x600 down_write+0x74/0x110 cifsFileInfo_put+0x88f/0x16a0 cifs_setattr+0x992/0x1680 notify_change+0x61a/0xa80 utimes_common+0x3d4/0x870 do_utimes+0x1c1/0x220 SyS_utimensat+0x84/0x1a0 entry_SYSCALL_64_fastpath+0x1f/0xbe Fixes: 19dfc1f5f2ef03a52 ("cifs: fix the race in cifs_writev()") Signed-off-by: Rabin Vincent <rabinv@axis.com> Signed-off-by: Steve French <smfrench@gmail.com> Acked-by: Pavel Shilovsky <pshilov@microsoft.com>
Diffstat (limited to 'fs/cifs')
-rw-r--r--fs/cifs/file.c4
1 files changed, 2 insertions, 2 deletions
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index dec70b304269..bc09df6b473a 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2812,12 +2812,12 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
struct TCP_Server_Info *server = tlink_tcon(cfile->tlink)->ses->server;
ssize_t rc;
+ inode_lock(inode);
/*
* We need to hold the sem to be sure nobody modifies lock list
* with a brlock that prevents writing.
*/
down_read(&cinode->lock_sem);
- inode_lock(inode);
rc = generic_write_checks(iocb, from);
if (rc <= 0)
@@ -2830,11 +2830,11 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
else
rc = -EACCES;
out:
+ up_read(&cinode->lock_sem);
inode_unlock(inode);
if (rc > 0)
rc = generic_write_sync(iocb, rc);
- up_read(&cinode->lock_sem);
return rc;
}