Date: Wed, 10 Jun 2015 20:43:08 +0000 (UTC) From: Julien Charbon <jch@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r284245 - head/sys/netinet Message-ID: <201506102043.t5AKh8YB058825@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jch Date: Wed Jun 10 20:43:07 2015 New Revision: 284245 URL: https://svnweb.freebsd.org/changeset/base/284245 Log: 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. Modified: head/sys/netinet/tcp_timer.c head/sys/netinet/tcp_timer.h Modified: head/sys/netinet/tcp_timer.c ============================================================================== --- head/sys/netinet/tcp_timer.c Wed Jun 10 20:11:28 2015 (r284244) +++ head/sys/netinet/tcp_timer.c Wed Jun 10 20:43:07 2015 (r284245) @@ -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, uin 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, uin 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_ { 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_ 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 Modified: head/sys/netinet/tcp_timer.h ============================================================================== --- head/sys/netinet/tcp_timer.h Wed Jun 10 20:11:28 2015 (r284244) +++ head/sys/netinet/tcp_timer.h Wed Jun 10 20:43:07 2015 (r284245) @@ -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)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201506102043.t5AKh8YB058825>