d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

NULL out cc_data in pluggable TCP {cc}_cb_destroy

Browse Source 
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@.

Submitted by: Kevin Bowling
Reviewed by: lstewart
Sponsored by: Limelight Networks
Differential Revision: https://reviews.freebsd.org/D16282
This commit is contained in:
Matt Macy 2018-07-22 05:37:58 +00:00
parent 8707733f71
commit 2269988749
8 changed files with 18 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
sys/netinet/cc/cc_chd.c
View File

@ -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

4
sys/netinet/cc/cc_cubic.c
View File

@ -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

3
sys/netinet/cc/cc_dctcp.c
View File

@ -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

4
sys/netinet/cc/cc_htcp.c
View File

@ -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

4
sys/netinet/cc/cc_newreno.c
View File

@ -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

4
sys/netinet/cc/cc_vegas.c
View File

@ -170,9 +170,7 @@ vegas_ack_received(struct cc_var *ccv, uint16_t ack_type)
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

13
sys/netinet/tcp_subr.c
View File

@ -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);

1
sys/netinet/tcp_usrreq.c
View File

@ -1729,6 +1729,7 @@ tcp_default_ctloutput(struct socket *so, struct sockopt *sopt, struct inpcb *inp
*/
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