svn commit: r307551 - head/sys/netinet

[Julien Charbon]

Author: jch
Date: Tue Oct 18 07:16:49 2016
New Revision: 307551
URL: https://svnweb.freebsd.org/changeset/base/307551

Log:
  Fix a double-free when an inp transitions to INP_TIMEWAIT state
  after having been dropped.
  
  This fixes enforces in_pcbdrop() logic in tcp_input():
  
  "in_pcbdrop() is used by TCP to mark an inpcb as unused and avoid future packet
  delivery or event notification when a socket remains open but TCP has closed."
  
  PR:			203175
  Reported by:		Palle Girgensohn, Slawa Olhovchenkov
  Tested by:		Slawa Olhovchenkov
  Reviewed by:		Slawa Olhovchenkov
  Approved by:		gnn, Slawa Olhovchenkov
  Differential Revision:	https://reviews.freebsd.org/D8211
  MFC after:		1 week
  Sponsored by:		Verisign, inc

Modified:
  head/sys/netinet/tcp_input.c
  head/sys/netinet/tcp_timewait.c
  head/sys/netinet/tcp_usrreq.c

Modified: head/sys/netinet/tcp_input.c
==============================================================================
--- head/sys/netinet/tcp_input.c	Tue Oct 18 05:43:12 2016	(r307550)
+++ head/sys/netinet/tcp_input.c	Tue Oct 18 07:16:49 2016	(r307551)
@@ -920,6 +920,16 @@ findpcb:
 		goto dropwithreset;
 	}
 	INP_WLOCK_ASSERT(inp);
+	/*
+	 * While waiting for inp lock during the lookup, another thread
+	 * can have dropped the inpcb, in which case we need to loop back
+	 * and try to find a new inpcb to deliver to.
+	 */
+	if (inp->inp_flags & INP_DROPPED) {
+		INP_WUNLOCK(inp);
+		inp = NULL;
+		goto findpcb;
+	}
 	if ((inp->inp_flowtype == M_HASHTYPE_NONE) &&
 	    (M_HASHTYPE_GET(m) != M_HASHTYPE_NONE) &&
 	    ((inp->inp_socket == NULL) ||
@@ -980,6 +990,10 @@ relocked:
 				if (in_pcbrele_wlocked(inp)) {
 					inp = NULL;
 					goto findpcb;
+				} else if (inp->inp_flags & INP_DROPPED) {
+					INP_WUNLOCK(inp);
+					inp = NULL;
+					goto findpcb;
 				}
 			} else
 				ti_locked = TI_RLOCKED;
@@ -1039,6 +1053,10 @@ relocked:
 				if (in_pcbrele_wlocked(inp)) {
 					inp = NULL;
 					goto findpcb;
+				} else if (inp->inp_flags & INP_DROPPED) {
+					INP_WUNLOCK(inp);
+					inp = NULL;
+					goto findpcb;
 				}
 				goto relocked;
 			} else

Modified: head/sys/netinet/tcp_timewait.c
==============================================================================
--- head/sys/netinet/tcp_timewait.c	Tue Oct 18 05:43:12 2016	(r307550)
+++ head/sys/netinet/tcp_timewait.c	Tue Oct 18 07:16:49 2016	(r307551)
@@ -231,6 +231,10 @@ tcp_twstart(struct tcpcb *tp)
 	INP_INFO_RLOCK_ASSERT(&V_tcbinfo);
 	INP_WLOCK_ASSERT(inp);
 
+	/* A dropped inp should never transition to TIME_WAIT state. */
+	KASSERT((inp->inp_flags & INP_DROPPED) == 0, ("tcp_twstart: "
+	    "(inp->inp_flags & INP_DROPPED) != 0"));
+
 	if (V_nolocaltimewait) {
 		int error = 0;
 #ifdef INET6

Modified: head/sys/netinet/tcp_usrreq.c
==============================================================================
--- head/sys/netinet/tcp_usrreq.c	Tue Oct 18 05:43:12 2016	(r307550)
+++ head/sys/netinet/tcp_usrreq.c	Tue Oct 18 07:16:49 2016	(r307551)
@@ -59,6 +59,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/protosw.h>
 #include <sys/proc.h>
 #include <sys/jail.h>
+#include <sys/syslog.h>
 
 #ifdef DDB
 #include <ddb/ddb.h>
@@ -210,10 +211,26 @@ tcp_detach(struct socket *so, struct inp
 		 *  In all three cases the tcptw should not be freed here.
 		 */
 		if (inp->inp_flags & INP_DROPPED) {
-			KASSERT(tp == NULL, ("tcp_detach: INP_TIMEWAIT && "
-			    "INP_DROPPED && tp != NULL"));
 			in_pcbdetach(inp);
-			in_pcbfree(inp);
+			if (__predict_true(tp == NULL)) {
+				in_pcbfree(inp);
+			} else {
+				/*
+				 * This case should not happen as in TIMEWAIT
+				 * state the inp should not be destroyed before
+				 * its tcptw.  If INVARIANTS is defined, panic.
+				 */
+#ifdef INVARIANTS
+				panic("%s: Panic before an inp double-free: "
+				    "INP_TIMEWAIT && INP_DROPPED && tp != NULL"
+				    , __func__);
+#else
+				log(LOG_ERR, "%s: Avoid an inp double-free: "
+				    "INP_TIMEWAIT && INP_DROPPED && tp != NULL"
+				    , __func__);
+#endif
+				INP_WUNLOCK(inp);
+			}
 		} else {
 			in_pcbdetach(inp);
 			INP_WUNLOCK(inp);