11832 smb/server is stuck in STOPPING state after svcadm restart

Reviewed by: Matt Barden <matt.barden@nexenta.com>
Reviewed by: Evan Layton <evan.layton@nexenta.com>
Reviewed by: Andy Stormont <AStormont@racktopsystems.com>
Approved by: Robert Mustacchi <rm@fingolfin.org>

5 files changed, 70 insertions, 8 deletions

diff --git a/usr/src/uts/common/fs/smbsrv/smb_ofile.c b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
index 531ca314fb..d5388037c3 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_ofile.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
@@ -22,7 +22,7 @@
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2016 Syneto S.R.L. All rights reserved.
  * Copyright (c) 2016 by Delphix. All rights reserved.
- * Copyright 2018 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2019 Nexenta Systems, Inc.  All rights reserved.
  */
 
 /*
@@ -954,6 +954,10 @@ smb_ofile_lookup_by_uniqid(smb_tree_t *tree, uint32_t uniqid)
 	return (NULL);
 }
 
+/*
+ * Durable ID (or persistent ID)
+ */
+
 static smb_ofile_t *
 smb_ofile_hold_cb(smb_ofile_t *of)
 {
@@ -1510,6 +1514,9 @@ smb_ofile_free(smb_ofile_t *of)
 
 	ASSERT(of->f_state == SMB_OFILE_STATE_ALLOC);
 
+	/* Make sure it's not in the persistid hash. */
+	ASSERT(of->f_persistid == 0);
+
 	if (tree != NULL) {
 		if (of->f_fid != 0)
 			smb_idpool_free(&tree->t_fid_pool, of->f_fid);
diff --git a/usr/src/uts/common/fs/smbsrv/smb_session.c b/usr/src/uts/common/fs/smbsrv/smb_session.c
index 2878df28e7..17bbc16e72 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_session.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_session.c
@@ -47,6 +47,16 @@ static volatile uint64_t smb_kids;
 uint32_t smb_keep_alive = SMB_PI_KEEP_ALIVE_MIN / 60;
 
 /*
+ * This is the maximum time we'll allow a "session" to exist with no
+ * authenticated smb_user_t objects on it.  This allows a client to
+ * logoff their "one and only" user session and then logon as some
+ * different user.  (There are some tests that do that.)  The same
+ * timeout mechanism also reduces the impact of clients that might
+ * open TCP connections but never authenticate.
+ */
+int smb_session_auth_tmo = 30; /* sec. */
+
+/*
  * There are many smbtorture test cases that send
  * racing requests, and where the tests fail if we
  * don't execute them in exactly the order sent.
@@ -482,6 +492,7 @@ void
 smb_session_receiver(smb_session_t *session)
 {
 	int	rc = 0;
+	timeout_id_t tmo = NULL;
 
 	SMB_SESSION_VALID(session);
 
@@ -501,6 +512,8 @@ smb_session_receiver(smb_session_t *session)
 
 	smb_rwx_rwenter(&session->s_lock, RW_WRITER);
 	session->s_state = SMB_SESSION_STATE_ESTABLISHED;
+	session->s_auth_tmo = timeout((tmo_func_t)smb_session_disconnect,
+	    session, SEC_TO_TICK(smb_session_auth_tmo));
 	smb_rwx_rwexit(&session->s_lock);
 
 	(void) smb_session_reader(session);
@@ -508,8 +521,14 @@ smb_session_receiver(smb_session_t *session)
 	smb_rwx_rwenter(&session->s_lock, RW_WRITER);
 	if (session->s_state != SMB_SESSION_STATE_TERMINATED)
 		session->s_state = SMB_SESSION_STATE_DISCONNECTED;
+	tmo = session->s_auth_tmo;
+	session->s_auth_tmo = NULL;
 	smb_rwx_rwexit(&session->s_lock);
 
+	/* Timeout callback takes s_lock. See untimeout(9f) */
+	if (tmo != NULL)
+		(void) untimeout(tmo);
+
 	smb_soshutdown(session->sock);
 
 	DTRACE_PROBE2(session__drop, struct session *, session, int, rc);
@@ -729,7 +748,6 @@ smb_session_create(ksocket_t new_so, uint16_t port, smb_server_t *sv,
 	session->opentime = now;
 	session->keep_alive = smb_keep_alive;
 	session->activity_timestamp = now;
-
 	smb_session_genkey(session);
 
 	mutex_init(&session->s_credits_mutex, NULL, MUTEX_DEFAULT, NULL);
@@ -882,9 +900,7 @@ smb_session_cancel(smb_session_t *session)
 	smb_slist_wait_for_empty(&session->s_req_list);
 
 	/*
-	 * At this point the reference count of the users, trees, files,
-	 * directories should be zero. It should be possible to destroy them
-	 * without any problem.
+	 * Cleanup transact state objects
 	 */
 	xa = smb_llist_head(&session->s_xa_list);
 	while (xa) {
@@ -893,6 +909,11 @@ smb_session_cancel(smb_session_t *session)
 		xa = nextxa;
 	}
 
+	/*
+	 * At this point the reference count of the files and directories
+	 * should be zero. It should be possible to destroy them without
+	 * any problem, which should trigger the destruction of other objects.
+	 */
 	smb_session_logoff(session);
 }
 
@@ -1210,8 +1231,8 @@ top:
 			break;
 
 		default:
-			ASSERT(0);
 			mutex_exit(&user->u_mutex);
+			ASSERT(0);
 			break;
 		}
 
diff --git a/usr/src/uts/common/fs/smbsrv/smb_user.c b/usr/src/uts/common/fs/smbsrv/smb_user.c
index 74bb502c56..8c69e95a56 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_user.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_user.c
@@ -234,6 +234,7 @@ smb_user_new(smb_session_t *session)
 {
 	smb_user_t	*user;
 	uint_t		gen;	// generation (low 3 bits of ssnid)
+	uint32_t	ucount;
 
 	ASSERT(session);
 	ASSERT(session->s_magic == SMB_SESSION_MAGIC);
@@ -256,10 +257,27 @@ smb_user_new(smb_session_t *session)
 	user->u_magic = SMB_USER_MAGIC;
 
 	smb_llist_enter(&session->s_user_list, RW_WRITER);
+	ucount = smb_llist_get_count(&session->s_user_list);
 	smb_llist_insert_tail(&session->s_user_list, user);
 	smb_llist_exit(&session->s_user_list);
 	smb_server_inc_users(session->s_server);
 
+	/*
+	 * If we added the first user to the session, cancel the
+	 * timeout that was started in smb_session_receiver().
+	 */
+	if (ucount == 0) {
+		timeout_id_t tmo = NULL;
+
+		smb_rwx_rwenter(&session->s_lock, RW_WRITER);
+		tmo = session->s_auth_tmo;
+		session->s_auth_tmo = NULL;
+		smb_rwx_rwexit(&session->s_lock);
+
+		if (tmo != NULL)
+			(void) untimeout(tmo);
+	}
+
 	return (user);
 
 errout:
@@ -672,9 +690,19 @@ smb_user_delete(void *arg)
 	ucount = smb_llist_get_count(&session->s_user_list);
 	smb_llist_exit(&session->s_user_list);
 
+	/*
+	 * When the last smb_user_t object goes away, schedule a timeout
+	 * after which we'll terminate this session if the client hasn't
+	 * authenticated another smb_user_t on this session by then.
+	 */
 	if (ucount == 0) {
 		smb_rwx_rwenter(&session->s_lock, RW_WRITER);
-		session->s_state = SMB_SESSION_STATE_SHUTDOWN;
+		if (session->s_state == SMB_SESSION_STATE_NEGOTIATED &&
+		    session->s_auth_tmo == NULL) {
+			session->s_auth_tmo =
+			    timeout((tmo_func_t)smb_session_disconnect,
+			    session, SEC_TO_TICK(smb_session_auth_tmo));
+		}
 		smb_rwx_cvbcast(&session->s_lock);
 		smb_rwx_rwexit(&session->s_lock);
 	}
diff --git a/usr/src/uts/common/smbsrv/smb_kproto.h b/usr/src/uts/common/smbsrv/smb_kproto.h
index aeb780dc35..504cfc1ade 100644
--- a/usr/src/uts/common/smbsrv/smb_kproto.h
+++ b/usr/src/uts/common/smbsrv/smb_kproto.h
@@ -22,7 +22,7 @@
 /*
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2016 Syneto S.R.L.  All rights reserved.
- * Copyright 2018 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2019 Nexenta by DDN, Inc. All rights reserved.
  */
 
 /*
@@ -56,6 +56,9 @@ extern "C" {
 #include <smbsrv/smb_ktypes.h>
 #include <smbsrv/smb_ioctl.h>
 
+/* For timeout(9F). Not in any system header, apparently. */
+typedef void (*tmo_func_t)(void *);
+
 extern	int smb_maxbufsize;
 extern	int smb_flush_required;
 extern	int smb_dirsymlink_enable;
@@ -69,6 +72,8 @@ extern	int smb_ssetup_timeout;
 extern	int smb_tcon_timeout;
 extern	int smb_opipe_timeout;
 extern	int smb_allow_advisory_locks;
+extern	int smb_session_auth_tmo;
+
 extern const uint32_t smb_vop_dosattr_settable;
 
 /* Thread priorities - see smb_init.c */
diff --git a/usr/src/uts/common/smbsrv/smb_ktypes.h b/usr/src/uts/common/smbsrv/smb_ktypes.h
index 1f8ce704fb..20d214a523 100644
--- a/usr/src/uts/common/smbsrv/smb_ktypes.h
+++ b/usr/src/uts/common/smbsrv/smb_ktypes.h
@@ -964,6 +964,7 @@ typedef struct smb_session {
 	uint32_t		challenge_len;
 	unsigned char		challenge_key[SMB_CHALLENGE_SZ];
 	int64_t			activity_timestamp;
+	timeout_id_t		s_auth_tmo;
 
 	/*
 	 * Maximum negotiated buffer sizes between SMB client and server