Skip to content

Commit 235e323

Browse files
namjaejeonsmfrench
authored andcommitted
ksmbd: fix use-after-free in __ksmbd_close_fd() via durable scavenger
When a durable file handle survives session disconnect (TCP close without SMB2_LOGOFF), session_fd_check() sets fp->conn = NULL to preserve the handle for later reconnection. However, it did not clean up the byte-range locks on fp->lock_list. Later, when the durable scavenger thread times out and calls __ksmbd_close_fd(NULL, fp), the lock cleanup loop did: spin_lock(&fp->conn->llist_lock); This caused a slab use-after-free because fp->conn was NULL and the original connection object had already been freed by ksmbd_tcp_disconnect(). The root cause is asymmetric cleanup: lock entries (smb_lock->clist) were left dangling on the freed conn->lock_list while fp->conn was nulled out. To fix this issue properly, we need to handle the lifetime of smb_lock->clist across three paths: - Safely skip clist deletion when list is empty and fp->conn is NULL. - Remove the lock from the old connection's lock_list in session_fd_check() - Re-add the lock to the new connection's lock_list in ksmbd_reopen_durable_fd(). Fixes: c8efcc7 ("ksmbd: add support for durable handles v1/v2") Co-developed-by: munan Huang <munanevil@gmail.com> Signed-off-by: munan Huang <munanevil@gmail.com> Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent 3df614e commit 235e323

1 file changed

Lines changed: 30 additions & 11 deletions

File tree

fs/smb/server/vfs_cache.c

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -463,9 +463,11 @@ static void __ksmbd_close_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp)
463463
* there are not accesses to fp->lock_list.
464464
*/
465465
list_for_each_entry_safe(smb_lock, tmp_lock, &fp->lock_list, flist) {
466-
spin_lock(&fp->conn->llist_lock);
467-
list_del(&smb_lock->clist);
468-
spin_unlock(&fp->conn->llist_lock);
466+
if (!list_empty(&smb_lock->clist) && fp->conn) {
467+
spin_lock(&fp->conn->llist_lock);
468+
list_del(&smb_lock->clist);
469+
spin_unlock(&fp->conn->llist_lock);
470+
}
469471

470472
list_del(&smb_lock->flist);
471473
locks_free_lock(smb_lock->fl);
@@ -995,6 +997,7 @@ static bool session_fd_check(struct ksmbd_tree_connect *tcon,
995997
struct ksmbd_inode *ci;
996998
struct oplock_info *op;
997999
struct ksmbd_conn *conn;
1000+
struct ksmbd_lock *smb_lock, *tmp_lock;
9981001

9991002
if (!is_reconnectable(fp))
10001003
return false;
@@ -1011,6 +1014,12 @@ static bool session_fd_check(struct ksmbd_tree_connect *tcon,
10111014
}
10121015
up_write(&ci->m_lock);
10131016

1017+
list_for_each_entry_safe(smb_lock, tmp_lock, &fp->lock_list, flist) {
1018+
spin_lock(&fp->conn->llist_lock);
1019+
list_del_init(&smb_lock->clist);
1020+
spin_unlock(&fp->conn->llist_lock);
1021+
}
1022+
10141023
fp->conn = NULL;
10151024
fp->tcon = NULL;
10161025
fp->volatile_id = KSMBD_NO_FID;
@@ -1090,6 +1099,9 @@ int ksmbd_reopen_durable_fd(struct ksmbd_work *work, struct ksmbd_file *fp)
10901099
{
10911100
struct ksmbd_inode *ci;
10921101
struct oplock_info *op;
1102+
struct ksmbd_conn *conn = work->conn;
1103+
struct ksmbd_lock *smb_lock;
1104+
unsigned int old_f_state;
10931105

10941106
if (!fp->is_durable || fp->conn || fp->tcon) {
10951107
pr_err("Invalid durable fd [%p:%p]\n", fp->conn, fp->tcon);
@@ -1101,9 +1113,23 @@ int ksmbd_reopen_durable_fd(struct ksmbd_work *work, struct ksmbd_file *fp)
11011113
return -EBADF;
11021114
}
11031115

1104-
fp->conn = work->conn;
1116+
old_f_state = fp->f_state;
1117+
fp->f_state = FP_NEW;
1118+
__open_id(&work->sess->file_table, fp, OPEN_ID_TYPE_VOLATILE_ID);
1119+
if (!has_file_id(fp->volatile_id)) {
1120+
fp->f_state = old_f_state;
1121+
return -EBADF;
1122+
}
1123+
1124+
fp->conn = conn;
11051125
fp->tcon = work->tcon;
11061126

1127+
list_for_each_entry(smb_lock, &fp->lock_list, flist) {
1128+
spin_lock(&conn->llist_lock);
1129+
list_add_tail(&smb_lock->clist, &conn->lock_list);
1130+
spin_unlock(&conn->llist_lock);
1131+
}
1132+
11071133
ci = fp->f_ci;
11081134
down_write(&ci->m_lock);
11091135
list_for_each_entry_rcu(op, &ci->m_op_list, op_entry) {
@@ -1114,13 +1140,6 @@ int ksmbd_reopen_durable_fd(struct ksmbd_work *work, struct ksmbd_file *fp)
11141140
}
11151141
up_write(&ci->m_lock);
11161142

1117-
fp->f_state = FP_NEW;
1118-
__open_id(&work->sess->file_table, fp, OPEN_ID_TYPE_VOLATILE_ID);
1119-
if (!has_file_id(fp->volatile_id)) {
1120-
fp->conn = NULL;
1121-
fp->tcon = NULL;
1122-
return -EBADF;
1123-
}
11241143
return 0;
11251144
}
11261145

0 commit comments

Comments
 (0)