From owner-svn-src-all@FreeBSD.ORG Thu Jun 11 13:44:08 2015 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 024389C1; Thu, 11 Jun 2015 13:44:08 +0000 (UTC) (envelope-from jch@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id D75C4103C; Thu, 11 Jun 2015 13:44:07 +0000 (UTC) (envelope-from jch@FreeBSD.org) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id t5BDi7kV073956; Thu, 11 Jun 2015 13:44:07 GMT (envelope-from jch@FreeBSD.org) Received: (from jch@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id t5BDi7p2073951; Thu, 11 Jun 2015 13:44:07 GMT (envelope-from jch@FreeBSD.org) Message-Id: <201506111344.t5BDi7p2073951@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: jch set sender to jch@FreeBSD.org using -f From: Julien Charbon Date: Thu, 11 Jun 2015 13:44:07 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r284261 - stable/10/sys/netinet X-SVN-Group: stable-10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Jun 2015 13:44:08 -0000 Author: jch Date: Thu Jun 11 13:44:06 2015 New Revision: 284261 URL: https://svnweb.freebsd.org/changeset/base/284261 Log: MFC r284245: 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. Modified: stable/10/sys/netinet/tcp_timer.c stable/10/sys/netinet/tcp_timer.h Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/netinet/tcp_timer.c ============================================================================== --- stable/10/sys/netinet/tcp_timer.c Thu Jun 11 13:26:16 2015 (r284260) +++ stable/10/sys/netinet/tcp_timer.c Thu Jun 11 13:44:06 2015 (r284261) @@ -298,10 +298,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_CPU(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); } @@ -381,11 +383,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_CPU(inp)); - } else - callout_reset_on(&tp->t_timers->tt_keep, TP_KEEPIDLE(tp), - tcp_timer_keep, tp, INP_CPU(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) @@ -760,6 +765,7 @@ tcp_timer_activate(struct tcpcb *tp, uin timeout_t *f_callout; struct inpcb *inp = tp->t_inpcb; int cpu = INP_CPU(inp); + uint32_t f_reset; #ifdef TCP_OFFLOAD if (tp->t_flags & TF_TOE) @@ -773,38 +779,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; + } } } } @@ -841,6 +858,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; @@ -848,30 +866,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: stable/10/sys/netinet/tcp_timer.h ============================================================================== --- stable/10/sys/netinet/tcp_timer.h Thu Jun 11 13:26:16 2015 (r284260) +++ stable/10/sys/netinet/tcp_timer.h Thu Jun 11 13:44:06 2015 (r284261) @@ -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)