ksmbd: reject repeated SMB2 NEGOTIATE requests

Unauthenticated client can send multiple successful SMB2 NEGOTIATE
requests on one connection before SESSION_SETUP. While the connection is
in KSMBD_SESS_NEED_SETUP, smb2_handle_negotiate() accepts another
SMB3.1.1 NEGOTIATE and overwrites conn->preauth_info with a new allocation.
Only the final allocation is freed when the connection is released, leaking
one object for every additional successful request.

A repeated SMB2 NEGOTIATE after a dialect has been selected is a protocol
violation. MS-SMB2 section 3.3.5.4 requires the server to disconnect
without replying in this case. Set the connection exiting when rejecting
the request, in addition to suppressing the response.

Reject SMB2 NEGOTIATE unless the connection is new or is waiting for the
SMB2 NEGOTIATE that follows an SMB1 multi-protocol negotiate. Serialize
both SMB1 and SMB2 negotiation paths under conn->srv_mutex, since they
update connection-wide dialect and negotiation state.

Move the locking contract to ksmbd_smb_negotiate_common(), where the state
and dialect are selected, and add ksmbd_conn_new() for consistent state
access.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Reported-by: Runa Takemoto <takemotoruna223@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/server/connection.h b/fs/smb/server/connection.h
index ec75633b7da0fa2b3bb6206dd0800a3fe02a8712..2a194ee36fb40cf55050313b22d8837f0e2c39b3 100644 (file)
--- a/fs/smb/server/connection.h
+++ b/fs/smb/server/connection.h
@@ -200,6 +200,11 @@ void ksmbd_conn_r_count_dec(struct ksmbd_conn *conn);
  * This is a hack. We will move status to a proper place once we land
  * a multi-sessions support.
  */
+static inline bool ksmbd_conn_new(struct ksmbd_conn *conn)
+{
+       return READ_ONCE(conn->status) == KSMBD_SESS_NEW;
+}
+
 static inline bool ksmbd_conn_good(struct ksmbd_conn *conn)
 {
        return READ_ONCE(conn->status) == KSMBD_SESS_GOOD;

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index c1ba5e01aa7fa24e19f2e8c3ac7d16067a86e3df..404a4203f7da5fe502b4b748e13c3000e34f84aa 100644 (file)
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -1325,6 +1325,8 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
  * smb2_handle_negotiate() - handler for smb2 negotiate command
  * @work:      smb work containing smb request buffer
  *
+ * The caller holds conn->srv_mutex.
+ *
  * Return:      0
  */
 int smb2_handle_negotiate(struct ksmbd_work *work)
@@ -1338,13 +1340,6 @@ int smb2_handle_negotiate(struct ksmbd_work *work)
 
        ksmbd_debug(SMB, "Received negotiate request\n");
        conn->need_neg = false;
-       if (ksmbd_conn_good(conn)) {
-               pr_err("conn->tcp_status is already in CifsGood State\n");
-               work->send_no_response = 1;
-               return rc;
-       }
-
-       ksmbd_conn_lock(conn);
        smb2_buf_len = get_rfc1002_len(work->request_buf);
        smb2_neg_size = offsetof(struct smb2_negotiate_req, Dialects);
        if (smb2_neg_size > smb2_buf_len) {
@@ -1495,7 +1490,6 @@ int smb2_handle_negotiate(struct ksmbd_work *work)
        ksmbd_conn_set_need_setup(conn);
 
 err_out:
-       ksmbd_conn_unlock(conn);
        if (rc)
                rsp->hdr.Status = STATUS_INSUFFICIENT_RESOURCES;
 

diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
index 7de73223189ae91f0c58b0ad87202b1e3fddd439..080fbc9eb47047131de52f24b62747467a599297 100644 (file)
--- a/fs/smb/server/smb_common.c
+++ b/fs/smb/server/smb_common.c
@@ -608,23 +608,46 @@ int ksmbd_smb_negotiate_common(struct ksmbd_work *work, unsigned int command)
        struct ksmbd_conn *conn = work->conn;
        int ret;
 
-       conn->dialect =
-               ksmbd_negotiate_smb_dialect(work->request_buf);
-       ksmbd_debug(SMB, "conn->dialect 0x%x\n", conn->dialect);
-
        if (command == SMB2_NEGOTIATE_HE) {
+               /*
+                * An SMB2 NEGOTIATE is valid for a new connection, or after an
+                * SMB1 multi-protocol negotiate has selected SMB2. Do not allow
+                * a second SMB2 NEGOTIATE to replace connection-wide state
+                * while a session setup is pending. KSMBD_SESS_NEED_RECONNECT
+                * is a transient session state and does not restart transport
+                * negotiation.
+                */
+               ksmbd_conn_lock(conn);
+               if (!ksmbd_conn_new(conn) &&
+                   !ksmbd_conn_need_negotiate(conn)) {
+                       work->send_no_response = 1;
+                       ksmbd_conn_set_exiting(conn);
+                       ksmbd_conn_unlock(conn);
+                       return 0;
+               }
+
+               conn->dialect =
+                       ksmbd_negotiate_smb_dialect(work->request_buf);
+               ksmbd_debug(SMB, "conn->dialect 0x%x\n", conn->dialect);
                ret = smb2_handle_negotiate(work);
+               ksmbd_conn_unlock(conn);
                return ret;
        }
 
        if (command == SMB_COM_NEGOTIATE) {
+               ksmbd_conn_lock(conn);
+               conn->dialect =
+                       ksmbd_negotiate_smb_dialect(work->request_buf);
+               ksmbd_debug(SMB, "conn->dialect 0x%x\n", conn->dialect);
                if (__smb2_negotiate(conn)) {
                        init_smb3_11_server(conn);
-                       init_smb2_neg_rsp(work);
+                       ret = init_smb2_neg_rsp(work);
                        ksmbd_debug(SMB, "Upgrade to SMB2 negotiation\n");
-                       return 0;
+               } else {
+                       ret = smb_handle_negotiate(work);
                }
-               return smb_handle_negotiate(work);
+               ksmbd_conn_unlock(conn);
+               return ret;
        }
 
        pr_err("Unknown SMB negotiation command: %u\n", command);