mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] smb: client: avoid leaking refcount in cifs_queue_oplock_break()
@ 2026-09-03 21:28 Bjoern Doebel
  2026-09-04  2:26 ` Namjae Jeon
  0 siblings, 1 reply; 2+ messages in thread
From: Bjoern Doebel @ 2026-09-03 21:28 UTC (permalink / raw)
  To: Paulo Alcantara, Namjae Jeon
  Cc: stable, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
	linux-cifs, samba-technical, linux-kernel, Aurelien Aptel,
	doebel

cifs_queue_oplock_break() unconditionally takes a reference on the
target file before queueing cifs_oplock_break(). Only that work item
decreases the reference counter again.

If another oplock break arrives while that work is still queued,
queue_work() will return false and not queue this second work item. As a
result, we will never reach the point to drop the file reference again
and are leaking this reference. This can be triggered when interacting
with a slow-responding server.

As a result, later unmount operations for this file system will fail with

  BUG: Dentry ... still in use (1) [unmount of cifs cifs]
  VFS: Busy inodes after unmount of cifs (cifs)
  kernel BUG at fs/super.c:777!

Fix this by only incrementing the reference count if the work has been
queued successfully. Taking it after queue_work() is safe because all
three callers hold tcon->open_file_lock across the call and
_cifsFileInfo_put() decrements under that same lock, so a worker that
starts the handler in the window cannot drop the reference before it has
been taken.

Fixes: b98749cac4a69 ("CIFS: keep FileInfo handle live during oplock break")
Cc: stable@vger.kernel.org
Assisted-by: Kiro:claude-opus-5
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
---
 fs/smb/client/misc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index 46e1382e8e04b..dc7070d70467f 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -378,10 +378,11 @@ void cifs_queue_oplock_break(struct cifsFileInfo *cfile)
 	 * 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.
+	 *
+	 * Only take a reference if the work is actually queued.
 	 */
-	cifsFileInfo_get(cfile);
-
-	queue_work(cifsoplockd_wq, &cfile->oplock_break);
+	if (queue_work(cifsoplockd_wq, &cfile->oplock_break))
+		cifsFileInfo_get(cfile);
 }
 
 void cifs_done_oplock_break(struct cifsInodeInfo *cinode)
-- 
2.50.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] smb: client: avoid leaking refcount in cifs_queue_oplock_break()
  2026-09-03 21:28 [PATCH] smb: client: avoid leaking refcount in cifs_queue_oplock_break() Bjoern Doebel
@ 2026-09-04  2:26 ` Namjae Jeon
  0 siblings, 0 replies; 2+ messages in thread
From: Namjae Jeon @ 2026-09-04  2:26 UTC (permalink / raw)
  To: Bjoern Doebel
  Cc: Paulo Alcantara, stable, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, linux-cifs, samba-technical,
	linux-kernel, Aurelien Aptel

On Fri, Sep 4, 2026 at 6:29 AM Bjoern Doebel <doebel@amazon.de> wrote:
>
> cifs_queue_oplock_break() unconditionally takes a reference on the
> target file before queueing cifs_oplock_break(). Only that work item
> decreases the reference counter again.
>
> If another oplock break arrives while that work is still queued,
> queue_work() will return false and not queue this second work item. As a
> result, we will never reach the point to drop the file reference again
> and are leaking this reference. This can be triggered when interacting
> with a slow-responding server.
>
> As a result, later unmount operations for this file system will fail with
>
>   BUG: Dentry ... still in use (1) [unmount of cifs cifs]
>   VFS: Busy inodes after unmount of cifs (cifs)
>   kernel BUG at fs/super.c:777!
>
> Fix this by only incrementing the reference count if the work has been
> queued successfully. Taking it after queue_work() is safe because all
> three callers hold tcon->open_file_lock across the call and
> _cifsFileInfo_put() decrements under that same lock, so a worker that
> starts the handler in the window cannot drop the reference before it has
> been taken.
>
> Fixes: b98749cac4a69 ("CIFS: keep FileInfo handle live during oplock break")
> Cc: stable@vger.kernel.org
> Assisted-by: Kiro:claude-opus-5
> Signed-off-by: Bjoern Doebel <doebel@amazon.de>
This seems to fix the issue previously reported:

  https://lore.kernel.org/linux-cifs/8a5fd5ca-9f77-42f0-9141-d5d88c1df067@chenxiaosong.com/T/#u

Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>

Thanks!

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-04  2:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-03 21:28 [PATCH] smb: client: avoid leaking refcount in cifs_queue_oplock_break() Bjoern Doebel
2026-09-04  2:26 ` Namjae Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®