Skip to content

Commit b98749c

Browse files
aaptelsmfrench
authored andcommitted
CIFS: keep FileInfo handle live during oplock break
In the oplock break handler, writing pending changes from pages puts the FileInfo handle. If the refcount reaches zero it closes the handle and waits for any oplock break handler to return, thus causing a deadlock. To prevent this situation: * We add a wait flag to cifsFileInfo_put() to decide whether we should wait for running/pending oplock break handlers * We keep an additionnal reference of the SMB FileInfo handle so that for the rest of the handler putting the handle won't close it. - The ref is bumped everytime we queue the handler via the cifs_queue_oplock_break() helper. - The ref is decremented at the end of the handler This bug was triggered by xfstest 464. Also important fix to address the various reports of oops in smb2_push_mandatory_locks Signed-off-by: Aurelien Aptel <aaptel@suse.com> Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> CC: Stable <stable@vger.kernel.org>
1 parent e6d0fb7 commit b98749c

File tree

4 files changed

+53
-10
lines changed

4 files changed

+53
-10
lines changed

fs/cifs/cifsglob.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,6 +1333,7 @@ cifsFileInfo_get_locked(struct cifsFileInfo *cifs_file)
13331333
}
13341334

13351335
struct cifsFileInfo *cifsFileInfo_get(struct cifsFileInfo *cifs_file);
1336+
void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_hdlr);
13361337
void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
13371338

13381339
#define CIFS_CACHE_READ_FLG 1
@@ -1855,6 +1856,7 @@ GLOBAL_EXTERN spinlock_t gidsidlock;
18551856
#endif /* CONFIG_CIFS_ACL */
18561857

18571858
void cifs_oplock_break(struct work_struct *work);
1859+
void cifs_queue_oplock_break(struct cifsFileInfo *cfile);
18581860

18591861
extern const struct slow_work_ops cifs_oplock_break_ops;
18601862
extern struct workqueue_struct *cifsiod_wq;

fs/cifs/file.c

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -360,12 +360,30 @@ cifsFileInfo_get(struct cifsFileInfo *cifs_file)
360360
return cifs_file;
361361
}
362362

363-
/*
364-
* Release a reference on the file private data. This may involve closing
365-
* the filehandle out on the server. Must be called without holding
366-
* tcon->open_file_lock and cifs_file->file_info_lock.
363+
/**
364+
* cifsFileInfo_put - release a reference of file priv data
365+
*
366+
* Always potentially wait for oplock handler. See _cifsFileInfo_put().
367367
*/
368368
void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
369+
{
370+
_cifsFileInfo_put(cifs_file, true);
371+
}
372+
373+
/**
374+
* _cifsFileInfo_put - release a reference of file priv data
375+
*
376+
* This may involve closing the filehandle @cifs_file out on the
377+
* server. Must be called without holding tcon->open_file_lock and
378+
* cifs_file->file_info_lock.
379+
*
380+
* If @wait_for_oplock_handler is true and we are releasing the last
381+
* reference, wait for any running oplock break handler of the file
382+
* and cancel any pending one. If calling this function from the
383+
* oplock break handler, you need to pass false.
384+
*
385+
*/
386+
void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_handler)
369387
{
370388
struct inode *inode = d_inode(cifs_file->dentry);
371389
struct cifs_tcon *tcon = tlink_tcon(cifs_file->tlink);
@@ -414,7 +432,8 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
414432

415433
spin_unlock(&tcon->open_file_lock);
416434

417-
oplock_break_cancelled = cancel_work_sync(&cifs_file->oplock_break);
435+
oplock_break_cancelled = wait_oplock_handler ?
436+
cancel_work_sync(&cifs_file->oplock_break) : false;
418437

419438
if (!tcon->need_reconnect && !cifs_file->invalidHandle) {
420439
struct TCP_Server_Info *server = tcon->ses->server;
@@ -4603,6 +4622,7 @@ void cifs_oplock_break(struct work_struct *work)
46034622
cinode);
46044623
cifs_dbg(FYI, "Oplock release rc = %d\n", rc);
46054624
}
4625+
_cifsFileInfo_put(cfile, false /* do not wait for ourself */);
46064626
cifs_done_oplock_break(cinode);
46074627
}
46084628

fs/cifs/misc.c

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -501,8 +501,7 @@ is_valid_oplock_break(char *buffer, struct TCP_Server_Info *srv)
501501
CIFS_INODE_DOWNGRADE_OPLOCK_TO_L2,
502502
&pCifsInode->flags);
503503

504-
queue_work(cifsoplockd_wq,
505-
&netfile->oplock_break);
504+
cifs_queue_oplock_break(netfile);
506505
netfile->oplock_break_cancelled = false;
507506

508507
spin_unlock(&tcon->open_file_lock);
@@ -607,6 +606,28 @@ void cifs_put_writer(struct cifsInodeInfo *cinode)
607606
spin_unlock(&cinode->writers_lock);
608607
}
609608

609+
/**
610+
* cifs_queue_oplock_break - queue the oplock break handler for cfile
611+
*
612+
* This function is called from the demultiplex thread when it
613+
* receives an oplock break for @cfile.
614+
*
615+
* Assumes the tcon->open_file_lock is held.
616+
* Assumes cfile->file_info_lock is NOT held.
617+
*/
618+
void cifs_queue_oplock_break(struct cifsFileInfo *cfile)
619+
{
620+
/*
621+
* Bump the handle refcount now while we hold the
622+
* open_file_lock to enforce the validity of it for the oplock
623+
* break handler. The matching put is done at the end of the
624+
* handler.
625+
*/
626+
cifsFileInfo_get(cfile);
627+
628+
queue_work(cifsoplockd_wq, &cfile->oplock_break);
629+
}
630+
610631
void cifs_done_oplock_break(struct cifsInodeInfo *cinode)
611632
{
612633
clear_bit(CIFS_INODE_PENDING_OPLOCK_BREAK, &cinode->flags);

fs/cifs/smb2misc.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp,
555555
clear_bit(CIFS_INODE_DOWNGRADE_OPLOCK_TO_L2,
556556
&cinode->flags);
557557

558-
queue_work(cifsoplockd_wq, &cfile->oplock_break);
558+
cifs_queue_oplock_break(cfile);
559559
kfree(lw);
560560
return true;
561561
}
@@ -712,8 +712,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server)
712712
CIFS_INODE_DOWNGRADE_OPLOCK_TO_L2,
713713
&cinode->flags);
714714
spin_unlock(&cfile->file_info_lock);
715-
queue_work(cifsoplockd_wq,
716-
&cfile->oplock_break);
715+
716+
cifs_queue_oplock_break(cfile);
717717

718718
spin_unlock(&tcon->open_file_lock);
719719
spin_unlock(&cifs_tcp_ses_lock);

0 commit comments

Comments
 (0)