13515 panic with bad mutex in smb_ofile_hold_olbrk

Portions contributed by: Matt Barden <mbarden@tintri.com>
Reviewed by: Evan Layton <elayton@tintri.com>
Reviewed by: Rick McNeal <rmcneal@tintri.com>
Reviewed by: Paul Winder <paul@winder.uk.net>
Approved by: Robert Mustacchi <rm@fingolfin.org>

5 files changed, 78 insertions, 34 deletions

diff --git a/usr/src/cmd/smbsrv/testoplock/smbsrv/smb_ktypes.h b/usr/src/cmd/smbsrv/testoplock/smbsrv/smb_ktypes.h
index 16c770ef1d..1347a8e995 100644
--- a/usr/src/cmd/smbsrv/testoplock/smbsrv/smb_ktypes.h
+++ b/usr/src/cmd/smbsrv/testoplock/smbsrv/smb_ktypes.h
@@ -20,7 +20,7 @@
  */
 /*
  * Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright 2018 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2020 Nexenta by DDN, Inc.  All rights reserved.
  */
 
 /*
@@ -98,6 +98,7 @@ typedef struct smb_oplock_grant {
 	uint32_t		og_state;	/* latest sent to client */
 	uint32_t		og_breaking;	/* BREAK_TO... flags */
 	uint16_t		og_dialect;	/* how to send breaks */
+	boolean_t		og_closing;
 	/* File-system level state */
 	uint8_t			onlist_II;
 	uint8_t			onlist_R;
diff --git a/usr/src/uts/common/fs/smbsrv/smb2_lease.c b/usr/src/uts/common/fs/smbsrv/smb2_lease.c
index 95d7d9c7f1..b4d0aef349 100644
--- a/usr/src/uts/common/fs/smbsrv/smb2_lease.c
+++ b/usr/src/uts/common/fs/smbsrv/smb2_lease.c
@@ -10,7 +10,7 @@
  */
 
 /*
- * Copyright 2017 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2020 Nexenta by DDN, Inc.  All rights reserved.
  */
 
 /*
@@ -653,7 +653,6 @@ done:
 		ofile->f_oplock.og_state = op->op_oplock_state;
 		mutex_enter(&lease->ls_mutex);
 		lease->ls_state = op->op_oplock_state & CACHE_RWH;
-		lease->ls_oplock_ofile = ofile;
 		lease->ls_epoch++;
 		mutex_exit(&lease->ls_mutex);
 	}
@@ -685,6 +684,9 @@ smb2_lease_ofile_close(smb_ofile_t *ofile)
 	smb_lease_t *lease = ofile->f_lease;
 	smb_ofile_t *o;
 
+	ASSERT(RW_READ_HELD(&node->n_ofile_list.ll_lock));
+	ASSERT(MUTEX_HELD(&node->n_oplock.ol_mutex));
+
 	/*
 	 * If this ofile was not the oplock owner for this lease,
 	 * we can leave things as they are.
@@ -696,24 +698,22 @@ smb2_lease_ofile_close(smb_ofile_t *ofile)
 	 * Find another ofile to which we can move the oplock.
 	 * The ofile must be open and allow a new ref.
 	 */
-	smb_llist_enter(&node->n_ofile_list, RW_READER);
 	FOREACH_NODE_OFILE(node, o) {
 		if (o == ofile)
 			continue;
 		if (o->f_lease != lease)
 			continue;
+		if (o->f_oplock.og_closing)
+			continue;
 		/* If we can get a hold, use this ofile. */
-		if (smb_ofile_hold(o))
+		if (smb_ofile_hold_olbrk(o))
 			break;
 	}
 	if (o == NULL) {
 		/* Normal for last close on a lease. */
-		smb_llist_exit(&node->n_ofile_list);
 		return;
 	}
 	smb_oplock_move(node, ofile, o);
-	lease->ls_oplock_ofile = o;
 
-	smb_llist_exit(&node->n_ofile_list);
 	smb_ofile_release(o);
 }
diff --git a/usr/src/uts/common/fs/smbsrv/smb_cmn_oplock.c b/usr/src/uts/common/fs/smbsrv/smb_cmn_oplock.c
index 39d67dd824..8ec21f5f37 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_cmn_oplock.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_cmn_oplock.c
@@ -10,7 +10,7 @@
  */
 
 /*
- * Copyright 2017 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2020 Nexenta by DDN, Inc.  All rights reserved.
  */
 
 /*
@@ -495,9 +495,20 @@ smb_oplock_request(smb_request_t *sr, smb_ofile_t *ofile, uint32_t *statep)
 	}
 
 	/* Give caller back the "Granular" bit. */
-	if (status == NT_STATUS_SUCCESS)
+	if (status == NT_STATUS_SUCCESS) {
 		*statep |= LEVEL_GRANULAR;
 
+		/*
+		 * The oplock lease may have moved to this ofile. Update.
+		 * Minor violation of layering here (leases vs oplocks)
+		 * but we want this update coverd by the oplock mutex.
+		 */
+#ifndef	TESTJIG
+		if (ofile->f_lease != NULL)
+			ofile->f_lease->ls_oplock_ofile = ofile;
+#endif
+	}
+
 out:
 	mutex_exit(&node->n_oplock.ol_mutex);
 	smb_llist_exit(&node->n_ofile_list);
@@ -545,6 +556,12 @@ smb_oplock_req_excl(
 	ASSERT(MUTEX_HELD(&node->n_oplock.ol_mutex));
 
 	/*
+	 * Don't allow grants on closing ofiles.
+	 */
+	if (ofile->f_oplock.og_closing)
+		return (status);
+
+	/*
 	 * If Open.Stream.Oplock is empty:
 	 *   Build a new Oplock object with fields initialized as follows:
 	 *	Oplock.State set to NO_OPLOCK.
@@ -1030,6 +1047,12 @@ smb_oplock_req_shared(
 	ASSERT(MUTEX_HELD(&node->n_oplock.ol_mutex));
 
 	/*
+	 * Don't allow grants on closing ofiles.
+	 */
+	if (ofile->f_oplock.og_closing)
+		return (status);
+
+	/*
 	 * If Open.Stream.Oplock is empty:
 	 *   Build a new Oplock object with fields initialized as follows:
 	 *	Oplock.State set to NO_OPLOCK.
@@ -2036,6 +2059,20 @@ smb_oplock_ack_break(
 	} /* Switch (oplock.state) */
 
 out:
+	if (status == NT_STATUS_INVALID_OPLOCK_PROTOCOL)
+		*rop = LEVEL_NONE;
+
+	if (status == NT_STATUS_SUCCESS &&
+	    type == LEVEL_GRANULAR &&
+	    *rop != LEVEL_NONE) {
+		*rop |= LEVEL_GRANULAR;
+		/* As above, leased oplock may have moved. */
+#ifndef	TESTJIG
+		if (ofile->f_lease != NULL)
+			ofile->f_lease->ls_oplock_ofile = ofile;
+#endif
+	}
+
 	/*
 	 * The spec. describes waiting for a break here,
 	 * but we let the caller do that (when needed) if
@@ -2044,14 +2081,6 @@ out:
 	mutex_exit(&node->n_oplock.ol_mutex);
 	smb_llist_exit(&node->n_ofile_list);
 
-	if (status == NT_STATUS_INVALID_OPLOCK_PROTOCOL)
-		*rop = LEVEL_NONE;
-
-	if (status == NT_STATUS_SUCCESS &&
-	    type == LEVEL_GRANULAR &&
-	    *rop != LEVEL_NONE)
-		*rop |= LEVEL_GRANULAR;
-
 	return (status);
 }
 
@@ -2257,13 +2286,12 @@ smb_oplock_break_CLOSE(smb_node_t *node, smb_ofile_t *ofile)
 {
 	smb_ofile_t *o;
 
-	if (ofile == NULL) {
-		ASSERT(0);
-		return;
-	}
+	ASSERT(RW_READ_HELD(&node->n_ofile_list.ll_lock));
+	ASSERT(MUTEX_HELD(&node->n_oplock.ol_mutex));
 
-	smb_llist_enter(&node->n_ofile_list, RW_READER);
-	mutex_enter(&node->n_oplock.ol_mutex);
+	if (ofile->f_oplock.og_closing)
+		return;
+	ofile->f_oplock.og_closing = B_TRUE;
 
 	/*
 	 * If Oplock.IIOplocks is not empty:
@@ -2481,8 +2509,6 @@ smb_oplock_break_CLOSE(smb_node_t *node, smb_ofile_t *ofile)
 	if ((node->n_oplock.ol_state & BREAK_ANY) == 0)
 		cv_broadcast(&node->n_oplock.WaitingOpenCV);
 
-	mutex_exit(&node->n_oplock.ol_mutex);
-	smb_llist_exit(&node->n_ofile_list);
 }
 
 /*
@@ -3515,8 +3541,7 @@ smb_oplock_move(smb_node_t *node,
 
 	ASSERT(fr_ofile->f_node == node);
 	ASSERT(to_ofile->f_node == node);
-
-	mutex_enter(&node->n_oplock.ol_mutex);
+	ASSERT(MUTEX_HELD(&node->n_oplock.ol_mutex));
 
 	/*
 	 * The ofile to which we're moving the oplock
@@ -3541,5 +3566,4 @@ smb_oplock_move(smb_node_t *node,
 	if (node->n_oplock.excl_open == fr_ofile)
 		node->n_oplock.excl_open = to_ofile;
 
-	mutex_exit(&node->n_oplock.ol_mutex);
 }
diff --git a/usr/src/uts/common/fs/smbsrv/smb_ofile.c b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
index 6f132cebd3..1d7a5c134f 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_ofile.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
@@ -449,6 +449,20 @@ smb_ofile_close(smb_ofile_t *of, int32_t mtime_sec)
 
 	SMB_OFILE_VALID(of);
 
+	if (of->f_ftype == SMB_FTYPE_DISK) {
+		smb_node_t *node = of->f_node;
+
+		smb_llist_enter(&node->n_ofile_list, RW_READER);
+		mutex_enter(&node->n_oplock.ol_mutex);
+
+		if (of->f_lease != NULL)
+			smb2_lease_ofile_close(of);
+		smb_oplock_break_CLOSE(node, of);
+
+		mutex_exit(&node->n_oplock.ol_mutex);
+		smb_llist_exit(&node->n_ofile_list);
+	}
+
 	mutex_enter(&of->f_mutex);
 	ASSERT(of->f_refcnt);
 
@@ -479,9 +493,6 @@ smb_ofile_close(smb_ofile_t *of, int32_t mtime_sec)
 			smb2_dh_close_persistent(of);
 		if (of->f_persistid != 0)
 			smb_ofile_del_persistid(of);
-		if (of->f_lease != NULL)
-			smb2_lease_ofile_close(of);
-		smb_oplock_break_CLOSE(of->f_node, of);
 		/* FALLTHROUGH */
 
 	case SMB_FTYPE_PRINTER: /* or FTYPE_DISK */
@@ -1442,11 +1453,18 @@ smb_ofile_delete(void *arg)
 	 */
 	if (of->f_ftype == SMB_FTYPE_DISK ||
 	    of->f_ftype == SMB_FTYPE_PRINTER) {
-		ASSERT(of->f_node != NULL);
+		smb_node_t *node = of->f_node;
+
+		/*
+		 * Oplock cleanup should have made sure that
+		 * excl_open does not point to this ofile.
+		 */
+		VERIFY(node->n_oplock.excl_open != of);
+
 		/*
 		 * Note smb_ofile_close did smb_node_dec_open_ofiles()
 		 */
-		smb_node_rem_ofile(of->f_node, of);
+		smb_node_rem_ofile(node, of);
 	}
 
 	/*
diff --git a/usr/src/uts/common/smbsrv/smb_ktypes.h b/usr/src/uts/common/smbsrv/smb_ktypes.h
index 3bdb713337..0a84506726 100644
--- a/usr/src/uts/common/smbsrv/smb_ktypes.h
+++ b/usr/src/uts/common/smbsrv/smb_ktypes.h
@@ -601,6 +601,7 @@ typedef struct smb_oplock_grant {
 	uint32_t		og_state;	/* latest sent to client */
 	uint32_t		og_breaking;	/* BREAK_TO... flags */
 	uint16_t		og_dialect;	/* how to send breaks */
+	boolean_t		og_closing;
 	/* File-system level state */
 	uint8_t			onlist_II;
 	uint8_t			onlist_R;