From e2fa20763af2f132e3f921bd7b1a8160ab6dbdf6 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 2 Jul 2020 14:09:15 +0200 Subject: [PATCH 1/7] smbd: increase loglevel when leases_db_del() with anything then NT_STATUS_NOT_FOUND BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher (cherry picked from commit fbb8bbe1243eb2a0351dc2422929278f85a99e26) --- source3/locking/locking.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/source3/locking/locking.c b/source3/locking/locking.c index 1220cb3a2be1..2d9569809e44 100644 --- a/source3/locking/locking.c +++ b/source3/locking/locking.c @@ -728,8 +728,13 @@ NTSTATUS remove_lease_if_stale(struct share_mode_lock *lck, status = leases_db_del(client_guid, lease_key, &d->id); if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("leases_db_del failed: %s\n", - nt_errstr(status)); + int level = DBGLVL_DEBUG; + + if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) { + level = DBGLVL_ERR; + } + DBG_PREFIX(level, ("leases_db_del failed: %s\n", + nt_errstr(status))); } return status; } -- 2.17.1 From dd816924ac282605d8d37b95d60f8691f54b8362 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 2 Jul 2020 14:10:05 +0200 Subject: [PATCH 2/7] s3/leases: log NDR decoding failure with level 0 in leases_db_get_fn() BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428 Signed-off-by: Ralph Boehme (cherry picked from commit 383a2457bd6cbe0acd571a8d601f8bdc5365f0b4) --- source3/locking/leases_db.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/locking/leases_db.c b/source3/locking/leases_db.c index a12b421d2603..2e2ccb150ac3 100644 --- a/source3/locking/leases_db.c +++ b/source3/locking/leases_db.c @@ -549,8 +549,8 @@ static void leases_db_get_fn(TDB_DATA key, TDB_DATA data, void *private_data) &blob, value, value, (ndr_pull_flags_fn_t)ndr_pull_leases_db_value); if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - DBG_DEBUG("ndr_pull_struct_blob_failed: %s\n", - ndr_errstr(ndr_err)); + DBG_ERR("ndr_pull_struct_blob_failed: %s\n", + ndr_errstr(ndr_err)); TALLOC_FREE(value); state->status = ndr_map_error2ntstatus(ndr_err); return; -- 2.17.1 From 0c23e5489e9783299e51062a75cfe150a2a3d084 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 2 Jul 2020 14:08:44 +0200 Subject: [PATCH 3/7] smbd: inverse if/else logic in get_lease_type() No change in behaviour. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher (cherry picked from commit e4328db1c94837a8ea5652971cea20055d3d24ff) --- source3/smbd/oplock.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c index 2c4449b10b26..c63f97aac0ad 100644 --- a/source3/smbd/oplock.c +++ b/source3/smbd/oplock.c @@ -171,24 +171,24 @@ static void downgrade_file_oplock(files_struct *fsp) uint32_t get_lease_type(const struct share_mode_entry *e, struct file_id id) { - if (e->op_type == LEASE_OPLOCK) { - NTSTATUS status; - uint32_t current_state; + NTSTATUS status; + uint32_t current_state; - status = leases_db_get( - &e->client_guid, - &e->lease_key, - &id, - ¤t_state, - NULL, /* breaking */ - NULL, /* breaking_to_requested */ - NULL, /* breaking_to_required */ - NULL, /* lease_version */ - NULL); /* epoch */ - SMB_ASSERT(NT_STATUS_IS_OK(status)); - return current_state; - } - return map_oplock_to_lease_type(e->op_type); + if (e->op_type != LEASE_OPLOCK) { + return map_oplock_to_lease_type(e->op_type); + } + + status = leases_db_get(&e->client_guid, + &e->lease_key, + &id, + ¤t_state, + NULL, /* breaking */ + NULL, /* breaking_to_requested */ + NULL, /* breaking_to_required */ + NULL, /* lease_version */ + NULL); /* epoch */ + SMB_ASSERT(NT_STATUS_IS_OK(status)); + return current_state; } /**************************************************************************** -- 2.17.1 From 39394ffea15f0fa3d49e85f984473d7033ab7e28 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 2 Jul 2020 14:45:59 +0200 Subject: [PATCH 4/7] smbd: let get_lease_type() take a non-const share_mode_entry We're going to add a call to share_entry_stale_pid(share_mode_entry) which takes a non-const pointer (in order to eventually set e->state = true). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher (cherry picked from commit 3f4a865821da27efbed4f7c38ad3efbcaae77a02) --- source3/smbd/oplock.c | 2 +- source3/smbd/proto.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c index c63f97aac0ad..97ab05b2ba67 100644 --- a/source3/smbd/oplock.c +++ b/source3/smbd/oplock.c @@ -169,7 +169,7 @@ static void downgrade_file_oplock(files_struct *fsp) TALLOC_FREE(fsp->oplock_timeout); } -uint32_t get_lease_type(const struct share_mode_entry *e, struct file_id id) +uint32_t get_lease_type(struct share_mode_entry *e, struct file_id id) { NTSTATUS status; uint32_t current_state; diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 60941ce6c1b2..ac021ad93fe1 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -736,7 +736,7 @@ NTSTATUS create_file_default(connection_struct *conn, /* The following definitions come from smbd/oplock.c */ -uint32_t get_lease_type(const struct share_mode_entry *e, struct file_id id); +uint32_t get_lease_type(struct share_mode_entry *e, struct file_id id); void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp); NTSTATUS set_file_oplock(files_struct *fsp); -- 2.17.1 From abb4a240e9a11ef39cbbccb1cd04793caec71c4a Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 2 Jul 2020 14:47:12 +0200 Subject: [PATCH 5/7] smbd: check for stale pid in get_lease_type() If leases_db_get() failed the leases_db record might have been cleaned up for stale processes. Check if the share-mode-entry owner is stale in this case and return a 0 lease state. In any other case, log a debug messages and panic. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428 Signed-off-by: Ralph Boehme Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Thu Jul 2 16:45:42 UTC 2020 on sn-devel-184 (cherry picked from commit 05d4466a6d1ad048fa86aea09ec0a56a7b961369) --- source3/smbd/oplock.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c index 97ab05b2ba67..1c1510f3aabc 100644 --- a/source3/smbd/oplock.c +++ b/source3/smbd/oplock.c @@ -171,6 +171,8 @@ static void downgrade_file_oplock(files_struct *fsp) uint32_t get_lease_type(struct share_mode_entry *e, struct file_id id) { + struct GUID_txt_buf guid_strbuf; + struct file_id_buf file_id_strbuf; NTSTATUS status; uint32_t current_state; @@ -187,8 +189,22 @@ uint32_t get_lease_type(struct share_mode_entry *e, struct file_id id) NULL, /* breaking_to_required */ NULL, /* lease_version */ NULL); /* epoch */ - SMB_ASSERT(NT_STATUS_IS_OK(status)); - return current_state; + if (NT_STATUS_IS_OK(status)) { + return current_state; + } + + if (share_entry_stale_pid(e)) { + return 0; + } + DBG_ERR("leases_db_get for client_guid [%s] " + "lease_key [%"PRIu64"/%"PRIu64"] " + "file_id [%s] failed: %s\n", + GUID_buf_string(&e->client_guid, &guid_strbuf), + e->lease_key.data[0], + e->lease_key.data[1], + file_id_str_buf(id, &file_id_strbuf), + nt_errstr(status)); + smb_panic("leases_db_get() failed"); } /**************************************************************************** -- 2.17.1 From 14358848397e52f5ae6fba2ec63d5e78da41de52 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 6 Jul 2020 14:03:39 +0200 Subject: [PATCH 6/7] s3:leases: log errors with level 0 in leases_db_do_locked_fn() BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison (cherry picked from commit 217693682d5bbd0f2d6b5331f47b2a6348840898) --- source3/locking/leases_db.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source3/locking/leases_db.c b/source3/locking/leases_db.c index 2e2ccb150ac3..855d6143ad71 100644 --- a/source3/locking/leases_db.c +++ b/source3/locking/leases_db.c @@ -121,7 +121,7 @@ static void leases_db_do_locked_fn( value, (ndr_pull_flags_fn_t)ndr_pull_leases_db_value); if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - DBG_DEBUG("ndr_pull_struct_blob_failed: %s\n", + DBG_ERR("ndr_pull_struct_blob_failed: %s\n", ndr_errstr(ndr_err)); state->status = ndr_map_error2ntstatus(ndr_err); goto done; @@ -137,7 +137,7 @@ static void leases_db_do_locked_fn( if (value->num_files == 0) { state->status = dbwrap_record_delete(rec); if (!NT_STATUS_IS_OK(state->status)) { - DBG_DEBUG("dbwrap_record_delete returned %s\n", + DBG_ERR("dbwrap_record_delete returned %s\n", nt_errstr(state->status)); } goto done; @@ -149,7 +149,7 @@ static void leases_db_do_locked_fn( value, (ndr_push_flags_fn_t)ndr_push_leases_db_value); if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - DBG_DEBUG("ndr_push_struct_blob_failed: %s\n", + DBG_ERR("ndr_push_struct_blob_failed: %s\n", ndr_errstr(ndr_err)); state->status = ndr_map_error2ntstatus(ndr_err); goto done; @@ -164,7 +164,7 @@ static void leases_db_do_locked_fn( state->status = dbwrap_record_store(rec, db_value, 0); if (!NT_STATUS_IS_OK(state->status)) { - DBG_DEBUG("dbwrap_record_store returned %s\n", + DBG_ERR("dbwrap_record_store returned %s\n", nt_errstr(state->status)); } -- 2.17.1 From 9cabdfaa07177aad2025b89a49f4d9ae92906a6e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 6 Jul 2020 08:58:22 +0200 Subject: [PATCH 7/7] s3:smbd: check for stale pid in delay_for_oplock_fn() when leases_db_get() fails If leases_db_get() failed the leases_db record might have been cleaned up for stale processes. Check if the share-mode-entry owner is stale in this case and return ignore the entry. In any other case, log a debug messages and panic. Commit 05d4466a6d1ad048fa86aea09ec0a56a7b961369 "smbd: check for stale pid in get_lease_type()" fixed only one half of this. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Tue Jul 7 02:47:46 UTC 2020 on sn-devel-184 (cherry picked from commit 58adf349edfd3001ad071cc7ed8cfc551f67f8a2) --- source3/smbd/open.c | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 03e66d30a3e1..4a31b8226463 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -2380,7 +2380,42 @@ static bool delay_for_oplock_fn( NULL, /* breaking_to_required */ NULL, /* lease_version */ NULL); /* epoch */ - SMB_ASSERT(NT_STATUS_IS_OK(status)); + + /* + * leases_db_get() can return NT_STATUS_NOT_FOUND + * if the share_mode_entry e is stale and the + * lease record was already removed. In this case return + * false so the traverse continues. + */ + + if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND) && + share_entry_stale_pid(e)) + { + struct GUID_txt_buf guid_strbuf; + struct file_id_buf file_id_strbuf; + DBG_DEBUG("leases_db_get for client_guid [%s] " + "lease_key [%"PRIu64"/%"PRIu64"] " + "file_id [%s] failed for stale " + "share_mode_entry\n", + GUID_buf_string(&e->client_guid, &guid_strbuf), + e->lease_key.data[0], + e->lease_key.data[1], + file_id_str_buf(fsp->file_id, &file_id_strbuf)); + return false; + } + if (!NT_STATUS_IS_OK(status)) { + struct GUID_txt_buf guid_strbuf; + struct file_id_buf file_id_strbuf; + DBG_ERR("leases_db_get for client_guid [%s] " + "lease_key [%"PRIu64"/%"PRIu64"] " + "file_id [%s] failed: %s\n", + GUID_buf_string(&e->client_guid, &guid_strbuf), + e->lease_key.data[0], + e->lease_key.data[1], + file_id_str_buf(fsp->file_id, &file_id_strbuf), + nt_errstr(status)); + smb_panic("leases_db_get() failed"); + } } if (!state->got_handle_lease && -- 2.17.1