| From 7a3552d0d99303b4de2153303e15f2afcab22c21 Mon Sep 17 00:00:00 2001 |
| From: Dave Wysochanski <dwysocha@redhat.com> |
| Date: Wed, 23 Oct 2019 05:02:33 -0400 |
| Subject: [PATCH] cifs: Fix cifsInodeInfo lock_sem deadlock when reconnect |
| occurs |
| |
| commit d46b0da7a33dd8c99d969834f682267a45444ab3 upstream. |
| |
| There's a deadlock that is possible and can easily be seen with |
| a test where multiple readers open/read/close of the same file |
| and a disruption occurs causing reconnect. The deadlock is due |
| a reader thread inside cifs_strict_readv calling down_read and |
| obtaining lock_sem, and then after reconnect inside |
| cifs_reopen_file calling down_read a second time. If in |
| between the two down_read calls, a down_write comes from |
| another process, deadlock occurs. |
| |
| CPU0 CPU1 |
| ---- ---- |
| cifs_strict_readv() |
| down_read(&cifsi->lock_sem); |
| _cifsFileInfo_put |
| OR |
| cifs_new_fileinfo |
| down_write(&cifsi->lock_sem); |
| cifs_reopen_file() |
| down_read(&cifsi->lock_sem); |
| |
| Fix the above by changing all down_write(lock_sem) calls to |
| down_write_trylock(lock_sem)/msleep() loop, which in turn |
| makes the second down_read call benign since it will never |
| block behind the writer while holding lock_sem. |
| |
| Signed-off-by: Dave Wysochanski <dwysocha@redhat.com> |
| Suggested-by: Ronnie Sahlberg <lsahlber@redhat.com> |
| Reviewed--by: Ronnie Sahlberg <lsahlber@redhat.com> |
| Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h |
| index 85aa1bc930f1..7b98889d4308 100644 |
| --- a/fs/cifs/cifsglob.h |
| +++ b/fs/cifs/cifsglob.h |
| @@ -1376,6 +1376,11 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file); |
| struct cifsInodeInfo { |
| bool can_cache_brlcks; |
| struct list_head llist; /* locks helb by this inode */ |
| + /* |
| + * NOTE: Some code paths call down_read(lock_sem) twice, so |
| + * we must always use use cifs_down_write() instead of down_write() |
| + * for this semaphore to avoid deadlocks. |
| + */ |
| struct rw_semaphore lock_sem; /* protect the fields above */ |
| /* BB add in lists for dirty pages i.e. write caching info for oplock */ |
| struct list_head openFileList; |
| diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h |
| index e23234207fc2..6d7c692ce2ba 100644 |
| --- a/fs/cifs/cifsproto.h |
| +++ b/fs/cifs/cifsproto.h |
| @@ -166,6 +166,7 @@ extern int cifs_unlock_range(struct cifsFileInfo *cfile, |
| struct file_lock *flock, const unsigned int xid); |
| extern int cifs_push_mandatory_locks(struct cifsFileInfo *cfile); |
| |
| +extern void cifs_down_write(struct rw_semaphore *sem); |
| extern struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, |
| struct file *file, |
| struct tcon_link *tlink, |
| diff --git a/fs/cifs/file.c b/fs/cifs/file.c |
| index 53dbb6e0d390..facb52d37d19 100644 |
| --- a/fs/cifs/file.c |
| +++ b/fs/cifs/file.c |
| @@ -281,6 +281,13 @@ cifs_has_mand_locks(struct cifsInodeInfo *cinode) |
| return has_locks; |
| } |
| |
| +void |
| +cifs_down_write(struct rw_semaphore *sem) |
| +{ |
| + while (!down_write_trylock(sem)) |
| + msleep(10); |
| +} |
| + |
| struct cifsFileInfo * |
| cifs_new_fileinfo(struct cifs_fid *fid, struct file *file, |
| struct tcon_link *tlink, __u32 oplock) |
| @@ -306,7 +313,7 @@ cifs_new_fileinfo(struct cifs_fid *fid, struct file *file, |
| INIT_LIST_HEAD(&fdlocks->locks); |
| fdlocks->cfile = cfile; |
| cfile->llist = fdlocks; |
| - down_write(&cinode->lock_sem); |
| + cifs_down_write(&cinode->lock_sem); |
| list_add(&fdlocks->llist, &cinode->llist); |
| up_write(&cinode->lock_sem); |
| |
| @@ -464,7 +471,7 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_handler) |
| * Delete any outstanding lock records. We'll lose them when the file |
| * is closed anyway. |
| */ |
| - down_write(&cifsi->lock_sem); |
| + cifs_down_write(&cifsi->lock_sem); |
| list_for_each_entry_safe(li, tmp, &cifs_file->llist->locks, llist) { |
| list_del(&li->llist); |
| cifs_del_lock_waiters(li); |
| @@ -1027,7 +1034,7 @@ static void |
| cifs_lock_add(struct cifsFileInfo *cfile, struct cifsLockInfo *lock) |
| { |
| struct cifsInodeInfo *cinode = CIFS_I(d_inode(cfile->dentry)); |
| - down_write(&cinode->lock_sem); |
| + cifs_down_write(&cinode->lock_sem); |
| list_add_tail(&lock->llist, &cfile->llist->locks); |
| up_write(&cinode->lock_sem); |
| } |
| @@ -1049,7 +1056,7 @@ cifs_lock_add_if(struct cifsFileInfo *cfile, struct cifsLockInfo *lock, |
| |
| try_again: |
| exist = false; |
| - down_write(&cinode->lock_sem); |
| + cifs_down_write(&cinode->lock_sem); |
| |
| exist = cifs_find_lock_conflict(cfile, lock->offset, lock->length, |
| lock->type, lock->flags, &conf_lock, |
| @@ -1072,7 +1079,7 @@ cifs_lock_add_if(struct cifsFileInfo *cfile, struct cifsLockInfo *lock, |
| (lock->blist.next == &lock->blist)); |
| if (!rc) |
| goto try_again; |
| - down_write(&cinode->lock_sem); |
| + cifs_down_write(&cinode->lock_sem); |
| list_del_init(&lock->blist); |
| } |
| |
| @@ -1125,7 +1132,7 @@ cifs_posix_lock_set(struct file *file, struct file_lock *flock) |
| return rc; |
| |
| try_again: |
| - down_write(&cinode->lock_sem); |
| + cifs_down_write(&cinode->lock_sem); |
| if (!cinode->can_cache_brlcks) { |
| up_write(&cinode->lock_sem); |
| return rc; |
| @@ -1331,7 +1338,7 @@ cifs_push_locks(struct cifsFileInfo *cfile) |
| int rc = 0; |
| |
| /* we are going to update can_cache_brlcks here - need a write access */ |
| - down_write(&cinode->lock_sem); |
| + cifs_down_write(&cinode->lock_sem); |
| if (!cinode->can_cache_brlcks) { |
| up_write(&cinode->lock_sem); |
| return rc; |
| @@ -1522,7 +1529,7 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, |
| if (!buf) |
| return -ENOMEM; |
| |
| - down_write(&cinode->lock_sem); |
| + cifs_down_write(&cinode->lock_sem); |
| for (i = 0; i < 2; i++) { |
| cur = buf; |
| num = 0; |
| diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c |
| index 54bffb2a1786..2abfa2703060 100644 |
| --- a/fs/cifs/smb2file.c |
| +++ b/fs/cifs/smb2file.c |
| @@ -139,7 +139,7 @@ smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, |
| |
| cur = buf; |
| |
| - down_write(&cinode->lock_sem); |
| + cifs_down_write(&cinode->lock_sem); |
| list_for_each_entry_safe(li, tmp, &cfile->llist->locks, llist) { |
| if (flock->fl_start > li->offset || |
| (flock->fl_start + length) < |
| -- |
| 2.7.4 |
| |