smbXsrv_tcon: avoid storing temporary (invalid!) records.
authorStefan Metzmacher <metze@samba.org>
Wed, 5 Apr 2023 14:59:44 +0000 (16:59 +0200)
committerStefan Metzmacher <metze@samba.org>
Wed, 12 Apr 2023 12:48:35 +0000 (12:48 +0000)
We used to store smbXsrv_tcon_global.tdb records in two steps,
first we created a record in order to allocate the tcon id.
The temporary record had a NULL share_name, which translated
into 0 bytes for the string during ndr_push_smbXsrv_tcon_global0.

The problem is that ndr_pull_smbXsrv_tcon_global0 fails on
this with something like:

Invalid record in smbXsrv_tcon_global.tdb:key '2CA0ED4A' ndr_pull_struct_blob(length=85) - Buffer Size Error

The blob looks like this:

[0000] 00 00 00 00 01 00 00 00   00 00 00 00 00 00 02 00   ........  ........
[0010] 00 00 00 00 4A ED A0 2C   4A ED A0 2C 00 00 00 00   ....J.., J..,....
[0020] F8 4B 00 00 00 00 00 00   00 00 00 00 FF FF FF FF   .K......  ........
[0030] 4D 59 9B 9F 83 F4 35 20   36 D2 B0 82 62 68 D9 01   MY....5 6...bh..
[0040] 00 00 00 00 00 00 00 00   00 00 00 00 00 00 00 00   ........  ........
[0050] 00 00 00 00 00                                      .....

The reason for having a temporary entry was just based on
the fact, that it was easier to keep the logic in
make_connection_snum() untouched.

But we have all information available in order to store
the final record directly. We only need to do the
"max connections" check first.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15353

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
source3/smbd/globals.h
source3/smbd/smb1_service.c
source3/smbd/smb2_service.c
source3/smbd/smb2_tcon.c
source3/smbd/smbXsrv_tcon.c

index ff69d95ddfb9f2f0a08aa030b08522e0b5f7dc72..837d3c8acd2aae328ca1360996cd12410416bd59 100644 (file)
@@ -645,6 +645,8 @@ NTSTATUS smbXsrv_tcon_update(struct smbXsrv_tcon *tcon);
 NTSTATUS smbXsrv_tcon_disconnect(struct smbXsrv_tcon *tcon, uint64_t vuid);
 NTSTATUS smb1srv_tcon_table_init(struct smbXsrv_connection *conn);
 NTSTATUS smb1srv_tcon_create(struct smbXsrv_connection *conn,
+                            uint32_t session_global_id,
+                            const char *share_name,
                             NTTIME now,
                             struct smbXsrv_tcon **_tcon);
 NTSTATUS smb1srv_tcon_lookup(struct smbXsrv_connection *conn,
@@ -653,6 +655,9 @@ NTSTATUS smb1srv_tcon_lookup(struct smbXsrv_connection *conn,
 NTSTATUS smb1srv_tcon_disconnect_all(struct smbXsrv_client *client);
 NTSTATUS smb2srv_tcon_table_init(struct smbXsrv_session *session);
 NTSTATUS smb2srv_tcon_create(struct smbXsrv_session *session,
+                            uint32_t session_global_id,
+                            uint8_t encryption_flags,
+                            const char *share_name,
                             NTTIME now,
                             struct smbXsrv_tcon **_tcon);
 NTSTATUS smb2srv_tcon_lookup(struct smbXsrv_session *session,
index ed18f298f5b0aeacc3e8b14ab5550d90f28c595f..df26b9fa9d82bd51b1d93e33ec909899a095ab70 100644 (file)
@@ -48,17 +48,43 @@ static connection_struct *make_connection_smb1(struct smb_request *req,
 {
        const struct loadparm_substitution *lp_sub =
                loadparm_s3_global_substitution();
+       uint32_t session_global_id;
+       char *share_name = NULL;
        struct smbXsrv_tcon *tcon;
        NTSTATUS status;
        struct connection_struct *conn;
 
-       status = smb1srv_tcon_create(req->xconn, now, &tcon);
+       session_global_id = req->session->global->session_global_id;
+       share_name = lp_servicename(talloc_tos(), lp_sub, snum);
+       if (share_name == NULL) {
+               *pstatus = NT_STATUS_NO_MEMORY;
+               return NULL;
+       }
+
+       if ((lp_max_connections(snum) > 0)
+           && (count_current_connections(lp_const_servicename(snum), true) >=
+               lp_max_connections(snum))) {
+
+               DBG_WARNING("Max connections (%d) exceeded for [%s][%s]\n",
+                         lp_max_connections(snum),
+                         lp_const_servicename(snum), share_name);
+               TALLOC_FREE(share_name);
+               *pstatus = NT_STATUS_INSUFFICIENT_RESOURCES;
+               return NULL;
+       }
+
+       status = smb1srv_tcon_create(req->xconn,
+                                    session_global_id,
+                                    share_name,
+                                    now, &tcon);
        if (!NT_STATUS_IS_OK(status)) {
-               DEBUG(0,("make_connection_smb1: Couldn't find free tcon %s.\n",
-                        nt_errstr(status)));
+               DEBUG(0,("make_connection_smb1: Couldn't find free tcon for [%s] - %s\n",
+                        share_name, nt_errstr(status)));
+               TALLOC_FREE(share_name);
                *pstatus = status;
                return NULL;
        }
+       TALLOC_FREE(share_name);
 
        conn = conn_new(req->sconn);
        if (!conn) {
@@ -83,24 +109,10 @@ static connection_struct *make_connection_smb1(struct smb_request *req,
                return NULL;
        }
 
-       tcon->global->share_name = lp_servicename(tcon->global, lp_sub, SNUM(conn));
-       if (tcon->global->share_name == NULL) {
-               conn_free(conn);
-               TALLOC_FREE(tcon);
-               *pstatus = NT_STATUS_NO_MEMORY;
-               return NULL;
-       }
-       tcon->global->session_global_id =
-               req->session->global->session_global_id;
-
        tcon->compat = talloc_move(tcon, &conn);
        tcon->status = NT_STATUS_OK;
 
-       *pstatus = smbXsrv_tcon_update(tcon);
-       if (!NT_STATUS_IS_OK(*pstatus)) {
-               TALLOC_FREE(tcon);
-               return NULL;
-       }
+       *pstatus = NT_STATUS_OK;
 
        return tcon->compat;
 }
index 6670b8a5a13a55cc013e5f9152b50f4bba5d83a4..53ed90f038d04973672b3777e48bffffc1cd7ecb 100644 (file)
@@ -620,21 +620,6 @@ NTSTATUS make_connection_snum(struct smbXsrv_connection *xconn,
         * in the logs. */
        widelinks_warning(snum);
 
-       /*
-        * Enforce the max connections parameter.
-        */
-
-       if ((lp_max_connections(snum) > 0)
-           && (count_current_connections(lp_const_servicename(SNUM(conn)), true) >=
-               lp_max_connections(snum))) {
-
-               DBG_WARNING("Max connections (%d) exceeded for %s\n",
-                         lp_max_connections(snum),
-                         lp_const_servicename(snum));
-               status = NT_STATUS_INSUFFICIENT_RESOURCES;
-               goto err_root_exit;
-       }
-
        /* Invoke VFS make connection hook - this must be the first
           filesystem operation that we do. */
 
index 14229366efa9ca3c309febabf9eb0751f58a56da..5bd01c77e053fb62c569ae23edbcb9e7b154963f 100644 (file)
@@ -217,6 +217,9 @@ static NTSTATUS smbd_smb2_tree_connect(struct smbd_smb2_request *req,
        bool encryption_required = req->session->global->encryption_flags & SMBXSRV_ENCRYPTION_REQUIRED;
        bool guest_session = false;
        bool require_signed_tcon = false;
+       uint32_t session_global_id;
+       char *share_name = NULL;
+       uint8_t encryption_flags = 0;
 
        *disconnect = false;
 
@@ -328,17 +331,39 @@ static NTSTATUS smbd_smb2_tree_connect(struct smbd_smb2_request *req,
                }
        }
 
-       /* create a new tcon as child of the session */
-       status = smb2srv_tcon_create(req->session, now, &tcon);
-       if (!NT_STATUS_IS_OK(status)) {
-               return status;
-       }
-
        if (encryption_desired) {
-               tcon->global->encryption_flags |= SMBXSRV_ENCRYPTION_DESIRED;
+               encryption_flags |= SMBXSRV_ENCRYPTION_DESIRED;
        }
        if (encryption_required) {
-               tcon->global->encryption_flags |= SMBXSRV_ENCRYPTION_REQUIRED;
+               encryption_flags |= SMBXSRV_ENCRYPTION_REQUIRED;
+       }
+
+       session_global_id = req->session->global->session_global_id;
+       share_name = lp_servicename(talloc_tos(), lp_sub, snum);
+       if (share_name == NULL) {
+               return NT_STATUS_NO_MEMORY;
+       }
+
+       if ((lp_max_connections(snum) > 0)
+           && (count_current_connections(lp_const_servicename(snum), true) >=
+               lp_max_connections(snum))) {
+
+               DBG_WARNING("Max connections (%d) exceeded for [%s][%s]\n",
+                         lp_max_connections(snum),
+                         lp_const_servicename(snum), share_name);
+               TALLOC_FREE(share_name);
+               return NT_STATUS_INSUFFICIENT_RESOURCES;
+       }
+
+       /* create a new tcon as child of the session */
+       status = smb2srv_tcon_create(req->session,
+                                    session_global_id,
+                                    encryption_flags,
+                                    share_name,
+                                    now, &tcon);
+       TALLOC_FREE(share_name);
+       if (!NT_STATUS_IS_OK(status)) {
+               return status;
        }
 
        compat_conn = make_connection_smb2(req,
@@ -350,27 +375,10 @@ static NTSTATUS smbd_smb2_tree_connect(struct smbd_smb2_request *req,
                return status;
        }
 
-       tcon->global->share_name = lp_servicename(tcon->global,
-                                                 lp_sub,
-                                                 SNUM(compat_conn));
-       if (tcon->global->share_name == NULL) {
-               conn_free(compat_conn);
-               TALLOC_FREE(tcon);
-               return NT_STATUS_NO_MEMORY;
-       }
-       tcon->global->session_global_id =
-               req->session->global->session_global_id;
-
        tcon->compat = talloc_move(tcon, &compat_conn);
 
        tcon->status = NT_STATUS_OK;
 
-       status = smbXsrv_tcon_update(tcon);
-       if (!NT_STATUS_IS_OK(status)) {
-               TALLOC_FREE(tcon);
-               return status;
-       }
-
        if (IS_PRINT(tcon->compat)) {
                *out_share_type = SMB2_SHARE_TYPE_PRINT;
        } else if (IS_IPC(tcon->compat)) {
index 7d98b987a63810d7a49010d331de04c940098096..27e6e1286b91ac932d1896ae3d746ac44305917a 100644 (file)
@@ -737,6 +737,9 @@ static NTSTATUS smbXsrv_tcon_create(struct smbXsrv_tcon_table *table,
                                    enum protocol_types protocol,
                                    struct server_id server_id,
                                    NTTIME now,
+                                   uint32_t session_global_id,
+                                   uint8_t encryption_flags,
+                                   const char *share_name,
                                    struct smbXsrv_tcon **_tcon)
 {
        struct db_record *local_rec = NULL;
@@ -766,6 +769,14 @@ static NTSTATUS smbXsrv_tcon_create(struct smbXsrv_tcon_table *table,
        }
        tcon->global = global;
 
+       global->session_global_id = session_global_id;
+       global->encryption_flags = encryption_flags;
+       global->share_name = talloc_strdup(global, share_name);
+       if (global->share_name == NULL) {
+               TALLOC_FREE(tcon);
+               return NT_STATUS_NO_MEMORY;
+       }
+
        if (protocol >= PROTOCOL_SMB2_02) {
                uint64_t id = global->tcon_global_id;
 
@@ -1097,14 +1108,21 @@ NTSTATUS smb1srv_tcon_table_init(struct smbXsrv_connection *conn)
 }
 
 NTSTATUS smb1srv_tcon_create(struct smbXsrv_connection *conn,
+                            uint32_t session_global_id,
+                            const char *share_name,
                             NTTIME now,
                             struct smbXsrv_tcon **_tcon)
 {
        struct server_id id = messaging_server_id(conn->client->msg_ctx);
+       const uint8_t encryption_flags = 0;
 
        return smbXsrv_tcon_create(conn->client->tcon_table,
                                   conn->protocol,
-                                  id, now, _tcon);
+                                  id, now,
+                                  session_global_id,
+                                  encryption_flags,
+                                  share_name,
+                                  _tcon);
 }
 
 NTSTATUS smb1srv_tcon_lookup(struct smbXsrv_connection *conn,
@@ -1153,6 +1171,9 @@ NTSTATUS smb2srv_tcon_table_init(struct smbXsrv_session *session)
 }
 
 NTSTATUS smb2srv_tcon_create(struct smbXsrv_session *session,
+                            uint32_t session_global_id,
+                            uint8_t encryption_flags,
+                            const char *share_name,
                             NTTIME now,
                             struct smbXsrv_tcon **_tcon)
 {
@@ -1160,7 +1181,11 @@ NTSTATUS smb2srv_tcon_create(struct smbXsrv_session *session,
 
        return smbXsrv_tcon_create(session->tcon_table,
                                   PROTOCOL_SMB2_02,
-                                  id, now, _tcon);
+                                  id, now,
+                                  session_global_id,
+                                  encryption_flags,
+                                  share_name,
+                                  _tcon);
 }
 
 NTSTATUS smb2srv_tcon_lookup(struct smbXsrv_session *session,