[SRU,B,1/1] CIFS: keep FileInfo handle live during oplock break
diff mbox series

Message ID 20190717200236.14920-2-gpiccoli@canonical.com
State New
Headers show
Series
  • kernel panic using CIFS share in smb2_push_mandatory_locks()
Related show

Commit Message

Guilherme Piccoli July 17, 2019, 8:02 p.m. UTC
From: Aurelien Aptel <aaptel@suse.com>

BugLink: http://bugs.launchpad.net/bugs/1795659

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>
(backported from commit b98749cac4a695f084a5ff076f4510b23e353ecd)
[gpiccoli: context adjustment.]
Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
---
 fs/cifs/cifsglob.h |  2 ++
 fs/cifs/file.c     | 30 +++++++++++++++++++++++++-----
 fs/cifs/misc.c     | 25 +++++++++++++++++++++++--
 fs/cifs/smb2misc.c |  6 +++---
 4 files changed, 53 insertions(+), 10 deletions(-)

Comments

Stefan Bader July 18, 2019, 9:22 a.m. UTC | #1
On 17.07.19 22:02, Guilherme G. Piccoli wrote:
> From: Aurelien Aptel <aaptel@suse.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1795659
> 
> 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>
> (backported from commit b98749cac4a695f084a5ff076f4510b23e353ecd)
> [gpiccoli: context adjustment.]
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> --->  fs/cifs/cifsglob.h |  2 ++
>  fs/cifs/file.c     | 30 +++++++++++++++++++++++++-----
>  fs/cifs/misc.c     | 25 +++++++++++++++++++++++--
>  fs/cifs/smb2misc.c |  6 +++---
>  4 files changed, 53 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 37b5ddf27ff1..edfa9e69fd10 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -1189,6 +1189,7 @@ cifsFileInfo_get_locked(struct cifsFileInfo *cifs_file)
>  }
>  
>  struct cifsFileInfo *cifsFileInfo_get(struct cifsFileInfo *cifs_file);
> +void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_hdlr);
>  void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
>  
>  #define CIFS_CACHE_READ_FLG	1
> @@ -1693,6 +1694,7 @@ GLOBAL_EXTERN spinlock_t gidsidlock;
>  #endif /* CONFIG_CIFS_ACL */
>  
>  void cifs_oplock_break(struct work_struct *work);
> +void cifs_queue_oplock_break(struct cifsFileInfo *cfile);
>  
>  extern const struct slow_work_ops cifs_oplock_break_ops;
>  extern struct workqueue_struct *cifsiod_wq;
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 3a85df2a9baf..7bb94b269fd2 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -358,12 +358,30 @@ cifsFileInfo_get(struct cifsFileInfo *cifs_file)
>  	return cifs_file;
>  }
>  
> -/*
> - * Release a reference on the file private data. This may involve closing
> - * the filehandle out on the server. Must be called without holding
> - * tcon->open_file_lock and cifs_file->file_info_lock.
> +/**
> + * cifsFileInfo_put - release a reference of file priv data
> + *
> + * Always potentially wait for oplock handler. See _cifsFileInfo_put().
>   */
>  void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
> +{
> +	_cifsFileInfo_put(cifs_file, true);
> +}
> +
> +/**
> + * _cifsFileInfo_put - release a reference of file priv data
> + *
> + * This may involve closing the filehandle @cifs_file out on the
> + * server. Must be called without holding tcon->open_file_lock and
> + * cifs_file->file_info_lock.
> + *
> + * If @wait_for_oplock_handler is true and we are releasing the last
> + * reference, wait for any running oplock break handler of the file
> + * and cancel any pending one. If calling this function from the
> + * oplock break handler, you need to pass false.
> + *
> + */
> +void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_handler)
>  {
>  	struct inode *inode = d_inode(cifs_file->dentry);
>  	struct cifs_tcon *tcon = tlink_tcon(cifs_file->tlink);
> @@ -411,7 +429,8 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
>  
>  	spin_unlock(&tcon->open_file_lock);
>  
> -	oplock_break_cancelled = cancel_work_sync(&cifs_file->oplock_break);
> +	oplock_break_cancelled = wait_oplock_handler ?
> +		cancel_work_sync(&cifs_file->oplock_break) : false;
>  
>  	if (!tcon->need_reconnect && !cifs_file->invalidHandle) {
>  		struct TCP_Server_Info *server = tcon->ses->server;
> @@ -4097,6 +4116,7 @@ void cifs_oplock_break(struct work_struct *work)
>  							     cinode);
>  		cifs_dbg(FYI, "Oplock release rc = %d\n", rc);
>  	}
> +	_cifsFileInfo_put(cfile, false /* do not wait for ourself */);
>  	cifs_done_oplock_break(cinode);
>  }
>  
> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> index bcab30d4a6c7..76f1649ab444 100644
> --- a/fs/cifs/misc.c
> +++ b/fs/cifs/misc.c
> @@ -486,8 +486,7 @@ is_valid_oplock_break(char *buffer, struct TCP_Server_Info *srv)
>  					   CIFS_INODE_DOWNGRADE_OPLOCK_TO_L2,
>  					   &pCifsInode->flags);
>  
> -				queue_work(cifsoplockd_wq,
> -					   &netfile->oplock_break);
> +				cifs_queue_oplock_break(netfile);
>  				netfile->oplock_break_cancelled = false;
>  
>  				spin_unlock(&tcon->open_file_lock);
> @@ -584,6 +583,28 @@ void cifs_put_writer(struct cifsInodeInfo *cinode)
>  	spin_unlock(&cinode->writers_lock);
>  }
>  
> +/**
> + * cifs_queue_oplock_break - queue the oplock break handler for cfile
> + *
> + * This function is called from the demultiplex thread when it
> + * receives an oplock break for @cfile.
> + *
> + * Assumes the tcon->open_file_lock is held.
> + * Assumes cfile->file_info_lock is NOT held.
> + */
> +void cifs_queue_oplock_break(struct cifsFileInfo *cfile)
> +{
> +	/*
> +	 * Bump the handle refcount now while we hold the
> +	 * open_file_lock to enforce the validity of it for the oplock
> +	 * break handler. The matching put is done at the end of the
> +	 * handler.
> +	 */
> +	cifsFileInfo_get(cfile);
> +
> +	queue_work(cifsoplockd_wq, &cfile->oplock_break);
> +}
> +
>  void cifs_done_oplock_break(struct cifsInodeInfo *cinode)
>  {
>  	clear_bit(CIFS_INODE_PENDING_OPLOCK_BREAK, &cinode->flags);
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index efdfdb47a7dd..bba371071ac6 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -506,7 +506,7 @@ smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp,
>  		else
>  			cfile->oplock_break_cancelled = true;
>  
> -		queue_work(cifsoplockd_wq, &cfile->oplock_break);
> +		cifs_queue_oplock_break(cfile);
>  		kfree(lw);
>  		return true;
>  	}
> @@ -650,8 +650,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server)
>  					   CIFS_INODE_DOWNGRADE_OPLOCK_TO_L2,
>  					   &cinode->flags);
>  				spin_unlock(&cfile->file_info_lock);
> -				queue_work(cifsoplockd_wq,
> -					   &cfile->oplock_break);
> +
> +				cifs_queue_oplock_break(cfile);
>  
>  				spin_unlock(&tcon->open_file_lock);
>  				spin_unlock(&cifs_tcp_ses_lock);
>
Marcelo Henrique Cerri July 18, 2019, 1:23 p.m. UTC | #2
Acked-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>

Is that needed for 4.4 too?

On Wed, Jul 17, 2019 at 05:02:36PM -0300, Guilherme G. Piccoli wrote:
> From: Aurelien Aptel <aaptel@suse.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1795659
> 
> 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>
> (backported from commit b98749cac4a695f084a5ff076f4510b23e353ecd)
> [gpiccoli: context adjustment.]
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
> ---
>  fs/cifs/cifsglob.h |  2 ++
>  fs/cifs/file.c     | 30 +++++++++++++++++++++++++-----
>  fs/cifs/misc.c     | 25 +++++++++++++++++++++++--
>  fs/cifs/smb2misc.c |  6 +++---
>  4 files changed, 53 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 37b5ddf27ff1..edfa9e69fd10 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -1189,6 +1189,7 @@ cifsFileInfo_get_locked(struct cifsFileInfo *cifs_file)
>  }
>  
>  struct cifsFileInfo *cifsFileInfo_get(struct cifsFileInfo *cifs_file);
> +void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_hdlr);
>  void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
>  
>  #define CIFS_CACHE_READ_FLG	1
> @@ -1693,6 +1694,7 @@ GLOBAL_EXTERN spinlock_t gidsidlock;
>  #endif /* CONFIG_CIFS_ACL */
>  
>  void cifs_oplock_break(struct work_struct *work);
> +void cifs_queue_oplock_break(struct cifsFileInfo *cfile);
>  
>  extern const struct slow_work_ops cifs_oplock_break_ops;
>  extern struct workqueue_struct *cifsiod_wq;
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 3a85df2a9baf..7bb94b269fd2 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -358,12 +358,30 @@ cifsFileInfo_get(struct cifsFileInfo *cifs_file)
>  	return cifs_file;
>  }
>  
> -/*
> - * Release a reference on the file private data. This may involve closing
> - * the filehandle out on the server. Must be called without holding
> - * tcon->open_file_lock and cifs_file->file_info_lock.
> +/**
> + * cifsFileInfo_put - release a reference of file priv data
> + *
> + * Always potentially wait for oplock handler. See _cifsFileInfo_put().
>   */
>  void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
> +{
> +	_cifsFileInfo_put(cifs_file, true);
> +}
> +
> +/**
> + * _cifsFileInfo_put - release a reference of file priv data
> + *
> + * This may involve closing the filehandle @cifs_file out on the
> + * server. Must be called without holding tcon->open_file_lock and
> + * cifs_file->file_info_lock.
> + *
> + * If @wait_for_oplock_handler is true and we are releasing the last
> + * reference, wait for any running oplock break handler of the file
> + * and cancel any pending one. If calling this function from the
> + * oplock break handler, you need to pass false.
> + *
> + */
> +void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_handler)
>  {
>  	struct inode *inode = d_inode(cifs_file->dentry);
>  	struct cifs_tcon *tcon = tlink_tcon(cifs_file->tlink);
> @@ -411,7 +429,8 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
>  
>  	spin_unlock(&tcon->open_file_lock);
>  
> -	oplock_break_cancelled = cancel_work_sync(&cifs_file->oplock_break);
> +	oplock_break_cancelled = wait_oplock_handler ?
> +		cancel_work_sync(&cifs_file->oplock_break) : false;
>  
>  	if (!tcon->need_reconnect && !cifs_file->invalidHandle) {
>  		struct TCP_Server_Info *server = tcon->ses->server;
> @@ -4097,6 +4116,7 @@ void cifs_oplock_break(struct work_struct *work)
>  							     cinode);
>  		cifs_dbg(FYI, "Oplock release rc = %d\n", rc);
>  	}
> +	_cifsFileInfo_put(cfile, false /* do not wait for ourself */);
>  	cifs_done_oplock_break(cinode);
>  }
>  
> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> index bcab30d4a6c7..76f1649ab444 100644
> --- a/fs/cifs/misc.c
> +++ b/fs/cifs/misc.c
> @@ -486,8 +486,7 @@ is_valid_oplock_break(char *buffer, struct TCP_Server_Info *srv)
>  					   CIFS_INODE_DOWNGRADE_OPLOCK_TO_L2,
>  					   &pCifsInode->flags);
>  
> -				queue_work(cifsoplockd_wq,
> -					   &netfile->oplock_break);
> +				cifs_queue_oplock_break(netfile);
>  				netfile->oplock_break_cancelled = false;
>  
>  				spin_unlock(&tcon->open_file_lock);
> @@ -584,6 +583,28 @@ void cifs_put_writer(struct cifsInodeInfo *cinode)
>  	spin_unlock(&cinode->writers_lock);
>  }
>  
> +/**
> + * cifs_queue_oplock_break - queue the oplock break handler for cfile
> + *
> + * This function is called from the demultiplex thread when it
> + * receives an oplock break for @cfile.
> + *
> + * Assumes the tcon->open_file_lock is held.
> + * Assumes cfile->file_info_lock is NOT held.
> + */
> +void cifs_queue_oplock_break(struct cifsFileInfo *cfile)
> +{
> +	/*
> +	 * Bump the handle refcount now while we hold the
> +	 * open_file_lock to enforce the validity of it for the oplock
> +	 * break handler. The matching put is done at the end of the
> +	 * handler.
> +	 */
> +	cifsFileInfo_get(cfile);
> +
> +	queue_work(cifsoplockd_wq, &cfile->oplock_break);
> +}
> +
>  void cifs_done_oplock_break(struct cifsInodeInfo *cinode)
>  {
>  	clear_bit(CIFS_INODE_PENDING_OPLOCK_BREAK, &cinode->flags);
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index efdfdb47a7dd..bba371071ac6 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -506,7 +506,7 @@ smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp,
>  		else
>  			cfile->oplock_break_cancelled = true;
>  
> -		queue_work(cifsoplockd_wq, &cfile->oplock_break);
> +		cifs_queue_oplock_break(cfile);
>  		kfree(lw);
>  		return true;
>  	}
> @@ -650,8 +650,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server)
>  					   CIFS_INODE_DOWNGRADE_OPLOCK_TO_L2,
>  					   &cinode->flags);
>  				spin_unlock(&cfile->file_info_lock);
> -				queue_work(cifsoplockd_wq,
> -					   &cfile->oplock_break);
> +
> +				cifs_queue_oplock_break(cfile);
>  
>  				spin_unlock(&tcon->open_file_lock);
>  				spin_unlock(&cifs_tcp_ses_lock);
> -- 
> 2.22.0
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Guilherme Piccoli July 18, 2019, 3:31 p.m. UTC | #3
On Thu, Jul 18, 2019 at 10:23 AM Marcelo Henrique Cerri
<marcelo.cerri@canonical.com> wrote:
>
> Acked-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
>
> Is that needed for 4.4 too?
>

Hi Cerri, thanks for the ack!

CIFS maintainers sent this patch to stable, it's on 4.9/4.14/4.19 LTS
stable kernels, as well
as 5.0.y and newer, but not in 4.4.y. I've checked and seems
"backportable" to 4.4, code is
quite similar.

But since CIFS maintainers and Greg/Sash didn't merged the commit in
version 4.4 and we
have no reports of this issue in 4.4, I vote for being conservative
and keep it on 4.15+, not
diverging from stable tree. If you or anybody here disagree with that,
I'd be glad to proceed
with the SRU to Xenial.

Thanks,


Guilherme

Patch
diff mbox series

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 37b5ddf27ff1..edfa9e69fd10 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1189,6 +1189,7 @@  cifsFileInfo_get_locked(struct cifsFileInfo *cifs_file)
 }
 
 struct cifsFileInfo *cifsFileInfo_get(struct cifsFileInfo *cifs_file);
+void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_hdlr);
 void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
 
 #define CIFS_CACHE_READ_FLG	1
@@ -1693,6 +1694,7 @@  GLOBAL_EXTERN spinlock_t gidsidlock;
 #endif /* CONFIG_CIFS_ACL */
 
 void cifs_oplock_break(struct work_struct *work);
+void cifs_queue_oplock_break(struct cifsFileInfo *cfile);
 
 extern const struct slow_work_ops cifs_oplock_break_ops;
 extern struct workqueue_struct *cifsiod_wq;
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 3a85df2a9baf..7bb94b269fd2 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -358,12 +358,30 @@  cifsFileInfo_get(struct cifsFileInfo *cifs_file)
 	return cifs_file;
 }
 
-/*
- * Release a reference on the file private data. This may involve closing
- * the filehandle out on the server. Must be called without holding
- * tcon->open_file_lock and cifs_file->file_info_lock.
+/**
+ * cifsFileInfo_put - release a reference of file priv data
+ *
+ * Always potentially wait for oplock handler. See _cifsFileInfo_put().
  */
 void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
+{
+	_cifsFileInfo_put(cifs_file, true);
+}
+
+/**
+ * _cifsFileInfo_put - release a reference of file priv data
+ *
+ * This may involve closing the filehandle @cifs_file out on the
+ * server. Must be called without holding tcon->open_file_lock and
+ * cifs_file->file_info_lock.
+ *
+ * If @wait_for_oplock_handler is true and we are releasing the last
+ * reference, wait for any running oplock break handler of the file
+ * and cancel any pending one. If calling this function from the
+ * oplock break handler, you need to pass false.
+ *
+ */
+void _cifsFileInfo_put(struct cifsFileInfo *cifs_file, bool wait_oplock_handler)
 {
 	struct inode *inode = d_inode(cifs_file->dentry);
 	struct cifs_tcon *tcon = tlink_tcon(cifs_file->tlink);
@@ -411,7 +429,8 @@  void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
 
 	spin_unlock(&tcon->open_file_lock);
 
-	oplock_break_cancelled = cancel_work_sync(&cifs_file->oplock_break);
+	oplock_break_cancelled = wait_oplock_handler ?
+		cancel_work_sync(&cifs_file->oplock_break) : false;
 
 	if (!tcon->need_reconnect && !cifs_file->invalidHandle) {
 		struct TCP_Server_Info *server = tcon->ses->server;
@@ -4097,6 +4116,7 @@  void cifs_oplock_break(struct work_struct *work)
 							     cinode);
 		cifs_dbg(FYI, "Oplock release rc = %d\n", rc);
 	}
+	_cifsFileInfo_put(cfile, false /* do not wait for ourself */);
 	cifs_done_oplock_break(cinode);
 }
 
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index bcab30d4a6c7..76f1649ab444 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -486,8 +486,7 @@  is_valid_oplock_break(char *buffer, struct TCP_Server_Info *srv)
 					   CIFS_INODE_DOWNGRADE_OPLOCK_TO_L2,
 					   &pCifsInode->flags);
 
-				queue_work(cifsoplockd_wq,
-					   &netfile->oplock_break);
+				cifs_queue_oplock_break(netfile);
 				netfile->oplock_break_cancelled = false;
 
 				spin_unlock(&tcon->open_file_lock);
@@ -584,6 +583,28 @@  void cifs_put_writer(struct cifsInodeInfo *cinode)
 	spin_unlock(&cinode->writers_lock);
 }
 
+/**
+ * cifs_queue_oplock_break - queue the oplock break handler for cfile
+ *
+ * This function is called from the demultiplex thread when it
+ * receives an oplock break for @cfile.
+ *
+ * Assumes the tcon->open_file_lock is held.
+ * Assumes cfile->file_info_lock is NOT held.
+ */
+void cifs_queue_oplock_break(struct cifsFileInfo *cfile)
+{
+	/*
+	 * Bump the handle refcount now while we hold the
+	 * open_file_lock to enforce the validity of it for the oplock
+	 * break handler. The matching put is done at the end of the
+	 * handler.
+	 */
+	cifsFileInfo_get(cfile);
+
+	queue_work(cifsoplockd_wq, &cfile->oplock_break);
+}
+
 void cifs_done_oplock_break(struct cifsInodeInfo *cinode)
 {
 	clear_bit(CIFS_INODE_PENDING_OPLOCK_BREAK, &cinode->flags);
diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index efdfdb47a7dd..bba371071ac6 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -506,7 +506,7 @@  smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp,
 		else
 			cfile->oplock_break_cancelled = true;
 
-		queue_work(cifsoplockd_wq, &cfile->oplock_break);
+		cifs_queue_oplock_break(cfile);
 		kfree(lw);
 		return true;
 	}
@@ -650,8 +650,8 @@  smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server)
 					   CIFS_INODE_DOWNGRADE_OPLOCK_TO_L2,
 					   &cinode->flags);
 				spin_unlock(&cfile->file_info_lock);
-				queue_work(cifsoplockd_wq,
-					   &cfile->oplock_break);
+
+				cifs_queue_oplock_break(cfile);
 
 				spin_unlock(&tcon->open_file_lock);
 				spin_unlock(&cifs_tcp_ses_lock);