Skip to content

Commit d46b0da

Browse files
DaveWysochanskiRHsmfrench
authored andcommitted
cifs: Fix cifsInodeInfo lock_sem deadlock when reconnect occurs
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 <[email protected]> Suggested-by: Ronnie Sahlberg <[email protected]> Reviewed--by: Ronnie Sahlberg <[email protected]> Reviewed-by: Pavel Shilovsky <[email protected]>
1 parent 1a67c41 commit d46b0da

File tree

4 files changed

+22
-9
lines changed

4 files changed

+22
-9
lines changed

fs/cifs/cifsglob.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1391,6 +1391,11 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
13911391
struct cifsInodeInfo {
13921392
bool can_cache_brlcks;
13931393
struct list_head llist; /* locks helb by this inode */
1394+
/*
1395+
* NOTE: Some code paths call down_read(lock_sem) twice, so
1396+
* we must always use use cifs_down_write() instead of down_write()
1397+
* for this semaphore to avoid deadlocks.
1398+
*/
13941399
struct rw_semaphore lock_sem; /* protect the fields above */
13951400
/* BB add in lists for dirty pages i.e. write caching info for oplock */
13961401
struct list_head openFileList;

fs/cifs/cifsproto.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ extern int cifs_unlock_range(struct cifsFileInfo *cfile,
170170
struct file_lock *flock, const unsigned int xid);
171171
extern int cifs_push_mandatory_locks(struct cifsFileInfo *cfile);
172172

173+
extern void cifs_down_write(struct rw_semaphore *sem);
173174
extern struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid,
174175
struct file *file,
175176
struct tcon_link *tlink,

fs/cifs/file.c

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,13 @@ cifs_has_mand_locks(struct cifsInodeInfo *cinode)
281281
return has_locks;
282282
}
283283

284+
void
285+
cifs_down_write(struct rw_semaphore *sem)
286+
{
287+
while (!down_write_trylock(sem))
288+
msleep(10);
289+
}
290+
284291
struct cifsFileInfo *
285292
cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
286293
struct tcon_link *tlink, __u32 oplock)
@@ -306,7 +313,7 @@ cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
306313
INIT_LIST_HEAD(&fdlocks->locks);
307314
fdlocks->cfile = cfile;
308315
cfile->llist = fdlocks;
309-
down_write(&cinode->lock_sem);
316+
cifs_down_write(&cinode->lock_sem);
310317
list_add(&fdlocks->llist, &cinode->llist);
311318
up_write(&cinode->lock_sem);
312319

@@ -464,7 +471,7 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_handler)
464471
* Delete any outstanding lock records. We'll lose them when the file
465472
* is closed anyway.
466473
*/
467-
down_write(&cifsi->lock_sem);
474+
cifs_down_write(&cifsi->lock_sem);
468475
list_for_each_entry_safe(li, tmp, &cifs_file->llist->locks, llist) {
469476
list_del(&li->llist);
470477
cifs_del_lock_waiters(li);
@@ -1027,7 +1034,7 @@ static void
10271034
cifs_lock_add(struct cifsFileInfo *cfile, struct cifsLockInfo *lock)
10281035
{
10291036
struct cifsInodeInfo *cinode = CIFS_I(d_inode(cfile->dentry));
1030-
down_write(&cinode->lock_sem);
1037+
cifs_down_write(&cinode->lock_sem);
10311038
list_add_tail(&lock->llist, &cfile->llist->locks);
10321039
up_write(&cinode->lock_sem);
10331040
}
@@ -1049,7 +1056,7 @@ cifs_lock_add_if(struct cifsFileInfo *cfile, struct cifsLockInfo *lock,
10491056

10501057
try_again:
10511058
exist = false;
1052-
down_write(&cinode->lock_sem);
1059+
cifs_down_write(&cinode->lock_sem);
10531060

10541061
exist = cifs_find_lock_conflict(cfile, lock->offset, lock->length,
10551062
lock->type, lock->flags, &conf_lock,
@@ -1072,7 +1079,7 @@ cifs_lock_add_if(struct cifsFileInfo *cfile, struct cifsLockInfo *lock,
10721079
(lock->blist.next == &lock->blist));
10731080
if (!rc)
10741081
goto try_again;
1075-
down_write(&cinode->lock_sem);
1082+
cifs_down_write(&cinode->lock_sem);
10761083
list_del_init(&lock->blist);
10771084
}
10781085

@@ -1125,7 +1132,7 @@ cifs_posix_lock_set(struct file *file, struct file_lock *flock)
11251132
return rc;
11261133

11271134
try_again:
1128-
down_write(&cinode->lock_sem);
1135+
cifs_down_write(&cinode->lock_sem);
11291136
if (!cinode->can_cache_brlcks) {
11301137
up_write(&cinode->lock_sem);
11311138
return rc;
@@ -1331,7 +1338,7 @@ cifs_push_locks(struct cifsFileInfo *cfile)
13311338
int rc = 0;
13321339

13331340
/* we are going to update can_cache_brlcks here - need a write access */
1334-
down_write(&cinode->lock_sem);
1341+
cifs_down_write(&cinode->lock_sem);
13351342
if (!cinode->can_cache_brlcks) {
13361343
up_write(&cinode->lock_sem);
13371344
return rc;
@@ -1522,7 +1529,7 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock,
15221529
if (!buf)
15231530
return -ENOMEM;
15241531

1525-
down_write(&cinode->lock_sem);
1532+
cifs_down_write(&cinode->lock_sem);
15261533
for (i = 0; i < 2; i++) {
15271534
cur = buf;
15281535
num = 0;

fs/cifs/smb2file.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock,
145145

146146
cur = buf;
147147

148-
down_write(&cinode->lock_sem);
148+
cifs_down_write(&cinode->lock_sem);
149149
list_for_each_entry_safe(li, tmp, &cfile->llist->locks, llist) {
150150
if (flock->fl_start > li->offset ||
151151
(flock->fl_start + length) <

0 commit comments

Comments
 (0)