One possible code path for syncache_respond() is:

 syncache_respond(A), ip_output(), ip_input(), tcp_input(), syncache_badack(B)

Which winds up deleting a different entry from the syncache.  Handle
this by not utilizing the next entry in the timer chain until after
syncache_respond() completes.  The case of A == B should not be possible.

Problem found by: Don Bowman <don@sandvine.com>

1 files changed, 7 insertions, 1 deletions

diff --git a/sys/netinet/tcp_syncache.c b/sys/netinet/tcp_syncache.c
index 0ef2f3d..a261d21 100644
--- a/sys/netinet/tcp_syncache.c
+++ b/sys/netinet/tcp_syncache.c
@@ -372,19 +372,25 @@ syncache_timer(xslot)
 		if (ticks < nsc->sc_rxttime)
 			break;
 		sc = nsc;
-		nsc = TAILQ_NEXT(sc, sc_timerq);
 		inp = sc->sc_tp->t_inpcb;
 		INP_LOCK(inp);
 		if (slot == SYNCACHE_MAXREXMTS ||
 		    slot >= tcp_syncache.rexmt_limit ||
 		    inp->inp_gencnt != sc->sc_inp_gencnt) {
+			nsc = TAILQ_NEXT(sc, sc_timerq);
 			syncache_drop(sc, NULL);
 			tcpstat.tcps_sc_stale++;
 			INP_UNLOCK(inp);
 			continue;
 		}
+		/*
+		 * syncache_respond() may call back into the syncache to
+		 * to modify another entry, so do not obtain the next
+		 * entry on the timer chain until it has completed.
+		 */
 		(void) syncache_respond(sc, NULL);
 		INP_UNLOCK(inp);
+		nsc = TAILQ_NEXT(sc, sc_timerq);
 		tcpstat.tcps_sc_retransmitted++;
 		TAILQ_REMOVE(&tcp_syncache.timerq[slot], sc, sc_timerq);
 		SYNCACHE_TIMEOUT(sc, slot + 1);