mirror of
https://github.com/torvalds/linux.git
synced 2026-09-11 20:13:02 +02:00
ksmbd: fix tree connection use-after-free in smb2_tree_connect()
ksmbd_tree_conn_connect() publishes a new tree connection in
sess->tree_conns with a single reference and returns its pointer to
smb2_tree_connect(). The handler continues to initialize the object and
build the response after publication. A concurrent session logoff can
erase the connection and drop that reference, freeing the object while
the handler still uses it.
BUG: KASAN: slab-use-after-free in smb2_tree_connect+0xe3d/0xf90
smb2_tree_connect (fs/smb/server/smb2pdu.c:2872)
handle_ksmbd_work
process_one_work
worker_thread
kthread
After xa_store() succeeds, take a second reference before releasing
tree_conns_lock. The original reference belongs to the xarray entry and
the second belongs to the creating smb2_tree_connect() handler.
Keep the references balanced in every path:
- On normal exit or an error after publication, smb2_tree_connect()
drops its creator reference. Error cleanup also calls
ksmbd_tree_conn_disconnect(), which drops the xarray reference only if
it removes the exact entry.
- SMB2 TREE_DISCONNECT uses the same helper to remove the entry and drop
its xarray reference. The request's existing lookup reference remains
owned by the request and is released by the existing cleanup.
- Session LOGOFF removes each entry and drops its xarray reference. If
it wins the race, later cleanup sees that the entry is gone and does
not drop that reference again.
To enforce this ownership, claim the disconnected state and erase the
exact entry atomically under tree_conns_lock. This guarantees one drop
for the xarray reference and one drop by each in-flight user, regardless
of which teardown path wins. If logoff removes the entry before
initialization completes, fail the connect instead of marking the
detached object TREE_CONNECTED.
Fixes: 33b235a6e6 ("ksmbd: fix race condition between tree conn lookup and disconnect")
Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Cc: AutonomousCodeSecurity@microsoft.com
Cc: stable@vger.kernel.org
Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@linux.microsoft.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
This commit is contained in:
parent
0480cee8cc
commit
b5ec6c462a
|
|
@ -82,6 +82,8 @@ ksmbd_tree_conn_connect(struct ksmbd_work *work, const char *share_name)
|
|||
down_write(&sess->tree_conns_lock);
|
||||
ret = xa_err(xa_store(&sess->tree_conns, tree_conn->id, tree_conn,
|
||||
KSMBD_DEFAULT_GFP));
|
||||
if (!ret)
|
||||
atomic_inc(&tree_conn->refcount);
|
||||
up_write(&sess->tree_conns_lock);
|
||||
if (ret) {
|
||||
status.ret = -ENOMEM;
|
||||
|
|
@ -129,6 +131,12 @@ int ksmbd_tree_conn_disconnect(struct ksmbd_session *sess,
|
|||
struct ksmbd_tree_connect *tree_conn)
|
||||
{
|
||||
down_write(&sess->tree_conns_lock);
|
||||
if (tree_conn->t_state == TREE_DISCONNECTED ||
|
||||
xa_load(&sess->tree_conns, tree_conn->id) != tree_conn) {
|
||||
up_write(&sess->tree_conns_lock);
|
||||
return -ENOENT;
|
||||
}
|
||||
tree_conn->t_state = TREE_DISCONNECTED;
|
||||
xa_erase(&sess->tree_conns, tree_conn->id);
|
||||
up_write(&sess->tree_conns_lock);
|
||||
|
||||
|
|
|
|||
|
|
@ -2790,6 +2790,7 @@ int smb2_tree_connect(struct ksmbd_work *work)
|
|||
struct ksmbd_session *sess = work->sess;
|
||||
char *treename = NULL, *name = NULL;
|
||||
struct ksmbd_tree_conn_status status;
|
||||
struct ksmbd_tree_connect *tree_conn = NULL;
|
||||
struct ksmbd_share_config *share = NULL;
|
||||
int rc = -EINVAL;
|
||||
|
||||
|
|
@ -2817,6 +2818,7 @@ int smb2_tree_connect(struct ksmbd_work *work)
|
|||
|
||||
status = ksmbd_tree_conn_connect(work, name);
|
||||
if (status.ret == KSMBD_TREE_CONN_STATUS_OK) {
|
||||
tree_conn = status.tree_conn;
|
||||
rsp->hdr.Id.SyncId.TreeId = cpu_to_le32(status.tree_conn->id);
|
||||
share = status.tree_conn->share_conf;
|
||||
|
||||
|
|
@ -2860,8 +2862,15 @@ int smb2_tree_connect(struct ksmbd_work *work)
|
|||
status.tree_conn->posix_extensions = true;
|
||||
|
||||
down_write(&sess->tree_conns_lock);
|
||||
status.tree_conn->t_state = TREE_CONNECTED;
|
||||
if (status.tree_conn->t_state == TREE_DISCONNECTED) {
|
||||
status.ret = KSMBD_TREE_CONN_STATUS_ERROR;
|
||||
share = NULL;
|
||||
} else {
|
||||
status.tree_conn->t_state = TREE_CONNECTED;
|
||||
}
|
||||
up_write(&sess->tree_conns_lock);
|
||||
if (status.ret != KSMBD_TREE_CONN_STATUS_OK)
|
||||
goto out_err1;
|
||||
rsp->StructureSize = cpu_to_le16(16);
|
||||
out_err1:
|
||||
/*
|
||||
|
|
@ -2888,9 +2897,6 @@ int smb2_tree_connect(struct ksmbd_work *work)
|
|||
rc = ksmbd_iov_pin_rsp(work, rsp, sizeof(struct smb2_tree_connect_rsp));
|
||||
if (rc) {
|
||||
if (status.ret == KSMBD_TREE_CONN_STATUS_OK) {
|
||||
down_write(&sess->tree_conns_lock);
|
||||
status.tree_conn->t_state = TREE_DISCONNECTED;
|
||||
up_write(&sess->tree_conns_lock);
|
||||
ksmbd_tree_conn_disconnect(sess, status.tree_conn);
|
||||
status.tree_conn = NULL;
|
||||
}
|
||||
|
|
@ -2931,6 +2937,9 @@ int smb2_tree_connect(struct ksmbd_work *work)
|
|||
if (status.ret != KSMBD_TREE_CONN_STATUS_OK)
|
||||
smb2_set_err_rsp(work);
|
||||
|
||||
if (tree_conn)
|
||||
ksmbd_tree_connect_put(tree_conn);
|
||||
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -3034,17 +3043,6 @@ int smb2_tree_disconnect(struct ksmbd_work *work)
|
|||
|
||||
ksmbd_close_tree_conn_fds(work);
|
||||
|
||||
down_write(&sess->tree_conns_lock);
|
||||
if (tcon->t_state == TREE_DISCONNECTED) {
|
||||
up_write(&sess->tree_conns_lock);
|
||||
rsp->hdr.Status = STATUS_NETWORK_NAME_DELETED;
|
||||
err = -ENOENT;
|
||||
goto err_out;
|
||||
}
|
||||
|
||||
tcon->t_state = TREE_DISCONNECTED;
|
||||
up_write(&sess->tree_conns_lock);
|
||||
|
||||
err = ksmbd_tree_conn_disconnect(sess, tcon);
|
||||
if (err) {
|
||||
rsp->hdr.Status = STATUS_NETWORK_NAME_DELETED;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user