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

Fix a double-free when an inp transitions to INP_TIMEWAIT state

Browse Source 
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
This commit is contained in:
Julien Charbon 2016-10-18 07:16:49 +00:00
parent c30dcf40ba
commit f5cf1e5f5a
3 changed files with 42 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
sys/netinet/tcp_input.c
View File

@ -920,6 +920,16 @@ findpcb:
goto dropwithreset; goto dropwithreset;
} }
INP_WLOCK_ASSERT(inp); 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) && if ((inp->inp_flowtype == M_HASHTYPE_NONE) &&
(M_HASHTYPE_GET(m) != M_HASHTYPE_NONE) && (M_HASHTYPE_GET(m) != M_HASHTYPE_NONE) &&
((inp->inp_socket == NULL) || ((inp->inp_socket == NULL) ||
@ -980,6 +990,10 @@ relocked:
if (in_pcbrele_wlocked(inp)) { if (in_pcbrele_wlocked(inp)) {
inp = NULL; inp = NULL;
goto findpcb; goto findpcb;
} else if (inp->inp_flags & INP_DROPPED) {
INP_WUNLOCK(inp);
inp = NULL;
goto findpcb;
} }
} else } else
ti_locked = TI_RLOCKED; ti_locked = TI_RLOCKED;
@ -1039,6 +1053,10 @@ relocked:
if (in_pcbrele_wlocked(inp)) { if (in_pcbrele_wlocked(inp)) {
inp = NULL; inp = NULL;
goto findpcb; goto findpcb;
} else if (inp->inp_flags & INP_DROPPED) {
INP_WUNLOCK(inp);
inp = NULL;
goto findpcb;
} }
goto relocked; goto relocked;
} else } else

4
sys/netinet/tcp_timewait.c
View File

@ -231,6 +231,10 @@ tcp_twstart(struct tcpcb *tp)
INP_INFO_RLOCK_ASSERT(&V_tcbinfo); INP_INFO_RLOCK_ASSERT(&V_tcbinfo);
INP_WLOCK_ASSERT(inp); 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) { if (V_nolocaltimewait) {
int error = 0; int error = 0;
#ifdef INET6 #ifdef INET6

23
sys/netinet/tcp_usrreq.c
View File

@ -59,6 +59,7 @@ __FBSDID("$FreeBSD$");
#include <sys/protosw.h> #include <sys/protosw.h>
#include <sys/proc.h> #include <sys/proc.h>
#include <sys/jail.h> #include <sys/jail.h>
#include <sys/syslog.h>
#ifdef DDB #ifdef DDB
#include <ddb/ddb.h> #include <ddb/ddb.h>
@ -210,10 +211,26 @@ tcp_detach(struct socket *so, struct inpcb *inp)
* In all three cases the tcptw should not be freed here. * In all three cases the tcptw should not be freed here.
*/ */
if (inp->inp_flags & INP_DROPPED) { if (inp->inp_flags & INP_DROPPED) {
KASSERT(tp == NULL, ("tcp_detach: INP_TIMEWAIT && "
"INP_DROPPED && tp != NULL"));
in_pcbdetach(inp); 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 { } else {
in_pcbdetach(inp); in_pcbdetach(inp);
INP_WUNLOCK(inp); INP_WUNLOCK(inp);