From f6e7d85e632c7e5ef63cefe7dc3be2e83978e269 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 10 Jul 2025 15:33:48 +0200 Subject: [PATCH] smbd: simplify create-replay Just check state->open_was_deferred and skip calling smb2srv_open_lookup_replay_cache() if it is set. Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source3/librpc/idl/smbXsrv.idl | 3 +- source3/smbd/smb2_create.c | 19 ++++++++++- source3/smbd/smbXsrv_open.c | 62 ++++++++-------------------------- source3/smbd/smbXsrv_open.h | 1 - 4 files changed, 33 insertions(+), 52 deletions(-) diff --git a/source3/librpc/idl/smbXsrv.idl b/source3/librpc/idl/smbXsrv.idl index 9aaa836cd7a..1c4ed7d88a2 100644 --- a/source3/librpc/idl/smbXsrv.idl +++ b/source3/librpc/idl/smbXsrv.idl @@ -493,9 +493,8 @@ interface smbXsrv [switch_is(version)] smbXsrv_openU info; } smbXsrv_openB; - const uint32 SMBXSRV_OPEN_REPLAY_CACHE_FIXED_SIZE = 28; + const uint32 SMBXSRV_OPEN_REPLAY_CACHE_FIXED_SIZE = 12; typedef [public] struct { - GUID holder_req_guid; NTTIME idle_time; uint32 local_id; } smbXsrv_open_replay_cache; diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c index 80b2d35df48..453e14e90b4 100644 --- a/source3/smbd/smb2_create.c +++ b/source3/smbd/smb2_create.c @@ -1433,8 +1433,25 @@ static void smbd_smb2_cc_before_exec_dhc2q(struct tevent_req *req) flags = IVAL(hdr, SMB2_HDR_FLAGS); state->replay_operation = flags & SMB2_HDR_FLAG_REPLAY_OPERATION; + if (state->open_was_deferred) { + /* + * When processing a redispatched deferred open, we have the + * following state: + * - no OPEN record + * - RC record with global_file_id=0 (pending open state) + * + * We just skip calling smb2srv_open_lookup_replay_cache() as + * - we already have a RC record + * - it would fail with NT_STATUS_FILE_NOT_AVAILABLE which is + * not what we want in this "internal replay" case + * + * So we just set replay_operation to false and move on. + */ + state->replay_operation = false; + return; + } + status = smb2srv_open_lookup_replay_cache(smb2req->xconn, - state->req_guid, *state->create_guid, state->fname, now, diff --git a/source3/smbd/smbXsrv_open.c b/source3/smbd/smbXsrv_open.c index e37aab2ce42..e71a547f264 100644 --- a/source3/smbd/smbXsrv_open.c +++ b/source3/smbd/smbXsrv_open.c @@ -992,26 +992,23 @@ NTSTATUS smb2srv_open_lookup(struct smbXsrv_connection *conn, * cases: * * 1. There is no record in the cache - * => we add the passes caller_req_guid as holder_req_guid - * together with local_id as 0. * => We return STATUS_FWP_RESERVED in order to indicate * that the caller holds the current reservation + * This is the "normal" CREATE processing * - * 2. There is a record in the cache and holder_req_guid - * is already the same as caller_req_guid and local_id is 0 - * => We return STATUS_FWP_RESERVED in order to indicate - * that the caller holds the current reservation - * - * 3. There is a record in the cache with a holder_req_guid - * other than caller_req_guid (and local_id is 0): + * 2. There is a record in the cache and open_persistent_id is 0 * => We return NT_STATUS_FILE_NOT_AVAILABLE to indicate * the original request is still pending + * This is the CREATE pending (still being processed) case * - * 4. There is a record in the cache with a zero holder_req_guid - * and a valid local_id: - * => We lookup the existing open by local_id - * => We return NT_STATUS_OK together with the smbXsrv_open - * + * 3. There is a record in the cache with a valid open_persistent_id: + * => We lookup the existing global open by open_persistent_id + * => We lookup a matching local open and return NT_STATUS_OK together + * with the smbXsrv_open if one is found + * This is the successful CREATE-replay case in the same process + * => Otherwise we return NT_STATUS_HANDLE_NO_LONGER_VALID together with + * open_persistent_id to trigger a handle reconnect + * This is the successful CREATE-replay case in a different process * * With NT_STATUS_OK the caller can continue the replay processing. * @@ -1019,8 +1016,7 @@ NTSTATUS smb2srv_open_lookup(struct smbXsrv_connection *conn, * open processing: * - On success: * - smbXsrv_open_update()/smbXsrv_open_set_replay_cache() - * will convert the record to a zero holder_req_guid - * with a valid local_id. + * will update the record with a valid open_persistent_id. * - On failure: * - smbXsrv_open_purge_replay_cache() should cleanup * the reservation. @@ -1030,7 +1026,6 @@ NTSTATUS smb2srv_open_lookup(struct smbXsrv_connection *conn, * retry loop on the client. */ NTSTATUS smb2srv_open_lookup_replay_cache(struct smbXsrv_connection *conn, - struct GUID caller_req_guid, struct GUID create_guid, const char *name, NTTIME now, @@ -1040,14 +1035,12 @@ 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 tmp_guid_buf; 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 = { - .holder_req_guid = caller_req_guid, .idle_time = now, .local_id = 0, }; @@ -1117,21 +1110,6 @@ NTSTATUS smb2srv_open_lookup_replay_cache(struct smbXsrv_connection *conn, if (rc.local_id != 0) { DBG_DEBUG("Found replay-cache record with local_id\n"); - if (GUID_equal(&rc.holder_req_guid, &caller_req_guid)) { - /* - * This should not happen - */ - status = NT_STATUS_INTERNAL_ERROR; - DBG_ERR("caller %s already holds local_id %u for create %s [%s] - %s\n", - GUID_buf_string(&caller_req_guid, &tmp_guid_buf), - (unsigned)rc.local_id, - create_guid_str, - name, - nt_errstr(status)); - - TALLOC_FREE(frame); - return status; - } status = smbXsrv_open_local_lookup(table, rc.local_id, @@ -1139,8 +1117,7 @@ NTSTATUS smb2srv_open_lookup_replay_cache(struct smbXsrv_connection *conn, now, &op); if (!NT_STATUS_IS_OK(status)) { - DBG_ERR("holder %s stale for local_id %u for create %s [%s] - %s\n", - GUID_buf_string(&rc.holder_req_guid, &tmp_guid_buf), + DBG_ERR("stale local_id %u for create %s [%s] - %s\n", (unsigned)rc.local_id, create_guid_str, name, @@ -1159,23 +1136,12 @@ NTSTATUS smb2srv_open_lookup_replay_cache(struct smbXsrv_connection *conn, return NT_STATUS_OK; } - if (GUID_equal(&rc.holder_req_guid, &caller_req_guid)) { - DBG_DEBUG("Still the holder\n"); - /* - * We're still the holder - */ - *_open = NULL; - TALLOC_FREE(frame); - return NT_STATUS_FWP_RESERVED; - } - /* * The original request (or a former replay) is still * pending, ask the client to retry by sending FILE_NOT_AVAILABLE. */ status = NT_STATUS_FILE_NOT_AVAILABLE; - DBG_DEBUG("holder %s still pending for create %s [%s] - %s\n", - GUID_buf_string(&rc.holder_req_guid, &tmp_guid_buf), + DBG_DEBUG("still pending create %s [%s] - %s\n", create_guid_str, name, nt_errstr(status)); diff --git a/source3/smbd/smbXsrv_open.h b/source3/smbd/smbXsrv_open.h index 58e585888e2..94672f2ada1 100644 --- a/source3/smbd/smbXsrv_open.h +++ b/source3/smbd/smbXsrv_open.h @@ -53,7 +53,6 @@ NTSTATUS smb2srv_open_lookup(struct smbXsrv_connection *conn, NTSTATUS smbXsrv_open_purge_replay_cache(struct smbXsrv_client *client, const struct GUID *create_guid); NTSTATUS smb2srv_open_lookup_replay_cache(struct smbXsrv_connection *conn, - struct GUID caller_req_guid, struct GUID create_guid, const char *name, NTTIME now, -- 2.47.2