[SCM] Samba Shared Repository - branch master updated

Volker Lendecke vlendec at samba.org
Tue Jan 24 09:16:01 UTC 2023


The branch, master has been updated
       via  f7b50bc059d smbd: Use smbXsrv_open_global_parse_record() in .._verify_record()
       via  132b83d0659 smbd: Simplify smbXsrv_open_global_parse_record()
       via  2f6776741dc smbd: Move smbXsrv_open_global_parse_record() up in smbXsrv_open.c
       via  3c779de8cf9 smbd: Simplify smbXsrv_open_global_verify_record()
       via  f1a66267bcf smbd: Save a few lines in smb2srv_open_lookup_replay_cache()
       via  35a32171b50 smbd: Fix a typo
      from  253891032ee python: Don't use deprecated escape sequences

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit f7b50bc059d1b5c7e40cdc4e88ef5ee16f7db670
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Jan 19 12:29:20 2023 +0100

    smbd: Use smbXsrv_open_global_parse_record() in .._verify_record()
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Tue Jan 24 09:15:26 UTC 2023 on atb-devel-224

commit 132b83d0659ddc25a96327edc1c7dd23b17a56fd
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Jan 19 12:25:21 2023 +0100

    smbd: Simplify smbXsrv_open_global_parse_record()
    
    It does not need a db_record.
    
    Signed-off-by: Volker Lendecke <vl at samba.org>

commit 2f6776741dc6469d78b94da22d75f26cccca5fc9
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Jan 19 12:22:33 2023 +0100

    smbd: Move smbXsrv_open_global_parse_record() up in smbXsrv_open.c
    
    Avoid a prototype in the next patches
    
    Signed-off-by: Volker Lendecke <vl at samba.org>

commit 3c779de8cf99d0936956a12484fd726d5be46c7e
Author: Volker Lendecke <vl at samba.org>
Date:   Fri Jan 6 16:25:03 2023 +0100

    smbd: Simplify smbXsrv_open_global_verify_record()
    
    Don't depend on the record to be passed in, return NTSTATUS. The two
    flags were a bit confusing to me, now NT_STATUS_OK means "found a
    valid record with a live process", and NT_STATUS_FATAL_APP_EXIT means
    we found a stale record from a crashed smbd
    
    Signed-off-by: Volker Lendecke <vl at samba.org>

commit f1a66267bcfcd48f3c7ca2ada3f62d40209163e3
Author: Volker Lendecke <vl at samba.org>
Date:   Wed Jan 11 11:44:29 2023 +0100

    smbd: Save a few lines in smb2srv_open_lookup_replay_cache()
    
    Directly initialize variables, don't leave dangling pointers in TDB_DATA
    
    Signed-off-by: Volker Lendecke <vl at samba.org>

commit 35a32171b5067d5b80acffc99f8d43cdc7f5f9a7
Author: Volker Lendecke <vl at samba.org>
Date:   Wed Jan 11 08:18:35 2023 +0100

    smbd: Fix a typo
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 source3/smbd/smbXsrv_open.c | 312 ++++++++++++++++++++------------------------
 1 file changed, 141 insertions(+), 171 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/smbXsrv_open.c b/source3/smbd/smbXsrv_open.c
index 6aa44ec4fcc..585d1ec0838 100644
--- a/source3/smbd/smbXsrv_open.c
+++ b/source3/smbd/smbXsrv_open.c
@@ -225,11 +225,11 @@ static NTSTATUS smbXsrv_open_local_lookup(struct smbXsrv_open_table *table,
 	return NT_STATUS_OK;
 }
 
-static void smbXsrv_open_global_verify_record(struct db_record *db_rec,
-					bool *is_free,
-					bool *was_free,
-					TALLOC_CTX *mem_ctx,
-					struct smbXsrv_open_global0 **_g);
+static NTSTATUS smbXsrv_open_global_verify_record(
+	TDB_DATA key,
+	TDB_DATA val,
+	TALLOC_CTX *mem_ctx,
+	struct smbXsrv_open_global0 **_global0);
 
 static NTSTATUS smbXsrv_open_global_allocate(
 	struct db_context *db, struct smbXsrv_open_global0 *global)
@@ -245,9 +245,11 @@ static NTSTATUS smbXsrv_open_global_allocate(
 	 * ID for SRVSVC.
 	 */
 	for (i = 0; i < UINT32_MAX; i++) {
-		bool is_free = false;
-		bool was_free = false;
+		struct smbXsrv_open_global_key_buf key_buf;
+		struct smbXsrv_open_global0 *tmp_global0 = NULL;
+		TDB_DATA key, val;
 		uint32_t id;
+		NTSTATUS status;
 
 		if (i >= min_tries && last_free != 0) {
 			id = last_free;
@@ -261,141 +263,154 @@ static NTSTATUS smbXsrv_open_global_allocate(
 			id--;
 		}
 
-		global->db_rec = smbXsrv_open_global_fetch_locked(
-			db, id, global);
+		key = smbXsrv_open_global_id_to_key(id, &key_buf);
+
+		global->db_rec = dbwrap_fetch_locked(db, global, key);
 		if (global->db_rec == NULL) {
 			return NT_STATUS_INSUFFICIENT_RESOURCES;
 		}
+		val = dbwrap_record_get_value(global->db_rec);
 
-		smbXsrv_open_global_verify_record(global->db_rec,
-						  &is_free,
-						  &was_free,
-						  NULL, NULL);
+		status = smbXsrv_open_global_verify_record(
+			key, val, talloc_tos(), &tmp_global0);
 
-		if (!is_free) {
-			TALLOC_FREE(global->db_rec);
-			continue;
+		if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
+			/*
+			 * Found an empty slot
+			 */
+			global->open_global_id = id;
+			return NT_STATUS_OK;
 		}
 
-		if (!was_free && i < min_tries) {
+		TALLOC_FREE(tmp_global0);
+
+		if (NT_STATUS_EQUAL(status, NT_STATUS_FATAL_APP_EXIT)) {
 			/*
-			 * The session_id is free now,
-			 * but was not free before.
-			 *
-			 * This happens if a smbd crashed
-			 * and did not cleanup the record.
-			 *
-			 * If this is one of our first tries,
-			 * then we try to find a real free one.
+			 * smbd crashed
 			 */
-			if (last_free == 0) {
+			status = dbwrap_record_delete(global->db_rec);
+			if (!NT_STATUS_IS_OK(status)) {
+				DBG_WARNING("dbwrap_record_delete() failed "
+					    "for record %"PRIu32": %s\n",
+					    id,
+					    nt_errstr(status));
+				return NT_STATUS_INTERNAL_DB_CORRUPTION;
+			}
+
+			if ((i < min_tries) && (last_free == 0)) {
+				/*
+				 * Remember "id" as free but also try
+				 * others to not recycle ids too
+				 * quickly.
+				 */
 				last_free = id;
 			}
-			TALLOC_FREE(global->db_rec);
-			continue;
 		}
 
-		global->open_global_id = id;
+		if (!NT_STATUS_IS_OK(status)) {
+			DBG_WARNING("smbXsrv_open_global_verify_record() "
+				    "failed for %s: %s, ignoring\n",
+				    tdb_data_dbg(key),
+				    nt_errstr(status));
+		}
 
-		return NT_STATUS_OK;
+		TALLOC_FREE(global->db_rec);
 	}
 
 	/* should not be reached */
 	return NT_STATUS_INTERNAL_ERROR;
 }
 
-static void smbXsrv_open_global_verify_record(struct db_record *db_rec,
-					bool *is_free,
-					bool *was_free,
-					TALLOC_CTX *mem_ctx,
-					struct smbXsrv_open_global0 **_g)
+static NTSTATUS smbXsrv_open_global_parse_record(
+	TALLOC_CTX *mem_ctx,
+	TDB_DATA key,
+	TDB_DATA val,
+	struct smbXsrv_open_global0 **global)
 {
-	TDB_DATA key;
-	TDB_DATA val;
-	DATA_BLOB blob;
+	DATA_BLOB blob = data_blob_const(val.dptr, val.dsize);
 	struct smbXsrv_open_globalB global_blob;
 	enum ndr_err_code ndr_err;
-	struct smbXsrv_open_global0 *global = NULL;
-	bool exists;
+	NTSTATUS status;
 	TALLOC_CTX *frame = talloc_stackframe();
 
-	*is_free = false;
-
-	if (was_free) {
-		*was_free = false;
-	}
-	if (_g) {
-		*_g = NULL;
-	}
-
-	key = dbwrap_record_get_key(db_rec);
-
-	val = dbwrap_record_get_value(db_rec);
-	if (val.dsize == 0) {
-		DEBUG(10, ("%s: empty value\n", __func__));
-		TALLOC_FREE(frame);
-		*is_free = true;
-		if (was_free) {
-			*was_free = true;
-		}
-		return;
-	}
-
-	blob = data_blob_const(val.dptr, val.dsize);
-
 	ndr_err = ndr_pull_struct_blob(&blob, frame, &global_blob,
 			(ndr_pull_flags_fn_t)ndr_pull_smbXsrv_open_globalB);
 	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-		NTSTATUS status = ndr_map_error2ntstatus(ndr_err);
-		DEBUG(1,("smbXsrv_open_global_verify_record: "
+		DEBUG(1,("Invalid record in smbXsrv_open_global.tdb:"
 			 "key '%s' ndr_pull_struct_blob - %s\n",
 			 tdb_data_dbg(key),
-			 nt_errstr(status)));
-		TALLOC_FREE(frame);
-		return;
+			 ndr_errstr(ndr_err)));
+		status = ndr_map_error2ntstatus(ndr_err);
+		goto done;
 	}
 
-	DEBUG(10,("smbXsrv_open_global_verify_record\n"));
+	DBG_DEBUG("\n");
 	if (CHECK_DEBUGLVL(10)) {
 		NDR_PRINT_DEBUG(smbXsrv_open_globalB, &global_blob);
 	}
 
 	if (global_blob.version != SMBXSRV_VERSION_0) {
-		DEBUG(0,("smbXsrv_open_global_verify_record: "
-			 "key '%s' use unsupported version %u\n",
+		status = NT_STATUS_INTERNAL_DB_CORRUPTION;
+		DEBUG(1,("Invalid record in smbXsrv_open_global.tdb:"
+			 "key '%s' unsupported version - %d - %s\n",
 			 tdb_data_dbg(key),
-			 global_blob.version));
-		NDR_PRINT_DEBUG(smbXsrv_open_globalB, &global_blob);
-		TALLOC_FREE(frame);
-		return;
+			 (int)global_blob.version,
+			 nt_errstr(status)));
+		goto done;
 	}
 
-	global = global_blob.info.info0;
+	if (global_blob.info.info0 == NULL) {
+		status = NT_STATUS_INTERNAL_DB_CORRUPTION;
+		DEBUG(1,("Invalid record in smbXsrv_tcon_global.tdb:"
+			 "key '%s' info0 NULL pointer - %s\n",
+			 tdb_data_dbg(key),
+			 nt_errstr(status)));
+		goto done;
+	}
 
-	if (server_id_is_disconnected(&global->server_id)) {
-		exists = true;
-	} else {
-		exists = serverid_exists(&global->server_id);
+	*global = talloc_move(mem_ctx, &global_blob.info.info0);
+	status = NT_STATUS_OK;
+done:
+	talloc_free(frame);
+	return status;
+}
+
+static NTSTATUS smbXsrv_open_global_verify_record(
+	TDB_DATA key,
+	TDB_DATA val,
+	TALLOC_CTX *mem_ctx,
+	struct smbXsrv_open_global0 **_global0)
+{
+	struct smbXsrv_open_global0 *global0 = NULL;
+	struct server_id_buf buf;
+	NTSTATUS status;
+
+	if (val.dsize == 0) {
+		return NT_STATUS_NOT_FOUND;
 	}
-	if (!exists) {
-		struct server_id_buf idbuf;
-		DEBUG(2,("smbXsrv_open_global_verify_record: "
-			 "key '%s' server_id %s does not exist.\n",
-			 tdb_data_dbg(key),
-			 server_id_str_buf(global->server_id, &idbuf)));
-		if (CHECK_DEBUGLVL(2)) {
-			NDR_PRINT_DEBUG(smbXsrv_open_globalB, &global_blob);
-		}
-		TALLOC_FREE(frame);
-		dbwrap_record_delete(db_rec);
-		*is_free = true;
-		return;
+
+	status = smbXsrv_open_global_parse_record(mem_ctx, key, val, &global0);
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_WARNING("smbXsrv_open_global_parse_record for %s failed: "
+			    "%s\n",
+			    tdb_data_dbg(key),
+			    nt_errstr(status));
+		return status;
 	}
+	*_global0 = global0;
 
-	if (_g) {
-		*_g = talloc_move(mem_ctx, &global);
+	if (server_id_is_disconnected(&global0->server_id)) {
+		return NT_STATUS_OK;
 	}
-	TALLOC_FREE(frame);
+	if (serverid_exists(&global0->server_id)) {
+		return NT_STATUS_OK;
+	}
+
+	DBG_WARNING("smbd %s did not clean up record %s\n",
+		    server_id_str_buf(global0->server_id, &buf),
+		    tdb_data_dbg(key));
+
+	return NT_STATUS_FATAL_APP_EXIT;
 }
 
 static NTSTATUS smbXsrv_open_global_store(struct smbXsrv_open_global0 *global)
@@ -462,8 +477,11 @@ static NTSTATUS smbXsrv_open_global_lookup(struct smbXsrv_open_table *table,
 					   TALLOC_CTX *mem_ctx,
 					   struct smbXsrv_open_global0 **_global)
 {
+	struct smbXsrv_open_global_key_buf key_buf;
+	TDB_DATA key = smbXsrv_open_global_id_to_key(open_global_id, &key_buf);
+	TDB_DATA val;
 	struct db_record *global_rec = NULL;
-	bool is_free = false;
+	NTSTATUS status;
 
 	*_global = NULL;
 
@@ -471,27 +489,25 @@ static NTSTATUS smbXsrv_open_global_lookup(struct smbXsrv_open_table *table,
 		return NT_STATUS_INTERNAL_ERROR;
 	}
 
-	global_rec = smbXsrv_open_global_fetch_locked(table->global.db_ctx,
-						      open_global_id,
-						      mem_ctx);
+	global_rec = dbwrap_fetch_locked(table->global.db_ctx, mem_ctx, key);
 	if (global_rec == NULL) {
 		return NT_STATUS_INTERNAL_DB_ERROR;
 	}
+	val = dbwrap_record_get_value(global_rec);
 
-	smbXsrv_open_global_verify_record(global_rec,
-					  &is_free,
-					  NULL,
-					  mem_ctx,
-					  _global);
-	if (is_free) {
-		DEBUG(10, ("%s: is_free=true\n", __func__));
-		talloc_free(global_rec);
-		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+	status = smbXsrv_open_global_verify_record(key, val, mem_ctx, _global);
+	if (NT_STATUS_IS_OK(status)) {
+		(*_global)->db_rec = talloc_move(*_global, &global_rec);
+		return NT_STATUS_OK;
 	}
 
-	(*_global)->db_rec = talloc_move(*_global, &global_rec);
+	TALLOC_FREE(global_rec);
 
-	return NT_STATUS_OK;
+	if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
+		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
+	}
+
+	return status;
 }
 
 static int smbXsrv_open_destructor(struct smbXsrv_open *op)
@@ -1034,10 +1050,10 @@ NTSTATUS smb2srv_open_lookup_replay_cache(struct smbXsrv_connection *conn,
 	NTSTATUS status;
 	struct smbXsrv_open_table *table = conn->client->open_table;
 	struct db_context *db = table->local.replay_cache_db_ctx;
-	struct GUID_txt_buf _create_guid_buf;
 	struct GUID_txt_buf tmp_guid_buf;
-	const char *create_guid_str = NULL;
-	TDB_DATA create_guid_key;
+	struct GUID_txt_buf _create_guid_buf;
+	const char *create_guid_str = GUID_buf_string(&create_guid, &_create_guid_buf);
+	TDB_DATA create_guid_key = string_term_tdb_data(create_guid_str);
 	struct db_record *db_rec = NULL;
 	struct smbXsrv_open *op = NULL;
 	struct smbXsrv_open_replay_cache rc = {
@@ -1051,9 +1067,6 @@ NTSTATUS smb2srv_open_lookup_replay_cache(struct smbXsrv_connection *conn,
 
 	*_open = NULL;
 
-	create_guid_str = GUID_buf_string(&create_guid, &_create_guid_buf);
-	create_guid_key = string_term_tdb_data(create_guid_str);
-
 	db_rec = dbwrap_fetch_locked(db, frame, create_guid_key);
 	if (db_rec == NULL) {
 		TALLOC_FREE(frame);
@@ -1218,7 +1231,7 @@ NTSTATUS smb2srv_open_recreate(struct smbXsrv_connection *conn,
 	/*
 	 * If the provided create_guid is NULL, this means that
 	 * the reconnect request was a v1 request. In that case
-	 * we should skipt the create GUID verification, since
+	 * we should skip the create GUID verification, since
 	 * it is valid to v1-reconnect a v2-opened handle.
 	 */
 	if ((create_guid != NULL) &&
@@ -1286,55 +1299,6 @@ NTSTATUS smb2srv_open_recreate(struct smbXsrv_connection *conn,
 }
 
 
-static NTSTATUS smbXsrv_open_global_parse_record(TALLOC_CTX *mem_ctx,
-						 struct db_record *rec,
-						 struct smbXsrv_open_global0 **global)
-{
-	TDB_DATA key = dbwrap_record_get_key(rec);
-	TDB_DATA val = dbwrap_record_get_value(rec);
-	DATA_BLOB blob = data_blob_const(val.dptr, val.dsize);
-	struct smbXsrv_open_globalB global_blob;
-	enum ndr_err_code ndr_err;
-	NTSTATUS status;
-	TALLOC_CTX *frame = talloc_stackframe();
-
-	ndr_err = ndr_pull_struct_blob(&blob, frame, &global_blob,
-			(ndr_pull_flags_fn_t)ndr_pull_smbXsrv_open_globalB);
-	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-		DEBUG(1,("Invalid record in smbXsrv_open_global.tdb:"
-			 "key '%s' ndr_pull_struct_blob - %s\n",
-			 tdb_data_dbg(key),
-			 ndr_errstr(ndr_err)));
-		status = ndr_map_error2ntstatus(ndr_err);
-		goto done;
-	}
-
-	if (global_blob.version != SMBXSRV_VERSION_0) {
-		status = NT_STATUS_INTERNAL_DB_CORRUPTION;
-		DEBUG(1,("Invalid record in smbXsrv_open_global.tdb:"
-			 "key '%s' unsupported version - %d - %s\n",
-			 tdb_data_dbg(key),
-			 (int)global_blob.version,
-			 nt_errstr(status)));
-		goto done;
-	}
-
-	if (global_blob.info.info0 == NULL) {
-		status = NT_STATUS_INTERNAL_DB_CORRUPTION;
-		DEBUG(1,("Invalid record in smbXsrv_tcon_global.tdb:"
-			 "key '%s' info0 NULL pointer - %s\n",
-			 tdb_data_dbg(key),
-			 nt_errstr(status)));
-		goto done;
-	}
-
-	*global = talloc_move(mem_ctx, &global_blob.info.info0);
-	status = NT_STATUS_OK;
-done:
-	talloc_free(frame);
-	return status;
-}
-
 struct smbXsrv_open_global_traverse_state {
 	int (*fn)(struct smbXsrv_open_global0 *, void *);
 	void *private_data;
@@ -1345,10 +1309,13 @@ static int smbXsrv_open_global_traverse_fn(struct db_record *rec, void *data)
 	struct smbXsrv_open_global_traverse_state *state =
 		(struct smbXsrv_open_global_traverse_state*)data;
 	struct smbXsrv_open_global0 *global = NULL;
+	TDB_DATA key = dbwrap_record_get_key(rec);
+	TDB_DATA val = dbwrap_record_get_value(rec);
 	NTSTATUS status;
 	int ret = -1;
 
-	status = smbXsrv_open_global_parse_record(talloc_tos(), rec, &global);
+	status = smbXsrv_open_global_parse_record(
+		talloc_tos(), key, val, &global);
 	if (!NT_STATUS_IS_OK(status)) {
 		return -1;
 	}
@@ -1394,7 +1361,7 @@ NTSTATUS smbXsrv_open_cleanup(uint64_t persistent_id)
 	NTSTATUS status = NT_STATUS_OK;
 	TALLOC_CTX *frame = talloc_stackframe();
 	struct smbXsrv_open_global0 *op = NULL;
-	TDB_DATA val;
+	TDB_DATA key, val;
 	struct db_record *rec;
 	bool delete_open = false;
 	uint32_t global_id = persistent_id & UINT32_MAX;
@@ -1407,7 +1374,9 @@ NTSTATUS smbXsrv_open_cleanup(uint64_t persistent_id)
 		goto done;
 	}
 
+	key = dbwrap_record_get_key(rec);
 	val = dbwrap_record_get_value(rec);
+
 	if (val.dsize == 0) {
 		DEBUG(10, ("smbXsrv_open_cleanup[global: 0x%08x] "
 			  "empty record in %s, skipping...\n",
@@ -1415,7 +1384,8 @@ NTSTATUS smbXsrv_open_cleanup(uint64_t persistent_id)
 		goto done;
 	}
 
-	status = smbXsrv_open_global_parse_record(talloc_tos(), rec, &op);
+	status = smbXsrv_open_global_parse_record(
+		talloc_tos(), key, val, &op);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(1, ("smbXsrv_open_cleanup[global: 0x%08x] "
 			  "failed to read record: %s\n",


-- 
Samba Shared Repository



More information about the samba-cvs mailing list