[SRU Jammy 3/3] ksmbd: fix racy issue from session setup and logoff
Cengiz Can
cengiz.can at canonical.com
Mon Nov 27 22:50:04 UTC 2023
From: Namjae Jeon <linkinjeon at kernel.org>
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 at vger.kernel.org
Reported-by: zdi-disclosures at trendmicro.com # ZDI-CAN-20481, ZDI-CAN-20590, ZDI-CAN-20596
Signed-off-by: Namjae Jeon <linkinjeon at kernel.org>
Signed-off-by: Steve French <stfrench at microsoft.com>
CVE-2023-32252
(backported from commit f5c779b7ddbda30866cf2a27c63e34158f858c73)
[cengizcan: we have commit 5005bcb42191 ("ksmbd: validate session id and tree id
in the compound request") but due to missing commit e2f34481b24d ("cifsd: add
server-side procedures for SMB3"), returned error values differ from upstream.
opt in to use -EINVAL in `smb2_check_user_session` just like upstream does]
[cengizcan: we don't have commit cb4517201b8a ("ksmbd: remove smb2_buf_length in
smb2_hdr") (it was introduced in 5.16 and needed a number of fixes anyway). so
instead of introducing lines that call `smb2_get_msg()`, keep direct `work->`
accesses]
Signed-off-by: Cengiz Can <cengiz.can at canonical.com>
---
fs/ksmbd/connection.c | 14 ++++----
fs/ksmbd/connection.h | 39 +++++++++++---------
fs/ksmbd/mgmt/user_session.c | 1 +
fs/ksmbd/server.c | 3 +-
fs/ksmbd/smb2pdu.c | 69 +++++++++++++++++++++++-------------
fs/ksmbd/transport_tcp.c | 2 +-
6 files changed, 78 insertions(+), 50 deletions(-)
diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
index 8e2500a5fa01..3c4b7a96919c 100644
--- 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();
@@ -142,12 +142,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);
}
@@ -240,7 +240,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())
@@ -300,7 +300,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
@@ -309,7 +309,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;
}
@@ -412,7 +412,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 fd243cdddb22..62ffc1c05c27 100644
--- a/fs/ksmbd/connection.h
+++ b/fs/ksmbd/connection.h
@@ -154,6 +154,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
@@ -161,43 +163,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 a2b128dedcfc..69b85a98e2c3 100644
--- 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 eb45d56b3577..656ef21db1fb 100644
--- 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 0cb263532cc4..ce29e54976e2 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -269,7 +269,7 @@ int init_smb2_neg_rsp(struct ksmbd_work *work)
rsp = 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);
@@ -299,7 +299,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;
}
@@ -588,8 +588,8 @@ int smb2_check_user_session(struct ksmbd_work *work)
cmd == SMB2_SESSION_SETUP_HE)
return 0;
- if (!ksmbd_conn_good(work))
- return -EIO;
+ if (!ksmbd_conn_good(conn))
+ return -EINVAL;
sess_id = le64_to_cpu(req_hdr->SessionId);
@@ -639,7 +639,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);
}
/**
@@ -1103,7 +1103,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;
@@ -1258,7 +1258,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)
@@ -1679,6 +1679,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
rsp->SecurityBufferLength = 0;
inc_rfc1001_len(rsp, 9);
+ ksmbd_conn_lock(conn);
if (!req->hdr.SessionId) {
sess = ksmbd_smb2_session_create();
if (!sess) {
@@ -1726,6 +1727,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;
@@ -1750,12 +1757,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) - 4) ||
@@ -1785,8 +1800,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) {
@@ -1807,8 +1824,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;
@@ -1876,14 +1895,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;
}
@@ -2102,21 +2120,24 @@ int smb2_session_logoff(struct ksmbd_work *work)
{
struct ksmbd_conn *conn = work->conn;
struct smb2_logoff_rsp *rsp = work->response_buf;
- struct ksmbd_session *sess = work->sess;
+ struct ksmbd_session *sess;
+ struct smb2_logoff_req *req = work->request_buf;
rsp->StructureSize = cpu_to_le16(4);
inc_rfc1001_len(rsp, 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 = 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);
@@ -2128,9 +2149,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 d1d7954368a5..42504e80f2af 100644
--- 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) {
--
2.40.1
More information about the kernel-team
mailing list