From acb5bfda99b753d9ead3529d04f20087c5f7d0a0 Mon Sep 17 00:00:00 2001 From: jch Date: Tue, 25 Oct 2016 12:53:14 +0000 Subject: [PATCH] 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 --- sys/netinet/tcp_input.c | 18 ++++++++++++++++++ sys/netinet/tcp_timewait.c | 4 ++++ sys/netinet/tcp_usrreq.c | 23 ++++++++++++++++++++--- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c index d1caa711525059..1956e59562111d 100644 --- a/sys/netinet/tcp_input.c +++ b/sys/netinet/tcp_input.c @@ -914,6 +914,16 @@ tcp_input(struct mbuf **mp, int *offp, int proto) 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 @@ tcp_input(struct mbuf **mp, int *offp, int proto) 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 @@ tcp_input(struct mbuf **mp, int *offp, int proto) 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 diff --git a/sys/netinet/tcp_timewait.c b/sys/netinet/tcp_timewait.c index a8b78f9379d4d2..27b965ea2655cc 100644 --- a/sys/netinet/tcp_timewait.c +++ b/sys/netinet/tcp_timewait.c @@ -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 diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c index 204c354ccfb2d5..e86481a3f6b585 100644 --- a/sys/netinet/tcp_usrreq.c +++ b/sys/netinet/tcp_usrreq.c @@ -59,6 +59,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #ifdef DDB #include @@ -210,10 +211,26 @@ tcp_detach(struct socket *so, struct inpcb *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);