Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 25 Oct 2016 12:53:15 +0000 (UTC)
From:      Julien Charbon <jch@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r307905 - stable/11/sys/netinet
Message-ID:  <201610251253.u9PCrFdt074268@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: jch
Date: Tue Oct 25 12:53:14 2016
New Revision: 307905
URL: https://svnweb.freebsd.org/changeset/base/307905

Log:
  MFC r307551:
  
  Fix a double-free when an inp transitions to INP_TIMEWAIT state
  after having been dropped.
  
  This change 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
  Sponsored by:		Verisign, inc

Modified:
  stable/11/sys/netinet/tcp_input.c
  stable/11/sys/netinet/tcp_timewait.c
  stable/11/sys/netinet/tcp_usrreq.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/netinet/tcp_input.c
==============================================================================
--- stable/11/sys/netinet/tcp_input.c	Tue Oct 25 10:59:21 2016	(r307904)
+++ stable/11/sys/netinet/tcp_input.c	Tue Oct 25 12:53:14 2016	(r307905)
@@ -914,6 +914,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) ||
@@ -974,6 +984,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;
@@ -1033,6 +1047,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: stable/11/sys/netinet/tcp_timewait.c
==============================================================================
--- stable/11/sys/netinet/tcp_timewait.c	Tue Oct 25 10:59:21 2016	(r307904)
+++ stable/11/sys/netinet/tcp_timewait.c	Tue Oct 25 12:53:14 2016	(r307905)
@@ -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: stable/11/sys/netinet/tcp_usrreq.c
==============================================================================
--- stable/11/sys/netinet/tcp_usrreq.c	Tue Oct 25 10:59:21 2016	(r307904)
+++ stable/11/sys/netinet/tcp_usrreq.c	Tue Oct 25 12:53:14 2016	(r307905)
@@ -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);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201610251253.u9PCrFdt074268>