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: e2f34481b2 ("cifsd: add server-side procedures for SMB3")
Signed-off-by: Ilan Dudnik <ilan.dudnik@safebreach.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
This commit is contained in:
Ilan Dudnik 2026-08-01 20:18:48 +03:00 committed by Namjae Jeon
parent 3a98de41b0
commit 29f74f0f2e

View File

@ -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);