smb: client: avoid leaking refcount in cifs_queue_oplock_break()

PATCH Applied Bjoern Doebel 3 participants 3 messages Sep 3 – Sep 5

Subsystems (3)

Stable branch SMB client (CIFS) Kernel SMB3 server (ksmbd)

lore

Share & tools

lore

Bjoern Doebel

author

2026-09-03 21:29 UTC #0 lore raw

PATCH 1/1 smb: client: avoid leaking refcount in cifs_queue_oplock_break()

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 [email protected]

Assisted-by Kiro:claude-opus-5

Signed-off-by Bjoern Doebel <[email protected]>

---
 fs/smb/client/misc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

fs/smb/client/misc.c +4 -3

index 46e1382e8e04b..dc7070d70467f 100644
@@ -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)

Signature · 1 line

2.50.1

Namjae Jeon

maintainer Reviewed-by on v1 1/1

2026-09-04 02:26 UTC #1 lore raw

Quoted 24 lines · Bjoern 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.

Quoted 4 lines

Fixes b98749cac4a69 ("CIFS: keep FileInfo handle live during oplock break")
Cc [email protected]
Assisted-by Kiro:claude-opus-5
Signed-off-by Bjoern Doebel <[email protected]>

This seems to fix the issue previously reported:

https://lore.kernel.org/linux-cifs/[email protected]/T/#u

Reviewed-by Namjae Jeon <[email protected]>

Thanks!

Paulo Alcantara

maintainer Applied

2026-09-05 23:21 UTC #2 lore raw

In reply to Bjoern Doebel

Quoted 16 lines · Bjoern 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!

Quoted 7 lines

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.
...

Applied.