ksmbd: fix racy issue from session setup and logoff

[ Upstream commit f5c779b7ddbda30866cf2a27c63e34158f858c73 ]

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: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
index b8f9d627f241dc6637d72ee516465430900decb2..3cb88853d69328d5eec05b441a402601dcb77729 100644 (file)
--- a/fs/ksmbd/connection.c
+++ b/fs/ksmbd/connection.c
@@ -56,7 +56,7 @@ struct ksmbd_conn *ksmbd_conn_alloc(void)
                return NULL;
 
        conn->need_neg = true;
-       conn->status = KSMBD_SESS_NEW;
+       ksmbd_conn_set_new(conn);
        conn->local_nls = load_nls("utf8");
        if (!conn->local_nls)
                conn->local_nls = load_nls_default();
@@ -149,12 +149,12 @@ int ksmbd_conn_try_dequeue_request(struct ksmbd_work *work)
        return ret;
 }
 
-static void ksmbd_conn_lock(struct ksmbd_conn *conn)
+void ksmbd_conn_lock(struct ksmbd_conn *conn)
 {
        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);
 }
@@ -245,7 +245,7 @@ bool ksmbd_conn_alive(struct ksmbd_conn *conn)
        if (!ksmbd_server_running())
                return false;
 
-       if (conn->status == KSMBD_SESS_EXITING)
+       if (ksmbd_conn_exiting(conn))
                return false;
 
        if (kthread_should_stop())
@@ -305,7 +305,7 @@ int ksmbd_conn_handler_loop(void *p)
                pdu_size = get_rfc1002_len(hdr_buf);
                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 =
                                SMB3_MAX_MSGSIZE + conn->vals->max_write_size;
                else
@@ -314,7 +314,7 @@ int ksmbd_conn_handler_loop(void *p)
                if (pdu_size > max_allowed_pdu_size) {
                        pr_err_ratelimited("PDU length(%u) excceed maximum allowed pdu size(%u) on connection(%d)\n",
                                        pdu_size, max_allowed_pdu_size,
-                                       conn->status);
+                                       READ_ONCE(conn->status));
                        break;
                }
 
@@ -418,7 +418,7 @@ static void stop_sessions(void)
                if (task)
                        ksmbd_debug(CONN, "Stop session handler %s/%d\n",
                                    task->comm, task_pid_nr(task));
-               conn->status = KSMBD_SESS_EXITING;
+               ksmbd_conn_set_exiting(conn);
                if (t->ops->shutdown) {
                        read_unlock(&conn_list_lock);
                        t->ops->shutdown(t);

diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h
index 0e3a848defaf3214ed752deed6c51854890a4194..98bb5f199fa24139d9ca80950a241118d19d4c6f 100644 (file)
--- a/fs/ksmbd/connection.h
+++ b/fs/ksmbd/connection.h
@@ -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_transport_init(void);
 void ksmbd_conn_transport_destroy(void);
+void ksmbd_conn_lock(struct ksmbd_conn *conn);
+void ksmbd_conn_unlock(struct ksmbd_conn *conn);
 
 /*
  * WARNING
@@ -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
  * 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__ */

diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c
index a2b128dedcfcf92a3b17144d59b09cd891b2d597..69b85a98e2c358b26cde630970a9fa18674249d1 100644 (file)
--- a/fs/ksmbd/mgmt/user_session.c
+++ b/fs/ksmbd/mgmt/user_session.c
@@ -324,6 +324,7 @@ static struct ksmbd_session *__session_create(int protocol)
        if (ksmbd_init_file_table(&sess->file_table))
                goto error;
 
+       sess->state = SMB2_SESSION_IN_PROGRESS;
        set_session_flag(sess, protocol);
        xa_init(&sess->tree_conns);
        xa_init(&sess->ksmbd_chann_list);

diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c
index 8c2bc513445c3e8b1aca93da155ba56475b4db3c..8a0ad399f24561abbeda6e44edba9fc69d6cd462 100644 (file)
--- a/fs/ksmbd/server.c
+++ b/fs/ksmbd/server.c
@@ -93,7 +93,8 @@ static inline int check_conn_state(struct ksmbd_work *work)
 {
        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->Status.CifsError = STATUS_CONNECTION_DISCONNECTED;
                return 1;

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index ac79d4c86067f37aa884d8644383245d8625cdb6..6c26934272ad5abf7830982c5bc7e6d3ebdb8659 100644 (file)
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -247,7 +247,7 @@ int init_smb2_neg_rsp(struct ksmbd_work *work)
 
        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);
        ksmbd_debug(SMB, "conn->dialect 0x%x\n", conn->dialect);
@@ -277,7 +277,7 @@ int init_smb2_neg_rsp(struct ksmbd_work *work)
                rsp->SecurityMode |= SMB2_NEGOTIATE_SIGNING_REQUIRED_LE;
        conn->use_spnego = true;
 
-       ksmbd_conn_set_need_negotiate(work);
+       ksmbd_conn_set_need_negotiate(conn);
        return 0;
 }
 
@@ -567,7 +567,7 @@ int smb2_check_user_session(struct ksmbd_work *work)
            cmd == SMB2_SESSION_SETUP_HE)
                return 0;
 
-       if (!ksmbd_conn_good(work))
+       if (!ksmbd_conn_good(conn))
                return -EINVAL;
 
        sess_id = le64_to_cpu(req_hdr->SessionId);
@@ -600,7 +600,7 @@ static void destroy_previous_session(struct ksmbd_conn *conn,
 
        prev_sess->state = SMB2_SESSION_EXPIRED;
        xa_for_each(&prev_sess->ksmbd_chann_list, index, chann)
-               chann->conn->status = KSMBD_SESS_EXITING;
+               ksmbd_conn_set_exiting(chann->conn);
 }
 
 /**
@@ -1067,7 +1067,7 @@ int smb2_handle_negotiate(struct ksmbd_work *work)
 
        ksmbd_debug(SMB, "Received negotiate request\n");
        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");
                work->send_no_response = 1;
                return rc;
@@ -1222,7 +1222,7 @@ int smb2_handle_negotiate(struct ksmbd_work *work)
        }
 
        conn->srv_sec_mode = le16_to_cpu(rsp->SecurityMode);
-       ksmbd_conn_set_need_negotiate(work);
+       ksmbd_conn_set_need_negotiate(conn);
 
 err_out:
        if (rc < 0)
@@ -1643,6 +1643,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
        rsp->SecurityBufferLength = 0;
        inc_rfc1001_len(work->response_buf, 9);
 
+       ksmbd_conn_lock(conn);
        if (!req->hdr.SessionId) {
                sess = ksmbd_smb2_session_create();
                if (!sess) {
@@ -1690,6 +1691,12 @@ int smb2_sess_setup(struct ksmbd_work *work)
                        goto out_err;
                }
 
+               if (ksmbd_conn_need_reconnect(conn)) {
+                       rc = -EFAULT;
+                       sess = NULL;
+                       goto out_err;
+               }
+
                if (ksmbd_session_lookup(conn, sess_id)) {
                        rc = -EACCES;
                        goto out_err;
@@ -1714,12 +1721,20 @@ int smb2_sess_setup(struct ksmbd_work *work)
                        rc = -ENOENT;
                        goto out_err;
                }
+
+               if (sess->state == SMB2_SESSION_EXPIRED) {
+                       rc = -EFAULT;
+                       goto out_err;
+               }
+
+               if (ksmbd_conn_need_reconnect(conn)) {
+                       rc = -EFAULT;
+                       sess = NULL;
+                       goto out_err;
+               }
        }
        work->sess = sess;
 
-       if (sess->state == SMB2_SESSION_EXPIRED)
-               sess->state = SMB2_SESSION_IN_PROGRESS;
-
        negblob_off = le16_to_cpu(req->SecurityBufferOffset);
        negblob_len = le16_to_cpu(req->SecurityBufferLength);
        if (negblob_off < offsetof(struct smb2_sess_setup_req, Buffer) ||
@@ -1749,8 +1764,10 @@ int smb2_sess_setup(struct ksmbd_work *work)
                                goto out_err;
                        }
 
-                       ksmbd_conn_set_good(work);
-                       sess->state = SMB2_SESSION_VALID;
+                       if (!ksmbd_conn_need_reconnect(conn)) {
+                               ksmbd_conn_set_good(conn);
+                               sess->state = SMB2_SESSION_VALID;
+                       }
                        kfree(sess->Preauth_HashValue);
                        sess->Preauth_HashValue = NULL;
                } else if (conn->preferred_auth_mech == KSMBD_AUTH_NTLMSSP) {
@@ -1772,8 +1789,10 @@ int smb2_sess_setup(struct ksmbd_work *work)
                                if (rc)
                                        goto out_err;
 
-                               ksmbd_conn_set_good(work);
-                               sess->state = SMB2_SESSION_VALID;
+                               if (!ksmbd_conn_need_reconnect(conn)) {
+                                       ksmbd_conn_set_good(conn);
+                                       sess->state = SMB2_SESSION_VALID;
+                               }
                                if (conn->binding) {
                                        struct preauth_session *preauth_sess;
 
@@ -1841,14 +1860,13 @@ int smb2_sess_setup(struct ksmbd_work *work)
                        if (sess->user && sess->user->flags & KSMBD_USER_FLAG_DELAY_SESSION)
                                try_delay = true;
 
-                       xa_erase(&conn->sessions, sess->id);
-                       ksmbd_session_destroy(sess);
-                       work->sess = NULL;
+                       sess->state = SMB2_SESSION_EXPIRED;
                        if (try_delay)
                                ssleep(5);
                }
        }
 
+       ksmbd_conn_unlock(conn);
        return rc;
 }
 
@@ -2073,21 +2091,24 @@ int smb2_session_logoff(struct ksmbd_work *work)
 {
        struct ksmbd_conn *conn = work->conn;
        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);
        inc_rfc1001_len(work->response_buf, 4);
 
        ksmbd_debug(SMB, "request\n");
 
-       /* setting CifsExiting here may race with start_tcp_sess */
-       ksmbd_conn_set_need_reconnect(work);
+       ksmbd_conn_set_need_reconnect(conn);
        ksmbd_close_session_fds(work);
        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)) {
-               struct smb2_logoff_req *req = smb2_get_msg(work->request_buf);
-
                ksmbd_debug(SMB, "Invalid tid %d\n", req->hdr.Id.SyncId.TreeId);
                rsp->hdr.Status = STATUS_NETWORK_NAME_DELETED;
                smb2_set_err_rsp(work);
@@ -2099,9 +2120,7 @@ int smb2_session_logoff(struct ksmbd_work *work)
 
        ksmbd_free_user(sess->user);
        sess->user = NULL;
-
-       /* let start_tcp_sess free connection info now */
-       ksmbd_conn_set_need_negotiate(work);
+       ksmbd_conn_set_need_negotiate(conn);
        return 0;
 }
 

diff --git a/fs/ksmbd/transport_tcp.c b/fs/ksmbd/transport_tcp.c
index 20e85e2701f26cf38daac453a41cc5eee2c14245..eff7a1d793f00382078f3132f818a2dd4fe62cda 100644 (file)
--- a/fs/ksmbd/transport_tcp.c
+++ b/fs/ksmbd/transport_tcp.c
@@ -333,7 +333,7 @@ static int ksmbd_tcp_readv(struct tcp_transport *t, struct kvec *iov_orig,
                if (length == -EINTR) {
                        total_read = -ESHUTDOWN;
                        break;
-               } else if (conn->status == KSMBD_SESS_NEED_RECONNECT) {
+               } else if (ksmbd_conn_need_reconnect(conn)) {
                        total_read = -EAGAIN;
                        break;
                } else if (length == -ERESTARTSYS || length == -EAGAIN) {