From 582bfe6ed02cc87cd3b190b486c6fd368e39fea8 Mon Sep 17 00:00:00 2001 From: jch Date: Wed, 10 Jun 2015 20:43:07 +0000 Subject: [PATCH] Fix a callout race condition introduced in TCP timers callouts with r281599. In TCP timer context, it is not enough to check callout_stop() return value to decide if a callout is still running or not, previous callout_reset() return values have also to be checked. Differential Revision: https://reviews.freebsd.org/D2763 Reviewed by: hiren Approved by: hiren MFC after: 1 day Sponsored by: Verisign, Inc. --- sys/netinet/tcp_timer.c | 55 +++++++++++++++++++++++++++++------------ sys/netinet/tcp_timer.h | 6 +++++ 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/sys/netinet/tcp_timer.c b/sys/netinet/tcp_timer.c index a64e403eb179..d992492cddc7 100644 --- a/sys/netinet/tcp_timer.c +++ b/sys/netinet/tcp_timer.c @@ -347,11 +347,12 @@ tcp_timer_2msl(void *xtp) tp = tcp_close(tp); } else { if (tp->t_state != TCPS_TIME_WAIT && - ticks - tp->t_rcvtime <= TP_MAXIDLE(tp)) - callout_reset_on(&tp->t_timers->tt_2msl, - TP_KEEPINTVL(tp), tcp_timer_2msl, tp, - inp_to_cpuid(inp)); - else + ticks - tp->t_rcvtime <= TP_MAXIDLE(tp)) { + if (!callout_reset(&tp->t_timers->tt_2msl, + TP_KEEPINTVL(tp), tcp_timer_2msl, tp)) { + tp->t_timers->tt_flags &= ~TT_2MSL_RST; + } + } else tp = tcp_close(tp); } @@ -431,11 +432,14 @@ tcp_timer_keep(void *xtp) tp->rcv_nxt, tp->snd_una - 1, 0); free(t_template, M_TEMP); } - callout_reset_on(&tp->t_timers->tt_keep, TP_KEEPINTVL(tp), - tcp_timer_keep, tp, inp_to_cpuid(inp)); - } else - callout_reset_on(&tp->t_timers->tt_keep, TP_KEEPIDLE(tp), - tcp_timer_keep, tp, inp_to_cpuid(inp)); + if (!callout_reset(&tp->t_timers->tt_keep, TP_KEEPINTVL(tp), + tcp_timer_keep, tp)) { + tp->t_timers->tt_flags &= ~TT_KEEP_RST; + } + } else if (!callout_reset(&tp->t_timers->tt_keep, TP_KEEPIDLE(tp), + tcp_timer_keep, tp)) { + tp->t_timers->tt_flags &= ~TT_KEEP_RST; + } #ifdef TCPDEBUG if (inp->inp_socket->so_options & SO_DEBUG) @@ -810,6 +814,7 @@ tcp_timer_activate(struct tcpcb *tp, uint32_t timer_type, u_int delta) timeout_t *f_callout; struct inpcb *inp = tp->t_inpcb; int cpu = inp_to_cpuid(inp); + uint32_t f_reset; #ifdef TCP_OFFLOAD if (tp->t_flags & TF_TOE) @@ -823,38 +828,49 @@ tcp_timer_activate(struct tcpcb *tp, uint32_t timer_type, u_int delta) case TT_DELACK: t_callout = &tp->t_timers->tt_delack; f_callout = tcp_timer_delack; + f_reset = TT_DELACK_RST; break; case TT_REXMT: t_callout = &tp->t_timers->tt_rexmt; f_callout = tcp_timer_rexmt; + f_reset = TT_REXMT_RST; break; case TT_PERSIST: t_callout = &tp->t_timers->tt_persist; f_callout = tcp_timer_persist; + f_reset = TT_PERSIST_RST; break; case TT_KEEP: t_callout = &tp->t_timers->tt_keep; f_callout = tcp_timer_keep; + f_reset = TT_KEEP_RST; break; case TT_2MSL: t_callout = &tp->t_timers->tt_2msl; f_callout = tcp_timer_2msl; + f_reset = TT_2MSL_RST; break; default: panic("tp %p bad timer_type %#x", tp, timer_type); } if (delta == 0) { if ((tp->t_timers->tt_flags & timer_type) && - callout_stop(t_callout)) { - tp->t_timers->tt_flags &= ~timer_type; + callout_stop(t_callout) && + (tp->t_timers->tt_flags & f_reset)) { + tp->t_timers->tt_flags &= ~(timer_type | f_reset); } } else { if ((tp->t_timers->tt_flags & timer_type) == 0) { - tp->t_timers->tt_flags |= timer_type; + tp->t_timers->tt_flags |= (timer_type | f_reset); callout_reset_on(t_callout, delta, f_callout, tp, cpu); } else { /* Reset already running callout on the same CPU. */ - callout_reset(t_callout, delta, f_callout, tp); + if (!callout_reset(t_callout, delta, f_callout, tp)) { + /* + * Callout not cancelled, consider it as not + * properly restarted. */ + tp->t_timers->tt_flags &= ~f_reset; + } } } } @@ -891,6 +907,7 @@ tcp_timer_stop(struct tcpcb *tp, uint32_t timer_type) { struct callout *t_callout; timeout_t *f_callout; + uint32_t f_reset; tp->t_timers->tt_flags |= TT_STOPPED; @@ -898,30 +915,36 @@ tcp_timer_stop(struct tcpcb *tp, uint32_t timer_type) case TT_DELACK: t_callout = &tp->t_timers->tt_delack; f_callout = tcp_timer_delack_discard; + f_reset = TT_DELACK_RST; break; case TT_REXMT: t_callout = &tp->t_timers->tt_rexmt; f_callout = tcp_timer_rexmt_discard; + f_reset = TT_REXMT_RST; break; case TT_PERSIST: t_callout = &tp->t_timers->tt_persist; f_callout = tcp_timer_persist_discard; + f_reset = TT_PERSIST_RST; break; case TT_KEEP: t_callout = &tp->t_timers->tt_keep; f_callout = tcp_timer_keep_discard; + f_reset = TT_KEEP_RST; break; case TT_2MSL: t_callout = &tp->t_timers->tt_2msl; f_callout = tcp_timer_2msl_discard; + f_reset = TT_2MSL_RST; break; default: panic("tp %p bad timer_type %#x", tp, timer_type); } if (tp->t_timers->tt_flags & timer_type) { - if (callout_stop(t_callout)) { - tp->t_timers->tt_flags &= ~timer_type; + if (callout_stop(t_callout) && + (tp->t_timers->tt_flags & f_reset)) { + tp->t_timers->tt_flags &= ~(timer_type | f_reset); } else { /* * Can't stop the callout, defer tcpcb actual deletion diff --git a/sys/netinet/tcp_timer.h b/sys/netinet/tcp_timer.h index 02a7782f2b78..dbb8aee1a415 100644 --- a/sys/netinet/tcp_timer.h +++ b/sys/netinet/tcp_timer.h @@ -160,6 +160,12 @@ struct tcp_timer { #define TT_2MSL 0x0010 #define TT_MASK (TT_DELACK|TT_REXMT|TT_PERSIST|TT_KEEP|TT_2MSL) +#define TT_DELACK_RST 0x0100 +#define TT_REXMT_RST 0x0200 +#define TT_PERSIST_RST 0x0400 +#define TT_KEEP_RST 0x0800 +#define TT_2MSL_RST 0x1000 + #define TT_STOPPED 0x00010000 #define TP_KEEPINIT(tp) ((tp)->t_keepinit ? (tp)->t_keepinit : tcp_keepinit)