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 <v...@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 <v...@samba.org>
Autobuild-User(master): Volker Lendecke <v...@samba.org>
Autobuild-Date(master): Tue Jan 24 09:15:26 UTC 2023 on atb-devel-224
commit 132b83d0659ddc25a96327edc1c7dd23b17a56fd
Author: Volker Lendecke <v...@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 <v...@samba.org>
commit 2f6776741dc6469d78b94da22d75f26cccca5fc9
Author: Volker Lendecke <v...@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 <v...@samba.org>
commit 3c779de8cf99d0936956a12484fd726d5be46c7e
Author: Volker Lendecke <v...@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 <v...@samba.org>
commit f1a66267bcfcd48f3c7ca2ada3f62d40209163e3
Author: Volker Lendecke <v...@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 <v...@samba.org>
commit 35a32171b5067d5b80acffc99f8d43cdc7f5f9a7
Author: Volker Lendecke <v...@samba.org>
Date: Wed Jan 11 08:18:35 2023 +0100
smbd: Fix a typo
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@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
