The branch, master has been updated via cc4e11d0282 smbd: Remove smbXsrv_open_global0->db_rec via 1bd16bc6d45 smbd: Use dbwrap_do_locked() in smb2srv_open_recreate() via fede6b9f465 smbd: rename 'op' into 'global' in smbXsrv_open_cleanup_fn() via ca872ad6ba1 smbd: let smbXsrv_open_cleanup() delete broken records via a69950db4a7 smbd: Use dbwrap_do_locked() in smbXsrv_open_cleanup() via 62a66331934 smbd: Use dbwrap_do_locked() in smbXsrv_open_close() via 26b29ecbb9d smbd: Use dbwrap_do_locked() in smbXsrv_open_update() via bfede670bd4 smbd: Use dbwrap_do_locked() in smbXsrv_open_global_allocate() via 84d22dc5f57 smbd: Make smbXsrv_open_global_allocate() store the record via 95e3ad7e437 smbd: Simplify smbXsrv_open_global_store() via fafebc46c8b smbd: Move smbXsrv_open_global_verify_record() down in smbXsrv_open.c via a93d93a97df smbd: Use generate_nonce_buffer() in smbXsrv_open_global_allocate() from e8abe52df2d s3: smbd: Fix log spam. Change a normal error message from DBG_ERR (level 0) to DBG_INFO (level 5).
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit cc4e11d02826526e61e85e1a939c515d01323dcb Author: Volker Lendecke <v...@samba.org> Date: Wed Jan 11 11:02:11 2023 +0100 smbd: Remove smbXsrv_open_global0->db_rec The only user by now was net serverid wipedbs, and there it was easy to replace Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> Autobuild-User(master): Stefan Metzmacher <me...@samba.org> Autobuild-Date(master): Mon Feb 13 10:49:43 UTC 2023 on atb-devel-224 commit 1bd16bc6d451e810dc215e7638de483a6e2d04a6 Author: Volker Lendecke <v...@samba.org> Date: Wed Jan 11 10:54:37 2023 +0100 smbd: Use dbwrap_do_locked() in smb2srv_open_recreate() Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit fede6b9f4652588825fdd4b458fcf23250339e79 Author: Stefan Metzmacher <me...@samba.org> Date: Tue Jan 31 12:39:06 2023 +0100 smbd: rename 'op' into 'global' in smbXsrv_open_cleanup_fn() Signed-off-by: Stefan Metzmacher <me...@samba.org> Reviewed-by: Volker Lendecke <v...@samba.org> commit ca872ad6ba1c7f84af5a9be89de5d2973d2cd87e Author: Volker Lendecke <v...@samba.org> Date: Tue Jan 10 12:29:18 2023 +0100 smbd: let smbXsrv_open_cleanup() delete broken records Pair-Programmed-With: Stefan Metzmacher <me...@samba.org> Signed-off-by: Volker Lendecke <v...@samba.org> Signed-off-by: Stefan Metzmacher <me...@samba.org> commit a69950db4a7344ee1bec8fc7b66a402597f578a2 Author: Volker Lendecke <v...@samba.org> Date: Tue Jan 10 12:29:18 2023 +0100 smbd: Use dbwrap_do_locked() in smbXsrv_open_cleanup() Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 62a66331934b298f9df1e661b61cb4c193d1a5a0 Author: Volker Lendecke <v...@samba.org> Date: Tue Jan 10 11:59:07 2023 +0100 smbd: Use dbwrap_do_locked() in smbXsrv_open_close() Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 26b29ecbb9dbc518856cd59629e1d291540e4ba7 Author: Volker Lendecke <v...@samba.org> Date: Sun Jan 8 21:04:25 2023 +0100 smbd: Use dbwrap_do_locked() in smbXsrv_open_update() Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit bfede670bd4152d22897ee52a176dd6e620974e6 Author: Volker Lendecke <v...@samba.org> Date: Thu Jan 26 09:08:27 2023 +0100 smbd: Use dbwrap_do_locked() in smbXsrv_open_global_allocate() Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 84d22dc5f57393baf5a914815eedd9536e398026 Author: Volker Lendecke <v...@samba.org> Date: Fri Jan 6 17:12:23 2023 +0100 smbd: Make smbXsrv_open_global_allocate() store the record Micro-step towards using dbwrap_do_locked() Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 95e3ad7e4378e1d82da8eb745147539a96a28f8c Author: Volker Lendecke <v...@samba.org> Date: Thu Jan 5 16:18:37 2023 +0100 smbd: Simplify smbXsrv_open_global_store() Avoid the dependency on global->db_rec. This makes the callers more verbose, but it makes the data dependencies much more obvious. This will enable removing smbXsrv_open_global0->db_rec at some point. Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit fafebc46c8bf624736995f3a87819b3c075cb383 Author: Volker Lendecke <v...@samba.org> Date: Thu Jan 26 08:46:31 2023 +0100 smbd: Move smbXsrv_open_global_verify_record() down in smbXsrv_open.c Avoid prototypes Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit a93d93a97df9ffb1c76c9923e147743d6865ff6a Author: Volker Lendecke <v...@samba.org> Date: Fri Jan 6 16:46:11 2023 +0100 smbd: Use generate_nonce_buffer() in smbXsrv_open_global_allocate() We don't need anything cryptographic for persistent file handle ids Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> ----------------------------------------------------------------------- Summary of changes: source3/librpc/idl/smbXsrv.idl | 1 - source3/smbd/smbXsrv_open.c | 861 ++++++++++++++++++++++------------------- source3/smbd/smbXsrv_open.h | 4 +- source3/utils/net_serverid.c | 9 +- 4 files changed, 480 insertions(+), 395 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/librpc/idl/smbXsrv.idl b/source3/librpc/idl/smbXsrv.idl index e0a751f32de..173bc64db77 100644 --- a/source3/librpc/idl/smbXsrv.idl +++ b/source3/librpc/idl/smbXsrv.idl @@ -459,7 +459,6 @@ interface smbXsrv } smbXsrv_open_flags; typedef struct { - [ignore] db_record *db_rec; server_id server_id; uint32 open_global_id; hyper open_persistent_id; diff --git a/source3/smbd/smbXsrv_open.c b/source3/smbd/smbXsrv_open.c index 585d1ec0838..3975a7e6cff 100644 --- a/source3/smbd/smbXsrv_open.c +++ b/source3/smbd/smbXsrv_open.c @@ -34,6 +34,7 @@ #include "serverid.h" #include "source3/include/util_tdb.h" #include "lib/util/idtree_random.h" +#include "lib/util/time_basic.h" struct smbXsrv_open_table { struct { @@ -107,26 +108,6 @@ static TDB_DATA smbXsrv_open_global_id_to_key( }; } -static struct db_record *smbXsrv_open_global_fetch_locked( - struct db_context *db, - uint32_t id, - TALLOC_CTX *mem_ctx) -{ - struct smbXsrv_open_global_key_buf key_buf; - TDB_DATA key = smbXsrv_open_global_id_to_key(id, &key_buf); - struct db_record *rec = NULL; - - - rec = dbwrap_fetch_locked(db, mem_ctx, key); - - if (rec == NULL) { - DBG_DEBUG("Failed to lock global id 0x%08x, key '%s'\n", id, - tdb_data_dbg(key)); - } - - return rec; -} - static NTSTATUS smbXsrv_open_table_init(struct smbXsrv_connection *conn, uint32_t lowest_id, uint32_t highest_id, @@ -225,102 +206,6 @@ static NTSTATUS smbXsrv_open_local_lookup(struct smbXsrv_open_table *table, return NT_STATUS_OK; } -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) -{ - uint32_t i; - uint32_t last_free = 0; - const uint32_t min_tries = 3; - - /* - * Here we just randomly try the whole 32-bit space - * - * We use just 32-bit, because we want to reuse the - * ID for SRVSVC. - */ - for (i = 0; i < UINT32_MAX; i++) { - 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; - } else { - id = generate_random(); - } - if (id == 0) { - id++; - } - if (id == UINT32_MAX) { - id--; - } - - 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); - - status = smbXsrv_open_global_verify_record( - key, val, talloc_tos(), &tmp_global0); - - if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) { - /* - * Found an empty slot - */ - global->open_global_id = id; - return NT_STATUS_OK; - } - - TALLOC_FREE(tmp_global0); - - if (NT_STATUS_EQUAL(status, NT_STATUS_FATAL_APP_EXIT)) { - /* - * smbd crashed - */ - 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; - } - } - - 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)); - } - - TALLOC_FREE(global->db_rec); - } - - /* should not be reached */ - return NT_STATUS_INTERNAL_ERROR; -} - static NTSTATUS smbXsrv_open_global_parse_record( TALLOC_CTX *mem_ctx, TDB_DATA key, @@ -413,12 +298,15 @@ static NTSTATUS smbXsrv_open_global_verify_record( return NT_STATUS_FATAL_APP_EXIT; } -static NTSTATUS smbXsrv_open_global_store(struct smbXsrv_open_global0 *global) +static NTSTATUS smbXsrv_open_global_store( + struct db_record *rec, + TDB_DATA key, + TDB_DATA oldval, + struct smbXsrv_open_global0 *global) { struct smbXsrv_open_globalB global_blob; DATA_BLOB blob = data_blob_null; - TDB_DATA key; - TDB_DATA val; + TDB_DATA val = { .dptr = NULL, }; NTSTATUS status; enum ndr_err_code ndr_err; @@ -428,15 +316,12 @@ static NTSTATUS smbXsrv_open_global_store(struct smbXsrv_open_global0 *global) * store the information in the old format. */ - key = dbwrap_record_get_key(global->db_rec); - val = dbwrap_record_get_value(global->db_rec); - global_blob = (struct smbXsrv_open_globalB) { .version = smbXsrv_version_global_current(), }; - if (val.dsize >= 8) { - global_blob.seqnum = IVAL(val.dptr, 4); + if (oldval.dsize >= 8) { + global_blob.seqnum = IVAL(oldval.dptr, 4); } global_blob.seqnum += 1; global_blob.info.info0 = global; @@ -447,18 +332,16 @@ static NTSTATUS smbXsrv_open_global_store(struct smbXsrv_open_global0 *global) DBG_WARNING("key '%s' ndr_push - %s\n", tdb_data_dbg(key), ndr_map_error2string(ndr_err)); - TALLOC_FREE(global->db_rec); return ndr_map_error2ntstatus(ndr_err); } val = make_tdb_data(blob.data, blob.length); - status = dbwrap_record_store(global->db_rec, val, TDB_REPLACE); + status = dbwrap_record_store(rec, val, TDB_REPLACE); TALLOC_FREE(blob.data); if (!NT_STATUS_IS_OK(status)) { DBG_WARNING("key '%s' store - %s\n", tdb_data_dbg(key), nt_errstr(status)); - TALLOC_FREE(global->db_rec); return status; } @@ -467,47 +350,152 @@ static NTSTATUS smbXsrv_open_global_store(struct smbXsrv_open_global0 *global) NDR_PRINT_DEBUG(smbXsrv_open_globalB, &global_blob); } - TALLOC_FREE(global->db_rec); - return NT_STATUS_OK; } -static NTSTATUS smbXsrv_open_global_lookup(struct smbXsrv_open_table *table, - uint32_t open_global_id, - 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; +struct smbXsrv_open_global_allocate_state { + uint32_t id; + struct smbXsrv_open_global0 *global; NTSTATUS status; +}; - *_global = NULL; +static void smbXsrv_open_global_allocate_fn( + struct db_record *rec, TDB_DATA oldval, void *private_data) +{ + struct smbXsrv_open_global_allocate_state *state = private_data; + struct smbXsrv_open_global0 *global = state->global; + struct smbXsrv_open_global0 *tmp_global0 = NULL; + TDB_DATA key = dbwrap_record_get_key(rec); - if (table->global.db_ctx == NULL) { - return NT_STATUS_INTERNAL_ERROR; + state->status = smbXsrv_open_global_verify_record( + key, oldval, talloc_tos(), &tmp_global0); + + if (NT_STATUS_IS_OK(state->status)) { + /* + * Found an existing record + */ + TALLOC_FREE(tmp_global0); + state->status = NT_STATUS_RETRY; + return; } - global_rec = dbwrap_fetch_locked(table->global.db_ctx, mem_ctx, key); - if (global_rec == NULL) { - return NT_STATUS_INTERNAL_DB_ERROR; + if (NT_STATUS_EQUAL(state->status, NT_STATUS_NOT_FOUND)) { + /* + * Found an empty slot + */ + global->open_global_id = state->id; + global->open_persistent_id = state->id; + + state->status = smbXsrv_open_global_store( + rec, key, (TDB_DATA) { .dsize = 0, }, state->global); + if (!NT_STATUS_IS_OK(state->status)) { + DBG_WARNING("smbXsrv_open_global_store() for " + "id %"PRIu32" failed: %s\n", + state->id, + nt_errstr(state->status)); + } + return; } - val = dbwrap_record_get_value(global_rec); - 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; + if (NT_STATUS_EQUAL(state->status, NT_STATUS_FATAL_APP_EXIT)) { + NTSTATUS status; + + TALLOC_FREE(tmp_global0); + + /* + * smbd crashed + */ + status = dbwrap_record_delete(rec); + if (!NT_STATUS_IS_OK(status)) { + DBG_WARNING("dbwrap_record_delete() failed " + "for record %"PRIu32": %s\n", + state->id, + nt_errstr(status)); + state->status = NT_STATUS_INTERNAL_DB_CORRUPTION; + return; + } + return; } +} - TALLOC_FREE(global_rec); +static NTSTATUS smbXsrv_open_global_allocate( + struct db_context *db, struct smbXsrv_open_global0 *global) +{ + struct smbXsrv_open_global_allocate_state state = { + .global = global, + }; + uint32_t i; + uint32_t last_free = 0; + const uint32_t min_tries = 3; - if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) { - return NT_STATUS_OBJECT_NAME_NOT_FOUND; + /* + * Here we just randomly try the whole 32-bit space + * + * We use just 32-bit, because we want to reuse the + * ID for SRVSVC. + */ + for (i = 0; i < UINT32_MAX; i++) { + struct smbXsrv_open_global_key_buf key_buf; + TDB_DATA key; + NTSTATUS status; + + if (i >= min_tries && last_free != 0) { + state.id = last_free; + } else { + generate_nonce_buffer( + (uint8_t *)&state.id, sizeof(state.id)); + state.id = MAX(state.id, 1); + state.id = MIN(state.id, UINT32_MAX-1); + } + + key = smbXsrv_open_global_id_to_key(state.id, &key_buf); + + status = dbwrap_do_locked( + db, key, smbXsrv_open_global_allocate_fn, &state); + + if (!NT_STATUS_IS_OK(status)) { + DBG_WARNING("dbwrap_do_locked() failed: %s\n", + nt_errstr(status)); + return NT_STATUS_INTERNAL_DB_ERROR; + } + + if (NT_STATUS_IS_OK(state.status)) { + /* + * Found an empty slot, done. + */ + DBG_DEBUG("Found slot %"PRIu32"\n", state.id); + return NT_STATUS_OK; + } + + if (NT_STATUS_EQUAL(state.status, NT_STATUS_FATAL_APP_EXIT)) { + + if ((i < min_tries) && (last_free == 0)) { + /* + * Remember "id" as free but also try + * others to not recycle ids too + * quickly. + */ + last_free = state.id; + } + continue; + } + + if (NT_STATUS_EQUAL(state.status, NT_STATUS_RETRY)) { + /* + * Normal collision, try next + */ + DBG_DEBUG("Found record for id %"PRIu32"\n", + state.id); + continue; + } + + DBG_WARNING("smbXsrv_open_global_allocate_fn() failed: %s\n", + nt_errstr(state.status)); + return state.status; } - return status; + /* should not be reached */ + return NT_STATUS_INTERNAL_ERROR; } static int smbXsrv_open_destructor(struct smbXsrv_open *op) @@ -582,12 +570,6 @@ NTSTATUS smbXsrv_open_create(struct smbXsrv_connection *conn, memset(global->lock_sequence_array, 0xFF, sizeof(global->lock_sequence_array)); - status = smbXsrv_open_global_allocate(table->global.db_ctx, global); - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(op); - return status; - } - local_id = idr_get_new_random( table->local.idr, op, @@ -599,7 +581,6 @@ NTSTATUS smbXsrv_open_create(struct smbXsrv_connection *conn, } op->local_id = local_id; - global->open_persistent_id = global->open_global_id; global->open_volatile_id = op->local_id; global->server_id = messaging_server_id(conn->client->msg_ctx); @@ -609,20 +590,21 @@ NTSTATUS smbXsrv_open_create(struct smbXsrv_connection *conn, global->client_guid = conn->smb2.client.guid; } - table->local.num_opens += 1; - - talloc_set_destructor(op, smbXsrv_open_destructor); - - status = smbXsrv_open_global_store(global); + status = smbXsrv_open_global_allocate(table->global.db_ctx, + global); if (!NT_STATUS_IS_OK(status)) { - DEBUG(0,("smbXsrv_open_create: " - "global_id (0x%08x) store failed - %s\n", - op->global->open_global_id, - nt_errstr(status))); + int ret = idr_remove(table->local.idr, local_id); + SMB_ASSERT(ret == 0); + + DBG_WARNING("smbXsrv_open_global_allocate() failed: %s\n", + nt_errstr(status)); TALLOC_FREE(op); return status; } + table->local.num_opens += 1; + talloc_set_destructor(op, smbXsrv_open_destructor); + if (CHECK_DEBUGLVL(10)) { struct smbXsrv_openB open_blob = { .version = SMBXSRV_VERSION_0, @@ -741,33 +723,45 @@ static NTSTATUS smbXsrv_open_clear_replay_cache(struct smbXsrv_open *op) return status; } +struct smbXsrv_open_update_state { + struct smbXsrv_open_global0 *global; + NTSTATUS status; +}; + +static void smbXsrv_open_update_fn( + struct db_record *rec, TDB_DATA oldval, void *private_data) +{ + struct smbXsrv_open_update_state *state = private_data; + TDB_DATA key = dbwrap_record_get_key(rec); + + state->status = smbXsrv_open_global_store( + rec, key, oldval, state->global); +} + NTSTATUS smbXsrv_open_update(struct smbXsrv_open *op) { + struct smbXsrv_open_update_state state = { .global = op->global, }; struct smbXsrv_open_table *table = op->table; + struct smbXsrv_open_global_key_buf key_buf; + TDB_DATA key = smbXsrv_open_global_id_to_key( + op->global->open_global_id, &key_buf); NTSTATUS status; - if (op->global->db_rec != NULL) { - DEBUG(0, ("smbXsrv_open_update(0x%08x): " - "Called with db_rec != NULL'\n", - op->global->open_global_id)); - return NT_STATUS_INTERNAL_ERROR; - } - - op->global->db_rec = smbXsrv_open_global_fetch_locked( - table->global.db_ctx, - op->global->open_global_id, - op->global /* TALLOC_CTX */); - if (op->global->db_rec == NULL) { + status = dbwrap_do_locked( + table->global.db_ctx, key, smbXsrv_open_update_fn, &state); + if (!NT_STATUS_IS_OK(status)) { + DBG_WARNING("global_id (0x%08x) dbwrap_do_locked failed: %s\n", + op->global->open_global_id, + nt_errstr(status)); return NT_STATUS_INTERNAL_DB_ERROR; } - status = smbXsrv_open_global_store(op->global); - if (!NT_STATUS_IS_OK(status)) { - DEBUG(0,("smbXsrv_open_update: " - "global_id (0x%08x) store failed - %s\n", - op->global->open_global_id, - nt_errstr(status))); - return status; + if (!NT_STATUS_IS_OK(state.status)) { + DBG_WARNING("global_id (0x%08x) smbXsrv_open_global_store " -- Samba Shared Repository