Attachment 17863 Details for Bug 15353 – Patch for v4-17-test

[patch] Patch for v4-17-test

bfixes-tmp417.txt (text/plain), 11.11 KB, created by Stefan Metzmacher on 2023-04-12 13:55:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2023-04-12 13:55:57 UTC

Size: 11.11 KB

patch

obsolete

>From 536e15914f48464a28067190006012682dfd2a33 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 5 Apr 2023 16:59:44 +0200
>Subject: [PATCH] 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 <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(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 125ef64f0705..acf09ca4ac57 100644
>--- a/source3/smbd/globals.h
>+++ b/source3/smbd/globals.h
>@@ -648,6 +648,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,
>@@ -656,6 +658,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 ed18f298f5b0..df26b9fa9d82 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 5affea6b3e42..71062e7bd84e 100644
>--- a/source3/smbd/smb2_service.c
>+++ b/source3/smbd/smb2_service.c
>@@ -646,21 +646,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 14229366efa9..5bd01c77e053 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 8707082edd63..cbb22852c92d 100644
>--- a/source3/smbd/smbXsrv_tcon.c
>+++ b/source3/smbd/smbXsrv_tcon.c
>@@ -738,6 +738,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;
>@@ -767,6 +770,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;
> 
>@@ -1100,14 +1111,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,
>@@ -1156,6 +1174,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)
> {
>@@ -1163,7 +1184,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,
>-- 
>2.34.1
>

Flags:

slow: review+

Actions: View

Attachments on bug 15353: 17862 | 17863