public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ksmbd: fix use-after-free in __ksmbd_close_fd() lock cleanup
@ 2026-04-02  8:39 munan Huang
  2026-04-04  2:28 ` Namjae Jeon
  0 siblings, 1 reply; 5+ messages in thread
From: munan Huang @ 2026-04-02  8:39 UTC (permalink / raw)
  To: linkinjeon, smfrench, senozhatsky, tom
  Cc: linux-cifs, linux-kernel, munan Huang, stable

In __ksmbd_close_fd(), when cleaning up byte-range locks on a durable
file handle closed by the scavenger, the lock cleanup loop
unconditionally dereferences fp->conn->llist_lock to remove each lock
from the connection's list:

  list_for_each_entry_safe(smb_lock, tmp_lock, &fp->lock_list, flist) {
      spin_lock(&fp->conn->llist_lock);
      list_del(&smb_lock->clist);
      spin_unlock(&fp->conn->llist_lock);
  }

However, when a client disconnects without SMB2 LOGOFF, ksmbd preserves
durable file handles via session_fd_check(), which sets fp->conn to
NULL and arms the durable scavenger timeout, but does not detach the
byte-range locks from the dying connection's lock list.

When the scavenger timeout expires, ksmbd_durable_scavenger() calls
__ksmbd_close_fd(NULL, fp). At this point fp->conn is NULL and the
original connection object has already been freed by ksmbd_conn_free(),
so it would cause a use-after-free or NULL pointer dereference.

Fix by checking fp->conn for NULL before accessing fp->conn->llist_lock
in the lock cleanup loop. 

Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Cc: stable@vger.kernel.org
Signed-off-by: munan Huang <munanevil@gmail.com>
---
 fs/smb/server/vfs_cache.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index 168f2dd7e200..772a84d95fe3 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -463,9 +463,11 @@ static void __ksmbd_close_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp)
 	 * there are not accesses to fp->lock_list.
 	 */
 	list_for_each_entry_safe(smb_lock, tmp_lock, &fp->lock_list, flist) {
-		spin_lock(&fp->conn->llist_lock);
-		list_del(&smb_lock->clist);
-		spin_unlock(&fp->conn->llist_lock);
+		if (fp->conn) {
+			spin_lock(&fp->conn->llist_lock);
+			list_del(&smb_lock->clist);
+			spin_unlock(&fp->conn->llist_lock);
+		}
 
 		list_del(&smb_lock->flist);
 		locks_free_lock(smb_lock->fl);
-- 
2.34.1


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

* Re: [PATCH] ksmbd: fix use-after-free in __ksmbd_close_fd() lock cleanup
  2026-04-02  8:39 [PATCH] ksmbd: fix use-after-free in __ksmbd_close_fd() lock cleanup munan Huang
@ 2026-04-04  2:28 ` Namjae Jeon
  2026-04-04  5:02   ` ChenXiaoSong
  0 siblings, 1 reply; 5+ messages in thread
From: Namjae Jeon @ 2026-04-04  2:28 UTC (permalink / raw)
  To: munan Huang; +Cc: smfrench, senozhatsky, tom, linux-cifs, linux-kernel, stable

On Thu, Apr 2, 2026 at 5:39 PM munan Huang <munanevil@gmail.com> wrote:
>
> In __ksmbd_close_fd(), when cleaning up byte-range locks on a durable
> file handle closed by the scavenger, the lock cleanup loop
> unconditionally dereferences fp->conn->llist_lock to remove each lock
> from the connection's list:
>
>   list_for_each_entry_safe(smb_lock, tmp_lock, &fp->lock_list, flist) {
>       spin_lock(&fp->conn->llist_lock);
>       list_del(&smb_lock->clist);
>       spin_unlock(&fp->conn->llist_lock);
>   }
>
> However, when a client disconnects without SMB2 LOGOFF, ksmbd preserves
> durable file handles via session_fd_check(), which sets fp->conn to
> NULL and arms the durable scavenger timeout, but does not detach the
> byte-range locks from the dying connection's lock list.
>
> When the scavenger timeout expires, ksmbd_durable_scavenger() calls
> __ksmbd_close_fd(NULL, fp). At this point fp->conn is NULL and the
> original connection object has already been freed by ksmbd_conn_free(),
> so it would cause a use-after-free or NULL pointer dereference.
>
> Fix by checking fp->conn for NULL before accessing fp->conn->llist_lock
> in the lock cleanup loop.
>
> Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
> Cc: stable@vger.kernel.org
> Signed-off-by: munan Huang <munanevil@gmail.com>
I will apply the following patch instead of your patch. Let me know if
I am missing something.
https://github.com/smfrench/smb3-kernel/commit/319ca5432460b0749e420f7cff637dfbc7e16be3
Thanks.

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

* Re: [PATCH] ksmbd: fix use-after-free in __ksmbd_close_fd() lock cleanup
  2026-04-04  2:28 ` Namjae Jeon
@ 2026-04-04  5:02   ` ChenXiaoSong
  2026-04-04 14:28     ` Namjae Jeon
  0 siblings, 1 reply; 5+ messages in thread
From: ChenXiaoSong @ 2026-04-04  5:02 UTC (permalink / raw)
  To: Namjae Jeon, munan Huang
  Cc: smfrench, senozhatsky, tom, linux-cifs, linux-kernel, stable

Hi Namjae and munan,

In `ksmbd_reopen_durable_fd()`, when -EBADF is returned, should 
`list_del(&smb_lock->clist)` be called?

If my understanding is incorrect, please let me know.

> 
> int ksmbd_reopen_durable_fd(struct ksmbd_work *work, struct ksmbd_file *fp)
> {
> 	...
> 	fp->conn = conn;
> 	...
> 
> 	list_for_each_entry(smb_lock, &fp->lock_list, flist) {
> 		spin_lock(&conn->llist_lock);
> 		list_add_tail(&smb_lock->clist, &conn->lock_list);
> 		spin_unlock(&conn->llist_lock);
> 	}
> 	...
> 	__open_id(&work->sess->file_table, fp, OPEN_ID_TYPE_VOLATILE_ID);
> 	if (!has_file_id(fp->volatile_id)) {
> 		fp->conn = NULL;
> 		fp->tcon = NULL;
> 		return -EBADF;
> 	}
> 	return 0;
> }

Thanks,
ChenXiaoSong <chenxiaosong@kylinos.cn>

On 4/4/26 10:28, Namjae Jeon wrote:
> I will apply the following patch instead of your patch. Let me know if
> I am missing something.
> https://github.com/smfrench/smb3-kernel/ 
> commit/319ca5432460b0749e420f7cff637dfbc7e16be3
> Thanks.


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

* Re: [PATCH] ksmbd: fix use-after-free in __ksmbd_close_fd() lock cleanup
  2026-04-04  5:02   ` ChenXiaoSong
@ 2026-04-04 14:28     ` Namjae Jeon
  2026-04-04 14:59       ` ChenXiaoSong
  0 siblings, 1 reply; 5+ messages in thread
From: Namjae Jeon @ 2026-04-04 14:28 UTC (permalink / raw)
  To: ChenXiaoSong
  Cc: munan Huang, smfrench, senozhatsky, tom, linux-cifs, linux-kernel,
	stable

On Sat, Apr 4, 2026 at 2:03 PM ChenXiaoSong
<chenxiaosong@chenxiaosong.com> wrote:
>
> Hi Namjae and munan,
>
> In `ksmbd_reopen_durable_fd()`, when -EBADF is returned, should
> `list_del(&smb_lock->clist)` be called?
>
> If my understanding is incorrect, please let me know.
I have updated the patch. Please check it.
https://github.com/smfrench/smb3-kernel/commit/38bf2f4ac44b0848677fd4d539404b8c0de15b98

Thanks for the review!

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

* Re: [PATCH] ksmbd: fix use-after-free in __ksmbd_close_fd() lock cleanup
  2026-04-04 14:28     ` Namjae Jeon
@ 2026-04-04 14:59       ` ChenXiaoSong
  0 siblings, 0 replies; 5+ messages in thread
From: ChenXiaoSong @ 2026-04-04 14:59 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: munan Huang, smfrench, senozhatsky, tom, linux-cifs, linux-kernel,
	stable

Looks good to me. Feel free to add:
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>

On 2026/4/4 22:28, Namjae Jeon wrote:
> I have updated the patch. Please check it.
> https://github.com/smfrench/smb3-kernel/commit/38bf2f4ac44b0848677fd4d539404b8c0de15b98
> 
> Thanks for the review!

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

end of thread, other threads:[~2026-04-04 15:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-02  8:39 [PATCH] ksmbd: fix use-after-free in __ksmbd_close_fd() lock cleanup munan Huang
2026-04-04  2:28 ` Namjae Jeon
2026-04-04  5:02   ` ChenXiaoSong
2026-04-04 14:28     ` Namjae Jeon
2026-04-04 14:59       ` ChenXiaoSong

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