From ded1556fba39d5de057bbd6b1c756135fcfec288 Mon Sep 17 00:00:00 2001 From: bluhm Date: Tue, 6 Feb 2018 15:13:08 +0000 Subject: [PATCH] There was a race in the TCP timers. As they may sleep to grab the netlock, timers may still run after they have been disarmed. Deleting the timeout is not sufficient to cancel them, but the code from 4.4 BSD is assuming this. The solution is to add a flag for every timer to see whether it has been armed or canceled. Remove the TF_DEAD check as tcp_canceltimers() is called before the reaper timer is fired. Cancelation works reliably now. OK mpi@ --- sys/netinet/tcp_timer.c | 27 ++++++++++++++++++++------- sys/netinet/tcp_timer.h | 14 ++++++++++---- sys/netinet/tcp_var.h | 8 +++++++- 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/sys/netinet/tcp_timer.c b/sys/netinet/tcp_timer.c index 37641bde19e..5c16bd04ab7 100644 --- a/sys/netinet/tcp_timer.c +++ b/sys/netinet/tcp_timer.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tcp_timer.c,v 1.62 2018/01/23 21:41:17 bluhm Exp $ */ +/* $OpenBSD: tcp_timer.c,v 1.63 2018/02/06 15:13:08 bluhm Exp $ */ /* $NetBSD: tcp_timer.c,v 1.14 1996/02/13 23:44:09 christos Exp $ */ /* @@ -185,8 +185,11 @@ tcp_timer_rexmt(void *arg) uint32_t rto; NET_LOCK(); - if (tp->t_flags & TF_DEAD) + /* Ignore canceled timeouts or timeouts that have been rescheduled. */ + if (!ISSET((tp)->t_flags, TF_TMR_REXMT) || + timeout_pending(&tp->t_timer[TCPT_REXMT])) goto out; + CLR((tp)->t_flags, TF_TMR_REXMT); if ((tp->t_flags & TF_PMTUD_PEND) && tp->t_inpcb && SEQ_GEQ(tp->t_pmtud_th_seq, tp->snd_una) && @@ -370,10 +373,14 @@ tcp_timer_persist(void *arg) uint32_t rto; NET_LOCK(); - if ((tp->t_flags & TF_DEAD) || - TCP_TIMER_ISARMED(tp, TCPT_REXMT)) { + /* Ignore canceled timeouts or timeouts that have been rescheduled. */ + if (!ISSET((tp)->t_flags, TF_TMR_PERSIST) || + timeout_pending(&tp->t_timer[TCPT_PERSIST])) + goto out; + CLR((tp)->t_flags, TF_TMR_PERSIST); + + if (TCP_TIMER_ISARMED(tp, TCPT_REXMT)) goto out; - } tcpstat_inc(tcps_persisttimeo); /* * Hack: if the peer is dead/unreachable, we do not @@ -406,8 +413,11 @@ tcp_timer_keep(void *arg) struct tcpcb *tp = arg; NET_LOCK(); - if (tp->t_flags & TF_DEAD) + /* Ignore canceled timeouts or timeouts that have been rescheduled. */ + if (!ISSET((tp)->t_flags, TF_TMR_KEEP) || + timeout_pending(&tp->t_timer[TCPT_KEEP])) goto out; + CLR((tp)->t_flags, TF_TMR_KEEP); tcpstat_inc(tcps_keeptimeo); if (TCPS_HAVEESTABLISHED(tp->t_state) == 0) @@ -452,8 +462,11 @@ tcp_timer_2msl(void *arg) struct tcpcb *tp = arg; NET_LOCK(); - if (tp->t_flags & TF_DEAD) + /* Ignore canceled timeouts or timeouts that have been rescheduled. */ + if (!ISSET((tp)->t_flags, TF_TMR_2MSL) || + timeout_pending(&tp->t_timer[TCPT_2MSL])) goto out; + CLR((tp)->t_flags, TF_TMR_2MSL); tcp_timer_freesack(tp); diff --git a/sys/netinet/tcp_timer.h b/sys/netinet/tcp_timer.h index 69910ecb54e..966b533e7e3 100644 --- a/sys/netinet/tcp_timer.h +++ b/sys/netinet/tcp_timer.h @@ -1,4 +1,4 @@ -/* $OpenBSD: tcp_timer.h,v 1.15 2018/01/23 21:41:17 bluhm Exp $ */ +/* $OpenBSD: tcp_timer.h,v 1.16 2018/02/06 15:13:08 bluhm Exp $ */ /* $NetBSD: tcp_timer.h,v 1.6 1995/03/26 20:32:37 jtc Exp $ */ /* @@ -120,13 +120,19 @@ const char *tcptimers[TCPT_NTIMERS] = timeout_set_proc(&(tp)->t_timer[(timer)], tcp_timer_funcs[(timer)], tp) #define TCP_TIMER_ARM(tp, timer, nticks) \ - timeout_add(&(tp)->t_timer[(timer)], (nticks) * (hz / PR_SLOWHZ)) +do { \ + SET((tp)->t_flags, TF_TIMER << (timer)); \ + timeout_add(&(tp)->t_timer[(timer)], (nticks) * (hz / PR_SLOWHZ)); \ +} while (0) #define TCP_TIMER_DISARM(tp, timer) \ - timeout_del(&(tp)->t_timer[(timer)]) +do { \ + CLR((tp)->t_flags, TF_TIMER << (timer)); \ + timeout_del(&(tp)->t_timer[(timer)]); \ +} while (0) #define TCP_TIMER_ISARMED(tp, timer) \ - timeout_pending(&(tp)->t_timer[(timer)]) + ISSET((tp)->t_flags, TF_TIMER << (timer)) /* * Force a time value to be in a certain range. diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var.h index 15103b88792..eb6d3ac2a7b 100644 --- a/sys/netinet/tcp_var.h +++ b/sys/netinet/tcp_var.h @@ -1,4 +1,4 @@ -/* $OpenBSD: tcp_var.h,v 1.129 2018/01/23 21:41:17 bluhm Exp $ */ +/* $OpenBSD: tcp_var.h,v 1.130 2018/02/06 15:13:08 bluhm Exp $ */ /* $NetBSD: tcp_var.h,v 1.17 1996/02/13 23:44:24 christos Exp $ */ /* @@ -100,6 +100,12 @@ struct tcpcb { #define TF_NEEDOUTPUT 0x00800000 /* call tcp_output after tcp_input */ #define TF_BLOCKOUTPUT 0x01000000 /* avert tcp_output during tcp_input */ #define TF_NOPUSH 0x02000000 /* don't push */ +#define TF_TMR_REXMT 0x04000000 /* retransmit timer armed */ +#define TF_TMR_PERSIST 0x08000000 /* retransmit persistence timer armed */ +#define TF_TMR_KEEP 0x10000000 /* keep alive timer armed */ +#define TF_TMR_2MSL 0x20000000 /* 2*msl quiet time timer armed */ +#define TF_TMR_REAPER 0x40000000 /* delayed cleanup timer armed, dead */ +#define TF_TIMER TF_TMR_REXMT /* used to shift with TCPT values */ struct mbuf *t_template; /* skeletal packet for transmit */ struct inpcb *t_inpcb; /* back pointer to internet pcb */ -- 2.20.1