mirror of
https://github.com/torvalds/linux.git
synced 2026-07-28 01:55:51 +02:00
ksmbd: fix durable reconnect double-bind race in ksmbd_reopen_durable_fd
Two concurrent same-user DHnC reconnects can both observe fp->conn == NULL
before either sets it. ksmbd_reopen_durable_fd() checks fp->conn to guard
against a handle already being reconnected, but the check and the binding
assignment are not atomic: both threads pass the guard, both call
ksmbd_conn_get() on the same fp, and both eventually reach
kfree(fp->owner.name) -- a double-free of the owner.name slab object.
The double-bound ksmbd_file also causes a write-UAF on the 344-byte
ksmbd_file_cache object when a concurrent smb2_close() spins on fp->f_lock
after the object has been freed by the losing reconnect path.
KASAN on 7.1-rc5 (48-thread concurrent reconnect, 3000 cycles):
BUG: KASAN: double-free in ksmbd_reopen_durable_fd+0x268/0x308
BUG: KASAN: slab-use-after-free in _raw_spin_lock+0xac/0x150
Write of size 4 at offset 24 into freed ksmbd_file_cache object
Five double-bind windows observed; 63 total KASAN reports triggered.
Fix: validate and claim fp->conn under write_lock(&global_ft.lock) so the
check-and-claim is atomic. ksmbd_lookup_durable_fd() already treats
fp->conn != NULL as "in use" and skips such an fp; setting fp->conn before
dropping the lock closes the race. ksmbd_conn_get() is a non-sleeping
refcount increment, safe under the rwlock. The rollback path on __open_id()
failure also clears fp->conn/tcon under the lock so concurrent readers see
a consistent state.
Fixes: b1f1e80620 ("ksmbd: centralize ksmbd_conn final release to plug transport leak")
Assisted-by: Henry (Claude):claude-opus-4
Signed-off-by: Gil Portnoy <dddhkts1@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
This commit is contained in:
parent
b003086d76
commit
7ce4fc4001
|
|
@ -1390,19 +1390,19 @@ int ksmbd_reopen_durable_fd(struct ksmbd_work *work, struct ksmbd_file *fp)
|
|||
struct ksmbd_lock *smb_lock;
|
||||
unsigned int old_f_state;
|
||||
|
||||
write_lock(&global_ft.lock);
|
||||
if (!fp->is_durable || fp->conn || fp->tcon) {
|
||||
write_unlock(&global_ft.lock);
|
||||
pr_err("Invalid durable fd [%p:%p]\n", fp->conn, fp->tcon);
|
||||
return -EBADF;
|
||||
}
|
||||
|
||||
if (has_file_id(fp->volatile_id)) {
|
||||
write_unlock(&global_ft.lock);
|
||||
pr_err("Still in use durable fd: %llu\n", fp->volatile_id);
|
||||
return -EBADF;
|
||||
}
|
||||
|
||||
old_f_state = fp->f_state;
|
||||
fp->f_state = FP_NEW;
|
||||
|
||||
/*
|
||||
* Initialize fp's connection binding before publishing fp into the
|
||||
* session's file table. If __open_id() is ordered first, a
|
||||
|
|
@ -1413,11 +1413,17 @@ int ksmbd_reopen_durable_fd(struct ksmbd_work *work, struct ksmbd_file *fp)
|
|||
*/
|
||||
fp->conn = ksmbd_conn_get(conn);
|
||||
fp->tcon = work->tcon;
|
||||
write_unlock(&global_ft.lock);
|
||||
|
||||
old_f_state = fp->f_state;
|
||||
fp->f_state = FP_NEW;
|
||||
|
||||
__open_id(&work->sess->file_table, fp, OPEN_ID_TYPE_VOLATILE_ID);
|
||||
if (!has_file_id(fp->volatile_id)) {
|
||||
write_lock(&global_ft.lock);
|
||||
fp->conn = NULL;
|
||||
fp->tcon = NULL;
|
||||
write_unlock(&global_ft.lock);
|
||||
ksmbd_conn_put(conn);
|
||||
fp->f_state = old_f_state;
|
||||
return -EBADF;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user