From 29f74f0f2e6df3b393b7b66e810136d0c64e3c59 Mon Sep 17 00:00:00 2001 From: Ilan Dudnik Date: Sat, 1 Aug 2026 20:18:48 +0300 Subject: [PATCH] ksmbd: defer publishing granted locks to prevent UAF/double-free race In smb2_lock(), mid-batch granted locks are published to connection-wide (conn->lock_list) and file-wide (fp->lock_list) lists immediately upon vfs_lock_file() success, while also remaining tracked on the stack-local rollback_list. If a subsequent element in the same SMB2_LOCK request array fails validation or execution, the thread jumps to out: and walks rollback_list to undo previously granted locks. However, because the granted lock was already published to conn->lock_list, a concurrent UNLOCK request on the same connection can find the lock object and kfree() it before the rollback loop executes. When the granting thread subsequently walks rollback_list, it dereferences and frees the already-freed ksmbd_lock structure, resulting in a Use-After-Free and Double-Free (on both ksmbd_lock and struct file_lock). Fix this by deferring the publication of granted locks to conn->lock_list and fp->lock_list until after the entire array of lock elements has been processed without error. Mid-batch grants remain tracked exclusively on the request-local rollback_list until the whole batch succeeds, eliminating the race window. Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Signed-off-by: Ilan Dudnik Signed-off-by: Namjae Jeon --- fs/smb/server/smb2pdu.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c index 9b64e28b5d56..2039a44d4b17 100644 --- a/fs/smb/server/smb2pdu.c +++ b/fs/smb/server/smb2pdu.c @@ -9498,16 +9498,10 @@ int smb2_lock(struct ksmbd_work *work) rsp->hdr.Status = STATUS_RANGE_NOT_LOCKED; kfree(smb_lock); - goto out2; + /* rollback_list may still hold earlier grants */ + goto out; } else if (!rc) { list_add(&smb_lock->llist, &rollback_list); - smb_lock->conn = ksmbd_conn_get(work->conn); - spin_lock(&work->conn->llist_lock); - list_add_tail(&smb_lock->clist, - &work->conn->lock_list); - list_add_tail(&smb_lock->flist, - &fp->lock_list); - spin_unlock(&work->conn->llist_lock); ksmbd_debug(SMB, "successful in taking lock\n"); } else { locks_free_lock(flock); @@ -9529,6 +9523,20 @@ int smb2_lock(struct ksmbd_work *work) err = ksmbd_iov_pin_rsp(work, rsp, sizeof(struct smb2_lock_rsp)); if (err) goto out; + + /* publish only once the whole batch has committed */ + if (!list_empty(&rollback_list)) { + spin_lock(&work->conn->llist_lock); + list_for_each_entry(smb_lock, &rollback_list, llist) { + smb_lock->conn = ksmbd_conn_get(work->conn); + list_add_tail(&smb_lock->clist, + &work->conn->lock_list); + list_add_tail(&smb_lock->flist, + &fp->lock_list); + } + spin_unlock(&work->conn->llist_lock); + } + if (!lock_replayed) smb2_update_lock_sequence(work, fp, req); @@ -9559,15 +9567,6 @@ int smb2_lock(struct ksmbd_work *work) } list_del(&smb_lock->llist); - conn = smb_lock->conn; - spin_lock(&conn->llist_lock); - if (!list_empty(&smb_lock->flist)) - list_del(&smb_lock->flist); - list_del(&smb_lock->clist); - smb_lock->conn = NULL; - spin_unlock(&conn->llist_lock); - ksmbd_conn_put(conn); - locks_free_lock(smb_lock->fl); if (rlock) locks_free_lock(rlock);