svn commit: r336596 - in head/sys/netinet: . cc

[Matt Macy]

Author: mmacy
Date: Sun Jul 22 05:37:58 2018
New Revision: 336596
URL: https://svnweb.freebsd.org/changeset/base/336596

Log:
  NULL out cc_data in pluggable TCP {cc}_cb_destroy
  
  When ABE was added (rS331214) to NewReno and leak fixed (rS333699) , it now has
  a destructor (newreno_cb_destroy) for per connection state. Other congestion
  controls may allocate and free cc_data on entry and exit, but the field is
  never explicitly NULLed if moving back to NewReno which only internally
  allocates stateful data (no entry contstructor) resulting in a situation where
  newreno_cb_destory might be called on a junk pointer.
  
   -    NULL out cc_data in the framework after calling {cc}_cb_destroy
   -    free(9) checks for NULL so there is no need to perform not NULL checks
       before calling free.
   -    Improve a comment about NewReno in tcp_ccalgounload
  
  This is the result of a debugging session from Jason Wolfe, Jason Eggleston,
  and mmacy@ and very helpful insight from lstewart at .
  
  Submitted by: Kevin Bowling
  Reviewed by: lstewart
  Sponsored by: Limelight Networks
  Differential Revision: https://reviews.freebsd.org/D16282

Modified:
  head/sys/netinet/cc/cc_chd.c
  head/sys/netinet/cc/cc_cubic.c
  head/sys/netinet/cc/cc_dctcp.c
  head/sys/netinet/cc/cc_htcp.c
  head/sys/netinet/cc/cc_newreno.c
  head/sys/netinet/cc/cc_vegas.c
  head/sys/netinet/tcp_subr.c
  head/sys/netinet/tcp_usrreq.c

Modified: head/sys/netinet/cc/cc_chd.c
==============================================================================
--- head/sys/netinet/cc/cc_chd.c	Sun Jul 22 03:58:01 2018	(r336595)
+++ head/sys/netinet/cc/cc_chd.c	Sun Jul 22 05:37:58 2018	(r336596)
@@ -307,8 +307,7 @@ static void
 chd_cb_destroy(struct cc_var *ccv)
 {
 
-	if (ccv->cc_data != NULL)
-		free(ccv->cc_data, M_CHD);
+	free(ccv->cc_data, M_CHD);
 }
 
 static int

Modified: head/sys/netinet/cc/cc_cubic.c
==============================================================================
--- head/sys/netinet/cc/cc_cubic.c	Sun Jul 22 03:58:01 2018	(r336595)
+++ head/sys/netinet/cc/cc_cubic.c	Sun Jul 22 05:37:58 2018	(r336596)
@@ -213,9 +213,7 @@ cubic_ack_received(struct cc_var *ccv, uint16_t type)
 static void
 cubic_cb_destroy(struct cc_var *ccv)
 {
-
-	if (ccv->cc_data != NULL)
-		free(ccv->cc_data, M_CUBIC);
+	free(ccv->cc_data, M_CUBIC);
 }
 
 static int

Modified: head/sys/netinet/cc/cc_dctcp.c
==============================================================================
--- head/sys/netinet/cc/cc_dctcp.c	Sun Jul 22 03:58:01 2018	(r336595)
+++ head/sys/netinet/cc/cc_dctcp.c	Sun Jul 22 05:37:58 2018	(r336596)
@@ -184,8 +184,7 @@ dctcp_after_idle(struct cc_var *ccv)
 static void
 dctcp_cb_destroy(struct cc_var *ccv)
 {
-	if (ccv->cc_data != NULL)
-		free(ccv->cc_data, M_dctcp);
+	free(ccv->cc_data, M_dctcp);
 }
 
 static int

Modified: head/sys/netinet/cc/cc_htcp.c
==============================================================================
--- head/sys/netinet/cc/cc_htcp.c	Sun Jul 22 03:58:01 2018	(r336595)
+++ head/sys/netinet/cc/cc_htcp.c	Sun Jul 22 05:37:58 2018	(r336596)
@@ -238,9 +238,7 @@ htcp_ack_received(struct cc_var *ccv, uint16_t type)
 static void
 htcp_cb_destroy(struct cc_var *ccv)
 {
-
-	if (ccv->cc_data != NULL)
-		free(ccv->cc_data, M_HTCP);
+	free(ccv->cc_data, M_HTCP);
 }
 
 static int

Modified: head/sys/netinet/cc/cc_newreno.c
==============================================================================
--- head/sys/netinet/cc/cc_newreno.c	Sun Jul 22 03:58:01 2018	(r336595)
+++ head/sys/netinet/cc/cc_newreno.c	Sun Jul 22 05:37:58 2018	(r336596)
@@ -127,9 +127,7 @@ newreno_malloc(struct cc_var *ccv)
 static void
 newreno_cb_destroy(struct cc_var *ccv)
 {
-
-	if (ccv->cc_data != NULL)
-		free(ccv->cc_data, M_NEWRENO);
+	free(ccv->cc_data, M_NEWRENO);
 }
 
 static void

Modified: head/sys/netinet/cc/cc_vegas.c
==============================================================================
--- head/sys/netinet/cc/cc_vegas.c	Sun Jul 22 03:58:01 2018	(r336595)
+++ head/sys/netinet/cc/cc_vegas.c	Sun Jul 22 05:37:58 2018	(r336596)
@@ -170,9 +170,7 @@ vegas_ack_received(struct cc_var *ccv, uint16_t ack_ty
 static void
 vegas_cb_destroy(struct cc_var *ccv)
 {
-
-	if (ccv->cc_data != NULL)
-		free(ccv->cc_data, M_VEGAS);
+	free(ccv->cc_data, M_VEGAS);
 }
 
 static int

Modified: head/sys/netinet/tcp_subr.c
==============================================================================
--- head/sys/netinet/tcp_subr.c	Sun Jul 22 03:58:01 2018	(r336595)
+++ head/sys/netinet/tcp_subr.c	Sun Jul 22 05:37:58 2018	(r336596)
@@ -1730,10 +1730,18 @@ tcp_ccalgounload(struct cc_algo *unload_algo)
 				 */
 				if (CC_ALGO(tp) == unload_algo) {
 					tmpalgo = CC_ALGO(tp);
-					/* NewReno does not require any init. */
-					CC_ALGO(tp) = &newreno_cc_algo;
 					if (tmpalgo->cb_destroy != NULL)
 						tmpalgo->cb_destroy(tp->ccv);
+					CC_DATA(tp) = NULL;
+					/*
+					 * NewReno may allocate memory on
+					 * demand for certain stateful
+					 * configuration as needed, but is
+					 * coded to never fail on memory
+					 * allocation failure so it is a safe
+					 * fallback.
+					 */
+					CC_ALGO(tp) = &newreno_cc_algo;
 				}
 			}
 			INP_WUNLOCK(inp);
@@ -1885,6 +1893,7 @@ tcp_discardcb(struct tcpcb *tp)
 	/* Allow the CC algorithm to clean up after itself. */
 	if (CC_ALGO(tp)->cb_destroy != NULL)
 		CC_ALGO(tp)->cb_destroy(tp->ccv);
+	CC_DATA(tp) = NULL;
 
 #ifdef TCP_HHOOK
 	khelp_destroy_osd(tp->osd);

Modified: head/sys/netinet/tcp_usrreq.c
==============================================================================
--- head/sys/netinet/tcp_usrreq.c	Sun Jul 22 03:58:01 2018	(r336595)
+++ head/sys/netinet/tcp_usrreq.c	Sun Jul 22 05:37:58 2018	(r336596)
@@ -1729,6 +1729,7 @@ unlock_and_done:
 			 */
 			if (CC_ALGO(tp)->cb_destroy != NULL)
 				CC_ALGO(tp)->cb_destroy(tp->ccv);
+			CC_DATA(tp) = NULL;
 			CC_ALGO(tp) = algo;
 			/*
 			 * If something goes pear shaped initialising the new