From 50d90531fae36f54c2d6a5e6cf1aba133473a29c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 5 Apr 2023 16:59:44 +0200 Subject: smbXsrv_tcon: avoid storing temporary (invalid!) records. 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 Reviewed-by: Ralph Boehme (cherry picked from commit e0e58ed0e2429f01265d544b444bf0e4075549e2) --- source3/smbd/globals.h | 5 ++++ source3/smbd/smb1_service.c | 48 +++++++++++++++++++++++-------------- source3/smbd/smb2_service.c | 15 ------------ source3/smbd/smb2_tcon.c | 58 ++++++++++++++++++++++++++------------------- source3/smbd/smbXsrv_tcon.c | 29 +++++++++++++++++++++-- 5 files changed, 95 insertions(+), 60 deletions(-) diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index 5fb9f2e647c..9d588066548 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -644,6 +644,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, @@ -652,6 +654,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, diff --git a/source3/smbd/smb1_service.c b/source3/smbd/smb1_service.c index ed18f298f5b..df26b9fa9d8 100644 --- a/source3/smbd/smb1_service.c +++ b/source3/smbd/smb1_service.c @@ -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; } diff --git a/source3/smbd/smb2_service.c b/source3/smbd/smb2_service.c index 6670b8a5a13..53ed90f038d 100644 --- a/source3/smbd/smb2_service.c +++ b/source3/smbd/smb2_service.c @@ -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. */ diff --git a/source3/smbd/smb2_tcon.c b/source3/smbd/smb2_tcon.c index 14229366efa..5bd01c77e05 100644 --- a/source3/smbd/smb2_tcon.c +++ b/source3/smbd/smb2_tcon.c @@ -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)) { diff --git a/source3/smbd/smbXsrv_tcon.c b/source3/smbd/smbXsrv_tcon.c index 7d98b987a63..27e6e1286b9 100644 --- a/source3/smbd/smbXsrv_tcon.c +++ b/source3/smbd/smbXsrv_tcon.c @@ -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, -- cgit v1.2.1