From owner-svn-src-stable-10@freebsd.org Tue Oct 25 12:58:38 2016 Return-Path: Delivered-To: svn-src-stable-10@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5515DC207F9; Tue, 25 Oct 2016 12:58:38 +0000 (UTC) (envelope-from jch@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 31DDF9A8; Tue, 25 Oct 2016 12:58:38 +0000 (UTC) (envelope-from jch@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u9PCwbOO074725; Tue, 25 Oct 2016 12:58:37 GMT (envelope-from jch@FreeBSD.org) Received: (from jch@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u9PCwbZG074722; Tue, 25 Oct 2016 12:58:37 GMT (envelope-from jch@FreeBSD.org) Message-Id: <201610251258.u9PCwbZG074722@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: jch set sender to jch@FreeBSD.org using -f From: Julien Charbon Date: Tue, 25 Oct 2016 12:58:37 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r307906 - stable/10/sys/netinet X-SVN-Group: stable-10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-10@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for only the 10-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Oct 2016 12:58:38 -0000 Author: jch Date: Tue Oct 25 12:58:36 2016 New Revision: 307906 URL: https://svnweb.freebsd.org/changeset/base/307906 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/10/sys/netinet/tcp_input.c stable/10/sys/netinet/tcp_timewait.c stable/10/sys/netinet/tcp_usrreq.c Modified: stable/10/sys/netinet/tcp_input.c ============================================================================== --- stable/10/sys/netinet/tcp_input.c Tue Oct 25 12:53:14 2016 (r307905) +++ stable/10/sys/netinet/tcp_input.c Tue Oct 25 12:58:36 2016 (r307906) @@ -879,6 +879,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) || @@ -940,6 +950,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_WLOCKED; @@ -999,6 +1013,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/10/sys/netinet/tcp_timewait.c ============================================================================== --- stable/10/sys/netinet/tcp_timewait.c Tue Oct 25 12:53:14 2016 (r307905) +++ stable/10/sys/netinet/tcp_timewait.c Tue Oct 25 12:58:36 2016 (r307906) @@ -230,6 +230,10 @@ tcp_twstart(struct tcpcb *tp) INP_INFO_WLOCK_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/10/sys/netinet/tcp_usrreq.c ============================================================================== --- stable/10/sys/netinet/tcp_usrreq.c Tue Oct 25 12:53:14 2016 (r307905) +++ stable/10/sys/netinet/tcp_usrreq.c Tue Oct 25 12:58:36 2016 (r307906) @@ -58,6 +58,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #ifdef DDB #include @@ -202,10 +203,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);