12976 system panics with error in IP module

Reviewed by: Andy Fiddaman <andy@omniosce.org>
Reviewed by: Paul Winder <p.winder@me.com>
Approved by: Robert Mustacchi <rm@fingolfin.org>

4 files changed, 67 insertions, 12 deletions

diff --git a/usr/src/uts/common/inet/ip/ipclassifier.c b/usr/src/uts/common/inet/ip/ipclassifier.c
index 09cafcb4e7..26cc629cca 100644
--- a/usr/src/uts/common/inet/ip/ipclassifier.c
+++ b/usr/src/uts/common/inet/ip/ipclassifier.c
@@ -21,6 +21,7 @@
 /*
  * Copyright (c) 2003, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2019 OmniOS Community Edition (OmniOSce) Association.
+ * Copyright 2020 Joyent, Inc.
  */
 
 /*
@@ -2745,7 +2746,11 @@ conn_get_socket_info(conn_t *connp, mib2_socketInfoEntry_t *sie)
 		return (NULL);
 	}
 
-	mutex_exit(&connp->conn_lock);
+	/*
+	 * Continue to hold conn_lock because we don't want to race with an
+	 * in-progress close, which will have set-to-NULL (and destroyed
+	 * upper_handle, aka sonode (and vnode)) BEFORE setting CONN_CLOSING.
+	 */
 
 	if (connp->conn_upper_handle != NULL) {
 		vn = (*connp->conn_upcalls->su_get_vnode)
@@ -2757,6 +2762,8 @@ conn_get_socket_info(conn_t *connp, mib2_socketInfoEntry_t *sie)
 		flags |= MIB2_SOCKINFO_STREAM;
 	}
 
+	mutex_exit(&connp->conn_lock);
+
 	if (vn == NULL || VOP_GETATTR(vn, &attr, 0, CRED(), NULL) != 0) {
 		if (vn != NULL)
 			VN_RELE(vn);
diff --git a/usr/src/uts/common/inet/tcp/tcp.c b/usr/src/uts/common/inet/tcp/tcp.c
index eef7ded43a..6cae350878 100644
--- a/usr/src/uts/common/inet/tcp/tcp.c
+++ b/usr/src/uts/common/inet/tcp/tcp.c
@@ -21,10 +21,10 @@
 
 /*
  * Copyright (c) 1991, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright 2019 Joyent, Inc.
  * Copyright (c) 2011 Nexenta Systems, Inc. All rights reserved.
  * Copyright (c) 2013, 2017 by Delphix. All rights reserved.
  * Copyright 2014, OmniTI Computer Consulting, Inc. All rights reserved.
+ * Copyright 2020 Joyent, Inc.
  */
 /* Copyright (c) 1990 Mentat Inc. */
 
@@ -1019,10 +1019,23 @@ finish:
 
 	/* If we have an upper handle (socket), release it */
 	if (IPCL_IS_NONSTR(connp)) {
-		ASSERT(connp->conn_upper_handle != NULL);
-		(*connp->conn_upcalls->su_closed)(connp->conn_upper_handle);
+		sock_upcalls_t *upcalls = connp->conn_upcalls;
+		sock_upper_handle_t handle = connp->conn_upper_handle;
+
+		ASSERT(upcalls != NULL);
+		ASSERT(upcalls->su_closed != NULL);
+		ASSERT(handle != NULL);
+		/*
+		 * Set these to NULL first because closed() will free upper
+		 * structures.  Acquire conn_lock because an external caller
+		 * like conn_get_socket_info() will upcall if these are
+		 * non-NULL.
+		 */
+		mutex_enter(&connp->conn_lock);
 		connp->conn_upper_handle = NULL;
 		connp->conn_upcalls = NULL;
+		mutex_exit(&connp->conn_lock);
+		upcalls->su_closed(handle);
 	}
 }
 
@@ -1421,13 +1434,26 @@ tcp_free(tcp_t *tcp)
 	 * nothing to do other than clearing the field.
 	 */
 	if (connp->conn_upper_handle != NULL) {
+		sock_upcalls_t *upcalls = connp->conn_upcalls;
+		sock_upper_handle_t handle = connp->conn_upper_handle;
+
+		/*
+		 * Set these to NULL first because closed() will free upper
+		 * structures.  Acquire conn_lock because an external caller
+		 * like conn_get_socket_info() will upcall if these are
+		 * non-NULL.
+		 */
+		mutex_enter(&connp->conn_lock);
+		connp->conn_upper_handle = NULL;
+		connp->conn_upcalls = NULL;
+		mutex_exit(&connp->conn_lock);
 		if (IPCL_IS_NONSTR(connp)) {
-			(*connp->conn_upcalls->su_closed)(
-			    connp->conn_upper_handle);
+			ASSERT(upcalls != NULL);
+			ASSERT(upcalls->su_closed != NULL);
+			ASSERT(handle != NULL);
+			upcalls->su_closed(handle);
 			tcp->tcp_detached = B_TRUE;
 		}
-		connp->conn_upper_handle = NULL;
-		connp->conn_upcalls = NULL;
 	}
 }
 
diff --git a/usr/src/uts/common/inet/tcp/tcp_output.c b/usr/src/uts/common/inet/tcp/tcp_output.c
index 7a0472f3dd..086668f435 100644
--- a/usr/src/uts/common/inet/tcp/tcp_output.c
+++ b/usr/src/uts/common/inet/tcp/tcp_output.c
@@ -22,7 +22,7 @@
 /*
  * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2014, 2017 by Delphix. All rights reserved.
- * Copyright 2019 Joyent, Inc.
+ * Copyright 2020 Joyent, Inc.
  */
 
 /* This file contains all TCP output processing functions. */
@@ -1677,11 +1677,23 @@ finish:
 
 		/* non-STREAM socket, release the upper handle */
 		if (IPCL_IS_NONSTR(connp)) {
-			ASSERT(connp->conn_upper_handle != NULL);
-			(*connp->conn_upcalls->su_closed)
-			    (connp->conn_upper_handle);
+			sock_upcalls_t *upcalls = connp->conn_upcalls;
+			sock_upper_handle_t handle = connp->conn_upper_handle;
+
+			ASSERT(upcalls != NULL);
+			ASSERT(upcalls->su_closed != NULL);
+			ASSERT(handle != NULL);
+			/*
+			 * Set these to NULL first because closed() will free
+			 * upper structures.  Acquire conn_lock because an
+			 * external caller like conn_get_socket_info() will
+			 * upcall if these are non-NULL.
+			 */
+			mutex_enter(&connp->conn_lock);
 			connp->conn_upper_handle = NULL;
 			connp->conn_upcalls = NULL;
+			mutex_exit(&connp->conn_lock);
+			upcalls->su_closed(handle);
 		}
 	}
 
diff --git a/usr/src/uts/common/sys/socket_proto.h b/usr/src/uts/common/sys/socket_proto.h
index 4e1a4a0f35..825d0501c7 100644
--- a/usr/src/uts/common/sys/socket_proto.h
+++ b/usr/src/uts/common/sys/socket_proto.h
@@ -21,6 +21,7 @@
 /*
  * Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2019 OmniOS Community Edition (OmniOSce) Association.
+ * Copyright 2020 Joyent, Inc.
  */
 
 #ifndef _SYS_SOCKET_PROTO_H_
@@ -202,7 +203,16 @@ struct sock_upcalls_s {
 	void	(*su_signal_oob)(sock_upper_handle_t, ssize_t);
 	void	(*su_zcopy_notify)(sock_upper_handle_t);
 	void	(*su_set_error)(sock_upper_handle_t, int);
+	/*
+	 * NOTE: This function frees upper handle items. Caller cannot
+	 * rely on them after this upcall.
+	 */
 	void	(*su_closed)(sock_upper_handle_t);
+	/*
+	 * NOTE: This function MUST be implemented without using lower-level
+	 * downcalls or accesses. This allows callers to ensure su_closed()
+	 * upcalls can happen indepdently or concurrently.
+	 */
 	vnode_t *(*su_get_vnode)(sock_upper_handle_t);
 };