Skip to content

Commit b106657

Browse files
dhkts1smfrench
authored andcommitted
ksmbd: remove stale channels from all sessions on teardown
ksmbd_sessions_deregister() removes a connection's channels from other sessions' channel lists only while conn->binding is still set: if (conn->binding) { hash_for_each_safe(sessions_table, ...) ksmbd_chann_del(conn, sess); } conn->binding is a transient flag: it is cleared once a binding SESSION_SETUP completes, and also by a subsequent non-binding SESSION_SETUP on the same connection (a reauthentication on a bound channel, or a new SessionId==0 setup). A connection that has bound a channel into another session's ksmbd_chann_list and then clears conn->binding leaves that channel behind when it disconnects: the channel, whose chann->conn points at the now freed struct ksmbd_conn, stays on the owner session's list. When the owning connection later tears down, the second loop dereferences the stale channel: xa_for_each(&sess->ksmbd_chann_list, chann_id, chann) if (chann->conn != conn) ksmbd_conn_set_exiting(chann->conn); /* freed */ which is a use-after-free write into the freed ksmbd_conn (the same stale channel is also walked by show_proc_session() through /proc). The session is leaked as well, because its channel list never empties. Remove the conn->binding gate so a connection always removes its channels from every session on teardown. Fixes: faf8578 ("ksmbd: find bound sessions during reauthentication") Signed-off-by: Gil Portnoy <dddhkts1@gmail.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent 6103461 commit b106657

1 file changed

Lines changed: 11 additions & 14 deletions

File tree

fs/smb/server/mgmt/user_session.c

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -457,22 +457,19 @@ void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
457457
{
458458
struct ksmbd_session *sess;
459459
unsigned long id;
460+
struct hlist_node *tmp;
461+
int bkt;
460462

461463
down_write(&sessions_table_lock);
462-
if (conn->binding) {
463-
int bkt;
464-
struct hlist_node *tmp;
465-
466-
hash_for_each_safe(sessions_table, bkt, tmp, sess, hlist) {
467-
if (!ksmbd_chann_del(conn, sess) &&
468-
xa_empty(&sess->ksmbd_chann_list)) {
469-
hash_del(&sess->hlist);
470-
down_write(&conn->session_lock);
471-
xa_erase(&conn->sessions, sess->id);
472-
up_write(&conn->session_lock);
473-
if (atomic_dec_and_test(&sess->refcnt))
474-
ksmbd_session_destroy(sess);
475-
}
464+
hash_for_each_safe(sessions_table, bkt, tmp, sess, hlist) {
465+
if (!ksmbd_chann_del(conn, sess) &&
466+
xa_empty(&sess->ksmbd_chann_list)) {
467+
hash_del(&sess->hlist);
468+
down_write(&conn->session_lock);
469+
xa_erase(&conn->sessions, sess->id);
470+
up_write(&conn->session_lock);
471+
if (atomic_dec_and_test(&sess->refcnt))
472+
ksmbd_session_destroy(sess);
476473
}
477474
}
478475

0 commit comments

Comments
 (0)