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)
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to