Commit f5c779b7 authored by Namjae Jeon's avatar Namjae Jeon Committed by Steve French

ksmbd: fix racy issue from session setup and logoff

This racy issue is triggered by sending concurrent session setup and
logoff requests. This patch does not set connection status as
KSMBD_SESS_GOOD if state is KSMBD_SESS_NEED_RECONNECT in session setup.
And relookup session to validate if session is deleted in logoff.

Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-20481, ZDI-CAN-20590, ZDI-CAN-20596
Signed-off-by: default avatarNamjae Jeon <linkinjeon@kernel.org>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 3ac00a2a
...@@ -56,7 +56,7 @@ struct ksmbd_conn *ksmbd_conn_alloc(void) ...@@ -56,7 +56,7 @@ struct ksmbd_conn *ksmbd_conn_alloc(void)
return NULL; return NULL;
conn->need_neg = true; conn->need_neg = true;
conn->status = KSMBD_SESS_NEW; ksmbd_conn_set_new(conn);
conn->local_nls = load_nls("utf8"); conn->local_nls = load_nls("utf8");
if (!conn->local_nls) if (!conn->local_nls)
conn->local_nls = load_nls_default(); conn->local_nls = load_nls_default();
...@@ -147,12 +147,12 @@ int ksmbd_conn_try_dequeue_request(struct ksmbd_work *work) ...@@ -147,12 +147,12 @@ int ksmbd_conn_try_dequeue_request(struct ksmbd_work *work)
return ret; return ret;
} }
static void ksmbd_conn_lock(struct ksmbd_conn *conn) void ksmbd_conn_lock(struct ksmbd_conn *conn)
{ {
mutex_lock(&conn->srv_mutex); mutex_lock(&conn->srv_mutex);
} }
static void ksmbd_conn_unlock(struct ksmbd_conn *conn) void ksmbd_conn_unlock(struct ksmbd_conn *conn)
{ {
mutex_unlock(&conn->srv_mutex); mutex_unlock(&conn->srv_mutex);
} }
...@@ -243,7 +243,7 @@ bool ksmbd_conn_alive(struct ksmbd_conn *conn) ...@@ -243,7 +243,7 @@ bool ksmbd_conn_alive(struct ksmbd_conn *conn)
if (!ksmbd_server_running()) if (!ksmbd_server_running())
return false; return false;
if (conn->status == KSMBD_SESS_EXITING) if (ksmbd_conn_exiting(conn))
return false; return false;
if (kthread_should_stop()) if (kthread_should_stop())
...@@ -303,7 +303,7 @@ int ksmbd_conn_handler_loop(void *p) ...@@ -303,7 +303,7 @@ int ksmbd_conn_handler_loop(void *p)
pdu_size = get_rfc1002_len(hdr_buf); pdu_size = get_rfc1002_len(hdr_buf);
ksmbd_debug(CONN, "RFC1002 header %u bytes\n", pdu_size); ksmbd_debug(CONN, "RFC1002 header %u bytes\n", pdu_size);
if (conn->status == KSMBD_SESS_GOOD) if (ksmbd_conn_good(conn))
max_allowed_pdu_size = max_allowed_pdu_size =
SMB3_MAX_MSGSIZE + conn->vals->max_write_size; SMB3_MAX_MSGSIZE + conn->vals->max_write_size;
else else
...@@ -312,7 +312,7 @@ int ksmbd_conn_handler_loop(void *p) ...@@ -312,7 +312,7 @@ int ksmbd_conn_handler_loop(void *p)
if (pdu_size > max_allowed_pdu_size) { if (pdu_size > max_allowed_pdu_size) {
pr_err_ratelimited("PDU length(%u) exceeded maximum allowed pdu size(%u) on connection(%d)\n", pr_err_ratelimited("PDU length(%u) exceeded maximum allowed pdu size(%u) on connection(%d)\n",
pdu_size, max_allowed_pdu_size, pdu_size, max_allowed_pdu_size,
conn->status); READ_ONCE(conn->status));
break; break;
} }
...@@ -416,7 +416,7 @@ static void stop_sessions(void) ...@@ -416,7 +416,7 @@ static void stop_sessions(void)
if (task) if (task)
ksmbd_debug(CONN, "Stop session handler %s/%d\n", ksmbd_debug(CONN, "Stop session handler %s/%d\n",
task->comm, task_pid_nr(task)); task->comm, task_pid_nr(task));
conn->status = KSMBD_SESS_EXITING; ksmbd_conn_set_exiting(conn);
if (t->ops->shutdown) { if (t->ops->shutdown) {
read_unlock(&conn_list_lock); read_unlock(&conn_list_lock);
t->ops->shutdown(t); t->ops->shutdown(t);
......
...@@ -162,6 +162,8 @@ void ksmbd_conn_init_server_callbacks(struct ksmbd_conn_ops *ops); ...@@ -162,6 +162,8 @@ void ksmbd_conn_init_server_callbacks(struct ksmbd_conn_ops *ops);
int ksmbd_conn_handler_loop(void *p); int ksmbd_conn_handler_loop(void *p);
int ksmbd_conn_transport_init(void); int ksmbd_conn_transport_init(void);
void ksmbd_conn_transport_destroy(void); void ksmbd_conn_transport_destroy(void);
void ksmbd_conn_lock(struct ksmbd_conn *conn);
void ksmbd_conn_unlock(struct ksmbd_conn *conn);
/* /*
* WARNING * WARNING
...@@ -169,43 +171,48 @@ void ksmbd_conn_transport_destroy(void); ...@@ -169,43 +171,48 @@ void ksmbd_conn_transport_destroy(void);
* This is a hack. We will move status to a proper place once we land * This is a hack. We will move status to a proper place once we land
* a multi-sessions support. * a multi-sessions support.
*/ */
static inline bool ksmbd_conn_good(struct ksmbd_work *work) static inline bool ksmbd_conn_good(struct ksmbd_conn *conn)
{ {
return work->conn->status == KSMBD_SESS_GOOD; return READ_ONCE(conn->status) == KSMBD_SESS_GOOD;
} }
static inline bool ksmbd_conn_need_negotiate(struct ksmbd_work *work) static inline bool ksmbd_conn_need_negotiate(struct ksmbd_conn *conn)
{ {
return work->conn->status == KSMBD_SESS_NEED_NEGOTIATE; return READ_ONCE(conn->status) == KSMBD_SESS_NEED_NEGOTIATE;
} }
static inline bool ksmbd_conn_need_reconnect(struct ksmbd_work *work) static inline bool ksmbd_conn_need_reconnect(struct ksmbd_conn *conn)
{ {
return work->conn->status == KSMBD_SESS_NEED_RECONNECT; return READ_ONCE(conn->status) == KSMBD_SESS_NEED_RECONNECT;
} }
static inline bool ksmbd_conn_exiting(struct ksmbd_work *work) static inline bool ksmbd_conn_exiting(struct ksmbd_conn *conn)
{ {
return work->conn->status == KSMBD_SESS_EXITING; return READ_ONCE(conn->status) == KSMBD_SESS_EXITING;
} }
static inline void ksmbd_conn_set_good(struct ksmbd_work *work) static inline void ksmbd_conn_set_new(struct ksmbd_conn *conn)
{ {
work->conn->status = KSMBD_SESS_GOOD; WRITE_ONCE(conn->status, KSMBD_SESS_NEW);
} }
static inline void ksmbd_conn_set_need_negotiate(struct ksmbd_work *work) static inline void ksmbd_conn_set_good(struct ksmbd_conn *conn)
{ {
work->conn->status = KSMBD_SESS_NEED_NEGOTIATE; WRITE_ONCE(conn->status, KSMBD_SESS_GOOD);
} }
static inline void ksmbd_conn_set_need_reconnect(struct ksmbd_work *work) static inline void ksmbd_conn_set_need_negotiate(struct ksmbd_conn *conn)
{ {
work->conn->status = KSMBD_SESS_NEED_RECONNECT; WRITE_ONCE(conn->status, KSMBD_SESS_NEED_NEGOTIATE);
} }
static inline void ksmbd_conn_set_exiting(struct ksmbd_work *work) static inline void ksmbd_conn_set_need_reconnect(struct ksmbd_conn *conn)
{ {
work->conn->status = KSMBD_SESS_EXITING; WRITE_ONCE(conn->status, KSMBD_SESS_NEED_RECONNECT);
}
static inline void ksmbd_conn_set_exiting(struct ksmbd_conn *conn)
{
WRITE_ONCE(conn->status, KSMBD_SESS_EXITING);
} }
#endif /* __CONNECTION_H__ */ #endif /* __CONNECTION_H__ */
...@@ -315,6 +315,7 @@ static struct ksmbd_session *__session_create(int protocol) ...@@ -315,6 +315,7 @@ static struct ksmbd_session *__session_create(int protocol)
if (ksmbd_init_file_table(&sess->file_table)) if (ksmbd_init_file_table(&sess->file_table))
goto error; goto error;
sess->state = SMB2_SESSION_IN_PROGRESS;
set_session_flag(sess, protocol); set_session_flag(sess, protocol);
xa_init(&sess->tree_conns); xa_init(&sess->tree_conns);
xa_init(&sess->ksmbd_chann_list); xa_init(&sess->ksmbd_chann_list);
......
...@@ -93,7 +93,8 @@ static inline int check_conn_state(struct ksmbd_work *work) ...@@ -93,7 +93,8 @@ static inline int check_conn_state(struct ksmbd_work *work)
{ {
struct smb_hdr *rsp_hdr; struct smb_hdr *rsp_hdr;
if (ksmbd_conn_exiting(work) || ksmbd_conn_need_reconnect(work)) { if (ksmbd_conn_exiting(work->conn) ||
ksmbd_conn_need_reconnect(work->conn)) {
rsp_hdr = work->response_buf; rsp_hdr = work->response_buf;
rsp_hdr->Status.CifsError = STATUS_CONNECTION_DISCONNECTED; rsp_hdr->Status.CifsError = STATUS_CONNECTION_DISCONNECTED;
return 1; return 1;
......
...@@ -248,7 +248,7 @@ int init_smb2_neg_rsp(struct ksmbd_work *work) ...@@ -248,7 +248,7 @@ int init_smb2_neg_rsp(struct ksmbd_work *work)
rsp = smb2_get_msg(work->response_buf); rsp = smb2_get_msg(work->response_buf);
WARN_ON(ksmbd_conn_good(work)); WARN_ON(ksmbd_conn_good(conn));
rsp->StructureSize = cpu_to_le16(65); rsp->StructureSize = cpu_to_le16(65);
ksmbd_debug(SMB, "conn->dialect 0x%x\n", conn->dialect); ksmbd_debug(SMB, "conn->dialect 0x%x\n", conn->dialect);
...@@ -277,7 +277,7 @@ int init_smb2_neg_rsp(struct ksmbd_work *work) ...@@ -277,7 +277,7 @@ int init_smb2_neg_rsp(struct ksmbd_work *work)
rsp->SecurityMode |= SMB2_NEGOTIATE_SIGNING_REQUIRED_LE; rsp->SecurityMode |= SMB2_NEGOTIATE_SIGNING_REQUIRED_LE;
conn->use_spnego = true; conn->use_spnego = true;
ksmbd_conn_set_need_negotiate(work); ksmbd_conn_set_need_negotiate(conn);
return 0; return 0;
} }
...@@ -561,7 +561,7 @@ int smb2_check_user_session(struct ksmbd_work *work) ...@@ -561,7 +561,7 @@ int smb2_check_user_session(struct ksmbd_work *work)
cmd == SMB2_SESSION_SETUP_HE) cmd == SMB2_SESSION_SETUP_HE)
return 0; return 0;
if (!ksmbd_conn_good(work)) if (!ksmbd_conn_good(conn))
return -EINVAL; return -EINVAL;
sess_id = le64_to_cpu(req_hdr->SessionId); sess_id = le64_to_cpu(req_hdr->SessionId);
...@@ -594,7 +594,7 @@ static void destroy_previous_session(struct ksmbd_conn *conn, ...@@ -594,7 +594,7 @@ static void destroy_previous_session(struct ksmbd_conn *conn,
prev_sess->state = SMB2_SESSION_EXPIRED; prev_sess->state = SMB2_SESSION_EXPIRED;
xa_for_each(&prev_sess->ksmbd_chann_list, index, chann) xa_for_each(&prev_sess->ksmbd_chann_list, index, chann)
chann->conn->status = KSMBD_SESS_EXITING; ksmbd_conn_set_exiting(chann->conn);
} }
/** /**
...@@ -1051,7 +1051,7 @@ int smb2_handle_negotiate(struct ksmbd_work *work) ...@@ -1051,7 +1051,7 @@ int smb2_handle_negotiate(struct ksmbd_work *work)
ksmbd_debug(SMB, "Received negotiate request\n"); ksmbd_debug(SMB, "Received negotiate request\n");
conn->need_neg = false; conn->need_neg = false;
if (ksmbd_conn_good(work)) { if (ksmbd_conn_good(conn)) {
pr_err("conn->tcp_status is already in CifsGood State\n"); pr_err("conn->tcp_status is already in CifsGood State\n");
work->send_no_response = 1; work->send_no_response = 1;
return rc; return rc;
...@@ -1205,7 +1205,7 @@ int smb2_handle_negotiate(struct ksmbd_work *work) ...@@ -1205,7 +1205,7 @@ int smb2_handle_negotiate(struct ksmbd_work *work)
} }
conn->srv_sec_mode = le16_to_cpu(rsp->SecurityMode); conn->srv_sec_mode = le16_to_cpu(rsp->SecurityMode);
ksmbd_conn_set_need_negotiate(work); ksmbd_conn_set_need_negotiate(conn);
err_out: err_out:
if (rc < 0) if (rc < 0)
...@@ -1628,6 +1628,7 @@ int smb2_sess_setup(struct ksmbd_work *work) ...@@ -1628,6 +1628,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
rsp->SecurityBufferLength = 0; rsp->SecurityBufferLength = 0;
inc_rfc1001_len(work->response_buf, 9); inc_rfc1001_len(work->response_buf, 9);
ksmbd_conn_lock(conn);
if (!req->hdr.SessionId) { if (!req->hdr.SessionId) {
sess = ksmbd_smb2_session_create(); sess = ksmbd_smb2_session_create();
if (!sess) { if (!sess) {
...@@ -1675,6 +1676,12 @@ int smb2_sess_setup(struct ksmbd_work *work) ...@@ -1675,6 +1676,12 @@ int smb2_sess_setup(struct ksmbd_work *work)
goto out_err; goto out_err;
} }
if (ksmbd_conn_need_reconnect(conn)) {
rc = -EFAULT;
sess = NULL;
goto out_err;
}
if (ksmbd_session_lookup(conn, sess_id)) { if (ksmbd_session_lookup(conn, sess_id)) {
rc = -EACCES; rc = -EACCES;
goto out_err; goto out_err;
...@@ -1694,11 +1701,19 @@ int smb2_sess_setup(struct ksmbd_work *work) ...@@ -1694,11 +1701,19 @@ int smb2_sess_setup(struct ksmbd_work *work)
rc = -ENOENT; rc = -ENOENT;
goto out_err; goto out_err;
} }
if (sess->state == SMB2_SESSION_EXPIRED) {
rc = -EFAULT;
goto out_err;
} }
work->sess = sess;
if (sess->state == SMB2_SESSION_EXPIRED) if (ksmbd_conn_need_reconnect(conn)) {
sess->state = SMB2_SESSION_IN_PROGRESS; rc = -EFAULT;
sess = NULL;
goto out_err;
}
}
work->sess = sess;
negblob_off = le16_to_cpu(req->SecurityBufferOffset); negblob_off = le16_to_cpu(req->SecurityBufferOffset);
negblob_len = le16_to_cpu(req->SecurityBufferLength); negblob_len = le16_to_cpu(req->SecurityBufferLength);
...@@ -1729,8 +1744,10 @@ int smb2_sess_setup(struct ksmbd_work *work) ...@@ -1729,8 +1744,10 @@ int smb2_sess_setup(struct ksmbd_work *work)
goto out_err; goto out_err;
} }
ksmbd_conn_set_good(work); if (!ksmbd_conn_need_reconnect(conn)) {
ksmbd_conn_set_good(conn);
sess->state = SMB2_SESSION_VALID; sess->state = SMB2_SESSION_VALID;
}
kfree(sess->Preauth_HashValue); kfree(sess->Preauth_HashValue);
sess->Preauth_HashValue = NULL; sess->Preauth_HashValue = NULL;
} else if (conn->preferred_auth_mech == KSMBD_AUTH_NTLMSSP) { } else if (conn->preferred_auth_mech == KSMBD_AUTH_NTLMSSP) {
...@@ -1752,8 +1769,10 @@ int smb2_sess_setup(struct ksmbd_work *work) ...@@ -1752,8 +1769,10 @@ int smb2_sess_setup(struct ksmbd_work *work)
if (rc) if (rc)
goto out_err; goto out_err;
ksmbd_conn_set_good(work); if (!ksmbd_conn_need_reconnect(conn)) {
ksmbd_conn_set_good(conn);
sess->state = SMB2_SESSION_VALID; sess->state = SMB2_SESSION_VALID;
}
if (conn->binding) { if (conn->binding) {
struct preauth_session *preauth_sess; struct preauth_session *preauth_sess;
...@@ -1819,14 +1838,13 @@ int smb2_sess_setup(struct ksmbd_work *work) ...@@ -1819,14 +1838,13 @@ int smb2_sess_setup(struct ksmbd_work *work)
if (sess->user && sess->user->flags & KSMBD_USER_FLAG_DELAY_SESSION) if (sess->user && sess->user->flags & KSMBD_USER_FLAG_DELAY_SESSION)
try_delay = true; try_delay = true;
xa_erase(&conn->sessions, sess->id); sess->state = SMB2_SESSION_EXPIRED;
ksmbd_session_destroy(sess);
work->sess = NULL;
if (try_delay) if (try_delay)
ssleep(5); ssleep(5);
} }
} }
ksmbd_conn_unlock(conn);
return rc; return rc;
} }
...@@ -2050,21 +2068,24 @@ int smb2_session_logoff(struct ksmbd_work *work) ...@@ -2050,21 +2068,24 @@ int smb2_session_logoff(struct ksmbd_work *work)
{ {
struct ksmbd_conn *conn = work->conn; struct ksmbd_conn *conn = work->conn;
struct smb2_logoff_rsp *rsp = smb2_get_msg(work->response_buf); struct smb2_logoff_rsp *rsp = smb2_get_msg(work->response_buf);
struct ksmbd_session *sess = work->sess; struct ksmbd_session *sess;
struct smb2_logoff_req *req = smb2_get_msg(work->request_buf);
rsp->StructureSize = cpu_to_le16(4); rsp->StructureSize = cpu_to_le16(4);
inc_rfc1001_len(work->response_buf, 4); inc_rfc1001_len(work->response_buf, 4);
ksmbd_debug(SMB, "request\n"); ksmbd_debug(SMB, "request\n");
/* setting CifsExiting here may race with start_tcp_sess */ ksmbd_conn_set_need_reconnect(conn);
ksmbd_conn_set_need_reconnect(work);
ksmbd_close_session_fds(work); ksmbd_close_session_fds(work);
ksmbd_conn_wait_idle(conn); ksmbd_conn_wait_idle(conn);
/*
* Re-lookup session to validate if session is deleted
* while waiting request complete
*/
sess = ksmbd_session_lookup(conn, le64_to_cpu(req->hdr.SessionId));
if (ksmbd_tree_conn_session_logoff(sess)) { if (ksmbd_tree_conn_session_logoff(sess)) {
struct smb2_logoff_req *req = smb2_get_msg(work->request_buf);
ksmbd_debug(SMB, "Invalid tid %d\n", req->hdr.Id.SyncId.TreeId); ksmbd_debug(SMB, "Invalid tid %d\n", req->hdr.Id.SyncId.TreeId);
rsp->hdr.Status = STATUS_NETWORK_NAME_DELETED; rsp->hdr.Status = STATUS_NETWORK_NAME_DELETED;
smb2_set_err_rsp(work); smb2_set_err_rsp(work);
...@@ -2076,9 +2097,7 @@ int smb2_session_logoff(struct ksmbd_work *work) ...@@ -2076,9 +2097,7 @@ int smb2_session_logoff(struct ksmbd_work *work)
ksmbd_free_user(sess->user); ksmbd_free_user(sess->user);
sess->user = NULL; sess->user = NULL;
ksmbd_conn_set_need_negotiate(conn);
/* let start_tcp_sess free connection info now */
ksmbd_conn_set_need_negotiate(work);
return 0; return 0;
} }
......
...@@ -333,7 +333,7 @@ static int ksmbd_tcp_readv(struct tcp_transport *t, struct kvec *iov_orig, ...@@ -333,7 +333,7 @@ static int ksmbd_tcp_readv(struct tcp_transport *t, struct kvec *iov_orig,
if (length == -EINTR) { if (length == -EINTR) {
total_read = -ESHUTDOWN; total_read = -ESHUTDOWN;
break; break;
} else if (conn->status == KSMBD_SESS_NEED_RECONNECT) { } else if (ksmbd_conn_need_reconnect(conn)) {
total_read = -EAGAIN; total_read = -EAGAIN;
break; break;
} else if (length == -ERESTARTSYS || length == -EAGAIN) { } else if (length == -ERESTARTSYS || length == -EAGAIN) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment