git: e34ea0196f44 - main - tcp: clear all TCP timers in tcp_timer_stop() when in callout

From: Gleb Smirnoff <glebius_at_FreeBSD.org>
Date: Mon, 18 Mar 2024 20:57:10 UTC
The branch main has been updated by glebius:

URL: https://cgit.FreeBSD.org/src/commit/?id=e34ea0196f4497d3f3939025aff3a89ee77b652e

commit e34ea0196f4497d3f3939025aff3a89ee77b652e
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2024-03-18 20:57:00 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2024-03-18 20:57:00 +0000

    tcp: clear all TCP timers in tcp_timer_stop() when in callout
    
    When a TCP callout decides to disable self, e.g. tcp_timer_2msl() calling
    tcp_close(), we must also clear all other possible timers.  Otherwise,
    upon return, the callout would be scheduled again in tcp_timer_enter().
    
    Revert 57e27ff07aff, which was a temporary partial revert of otherwise
    correct 62d47d73b7eb, that exposed the problem being fixed now.  Add an
    extra assertion in tcp_timer_enter() to check we aren't arming callout for
    a closed connection.
    
    Reviewed by:    rscheff
---
 sys/netinet/tcp_subr.c  | 3 +--
 sys/netinet/tcp_timer.c | 6 ++++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c
index f618bc1ba04b..a6f84c297688 100644
--- a/sys/netinet/tcp_subr.c
+++ b/sys/netinet/tcp_subr.c
@@ -2395,10 +2395,9 @@ tcp_discardcb(struct tcpcb *tp)
 #endif
 
 	INP_WLOCK_ASSERT(inp);
+	MPASS(!callout_active(&tp->t_callout));
 	MPASS(TAILQ_EMPTY(&tp->snd_holes));
 
-	tcp_timer_stop(tp);
-
 	/* free the reassembly queue, if any */
 	tcp_reass_flush(tp);
 
diff --git a/sys/netinet/tcp_timer.c b/sys/netinet/tcp_timer.c
index ed50659abf8e..785f68be5621 100644
--- a/sys/netinet/tcp_timer.c
+++ b/sys/netinet/tcp_timer.c
@@ -881,6 +881,7 @@ tcp_timer_enter(void *xtp)
 	if (tp_valid) {
 		tcp_bblog_timer(tp, which, TT_PROCESSED, 0);
 		if ((which = tcp_timer_next(tp, &precision)) != TT_N) {
+			MPASS(tp->t_state > TCPS_CLOSED);
 			callout_reset_sbt_on(&tp->t_callout,
 			    tp->t_timers[which], precision, tcp_timer_enter,
 			    tp, inp_to_cpuid(inp), C_ABSOLUTE);
@@ -939,8 +940,7 @@ tcp_timer_active(struct tcpcb *tp, tt_which which)
 /*
  * Stop all timers associated with tcpcb.
  *
- * Called only on tcpcb destruction.  The tcpcb shall already be dropped from
- * the pcb lookup database and socket is not losing the last reference.
+ * Called when tcpcb moves to TCPS_CLOSED.
  *
  * XXXGL: unfortunately our callout(9) is not able to fully stop a locked
  * callout even when only two threads are involved: the callout itself and the
@@ -963,6 +963,8 @@ tcp_timer_stop(struct tcpcb *tp)
 
 		stopped = callout_stop(&tp->t_callout);
 		MPASS(stopped == 0);
+		for (tt_which i = 0; i < TT_N; i++)
+			tp->t_timers[i] = SBT_MAX;
 	} else while(__predict_false(callout_stop(&tp->t_callout) == 0)) {
 		INP_WUNLOCK(inp);
 		kern_yield(PRI_UNCHANGED);