[PATCH] Make Durable-Handle reconnect more robust
Ralph Böhme
slow at samba.org
Thu Aug 30 14:37:06 UTC 2018
Hi!
Attached is the fix for bug 13549 which fixes a problem where Durable Handle
reconnects fail as old and new session are not properly doing a rendezvous.
The new session is supposed to rendezvous with and wait for destruction of the
old session, which is internally implemented with dbwrap_watch_send() on the old
session record.
If the old session deletes the session record before calling file_close_user()
which marks all file handles as disconnected, the durable handle reconnect in
the new session will fail as the records are not yet marked as disconnected
which is a prerequisite.
I saw this race getting triggerd on a test cluster with GlusterFS where the
disconnect got stuck in utimes() for 800 ms so the reconnect overtook the
disconnect.
Please review&push if happy. Thanks!
-slow
--
Ralph Boehme, Samba Team https://samba.org/
Samba Developer, SerNet GmbH https://sernet.de/en/samba/
GPG Key Fingerprint: FAE2 C608 8A24 2520 51C5
59E4 AA1E 9B71 2639 9E46
-------------- next part --------------
From 99121a72a0769f674fc32db72a048dd9cebf034e Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 30 Aug 2018 15:50:02 +0200
Subject: [PATCH 1/3] s3:smbd: reorder tcon global record deletion and closing
files of a tcon
As such, this doesn't change overall behaviour, but in case we ever add
semantics acting on tcon record changes via an API like
dbwrap_watch_send(), this will make a difference as it enforces
ordering.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=13549
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smbXsrv_tcon.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/source3/smbd/smbXsrv_tcon.c b/source3/smbd/smbXsrv_tcon.c
index eb483937d63..ded4010f5da 100644
--- a/source3/smbd/smbXsrv_tcon.c
+++ b/source3/smbd/smbXsrv_tcon.c
@@ -904,6 +904,25 @@ NTSTATUS smbXsrv_tcon_disconnect(struct smbXsrv_tcon *tcon, uint64_t vuid)
table = tcon->table;
tcon->table = NULL;
+ if (tcon->compat) {
+ bool ok;
+
+ ok = chdir_current_service(tcon->compat);
+ if (!ok) {
+ status = NT_STATUS_INTERNAL_ERROR;
+ DEBUG(0, ("smbXsrv_tcon_disconnect(0x%08x, '%s'): "
+ "chdir_current_service() failed: %s\n",
+ tcon->global->tcon_global_id,
+ tcon->global->share_name,
+ nt_errstr(status)));
+ tcon->compat = NULL;
+ return status;
+ }
+
+ close_cnum(tcon->compat, vuid);
+ tcon->compat = NULL;
+ }
+
tcon->status = NT_STATUS_NETWORK_NAME_DELETED;
global_rec = tcon->global->db_rec;
@@ -966,25 +985,6 @@ NTSTATUS smbXsrv_tcon_disconnect(struct smbXsrv_tcon *tcon, uint64_t vuid)
}
tcon->db_rec = NULL;
- if (tcon->compat) {
- bool ok;
-
- ok = chdir_current_service(tcon->compat);
- if (!ok) {
- status = NT_STATUS_INTERNAL_ERROR;
- DEBUG(0, ("smbXsrv_tcon_disconnect(0x%08x, '%s'): "
- "chdir_current_service() failed: %s\n",
- tcon->global->tcon_global_id,
- tcon->global->share_name,
- nt_errstr(status)));
- tcon->compat = NULL;
- return status;
- }
-
- close_cnum(tcon->compat, vuid);
- tcon->compat = NULL;
- }
-
return error;
}
--
2.13.6
From cb6130fca27a8614e61cea4a69ccae054d2957fb Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 29 Aug 2018 17:19:29 +0200
Subject: [PATCH 2/3] s3:smbd: let session logoff close files and tcons before
deleting the session
This avoids a race in durable handle reconnects if the reconnect comes
in while the old session is still in the tear-down phase.
The new session is supposed to rendezvous with and wait for destruction
of the old session, which is internally implemented with
dbwrap_watch_send() on the old session record.
If the old session deletes the session record before calling
file_close_user() which marks all file handles as disconnected, the
durable handle reconnect in the new session will fail as the records are
not yet marked as disconnected which is a prerequisite.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=13549
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smbXsrv_session.c | 46 +++++++++++++++++++++---------------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
index af47f4aa8c3..1193b4a3bea 100644
--- a/source3/smbd/smbXsrv_session.c
+++ b/source3/smbd/smbXsrv_session.c
@@ -1663,6 +1663,29 @@ NTSTATUS smbXsrv_session_logoff(struct smbXsrv_session *session)
session->client = NULL;
session->status = NT_STATUS_USER_SESSION_DELETED;
+ if (session->compat) {
+ file_close_user(sconn, session->compat->vuid);
+ }
+
+ if (session->tcon_table != NULL) {
+ /*
+ * Note: We only have a tcon_table for SMB2.
+ */
+ status = smb2srv_tcon_disconnect_all(session);
+ if (!NT_STATUS_IS_OK(status)) {
+ DEBUG(0, ("smbXsrv_session_logoff(0x%08x): "
+ "smb2srv_tcon_disconnect_all() failed: %s\n",
+ session->global->session_global_id,
+ nt_errstr(status)));
+ error = status;
+ }
+ }
+
+ if (session->compat) {
+ invalidate_vuid(sconn, session->compat->vuid);
+ session->compat = NULL;
+ }
+
global_rec = session->global->db_rec;
session->global->db_rec = NULL;
if (global_rec == NULL) {
@@ -1722,29 +1745,6 @@ NTSTATUS smbXsrv_session_logoff(struct smbXsrv_session *session)
}
session->db_rec = NULL;
- if (session->compat) {
- file_close_user(sconn, session->compat->vuid);
- }
-
- if (session->tcon_table != NULL) {
- /*
- * Note: We only have a tcon_table for SMB2.
- */
- status = smb2srv_tcon_disconnect_all(session);
- if (!NT_STATUS_IS_OK(status)) {
- DEBUG(0, ("smbXsrv_session_logoff(0x%08x): "
- "smb2srv_tcon_disconnect_all() failed: %s\n",
- session->global->session_global_id,
- nt_errstr(status)));
- error = status;
- }
- }
-
- if (session->compat) {
- invalidate_vuid(sconn, session->compat->vuid);
- session->compat = NULL;
- }
-
return error;
}
--
2.13.6
From 87e932815e10dba5fecd540199cb4ad85dd2fe7b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 30 Aug 2018 15:57:33 +0200
Subject: [PATCH 3/3] s3:smbd: add a comment stating that file_close_user() is
redundant for SMB2
Bug: https://bugzilla.samba.org/show_bug.cgi?id=13549
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smbXsrv_session.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
index 1193b4a3bea..19f4bc05600 100644
--- a/source3/smbd/smbXsrv_session.c
+++ b/source3/smbd/smbXsrv_session.c
@@ -1664,6 +1664,12 @@ NTSTATUS smbXsrv_session_logoff(struct smbXsrv_session *session)
session->status = NT_STATUS_USER_SESSION_DELETED;
if (session->compat) {
+ /*
+ * For SMB2 this is a bit redundant as files are also close
+ * below via smb2srv_tcon_disconnect_all() -> ... ->
+ * smbXsrv_tcon_disconnect() -> close_cnum() ->
+ * file_close_conn().
+ */
file_close_user(sconn, session->compat->vuid);
}
--
2.13.6
More information about the samba-technical
mailing list