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);
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to