Date: Thu, 7 May 2020 01:34:41 +0000 (UTC) From: Michael Tuexen <tuexen@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r360735 - stable/11/sys/netinet Message-ID: <202005070134.0471Yfg4061271@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: tuexen Date: Thu May 7 01:34:41 2020 New Revision: 360735 URL: https://svnweb.freebsd.org/changeset/base/360735 Log: MFC r352592: Cleanup the RTO calculation and perform some consistency checks before computing the RTO. This should fix an overflow issue reported by Felix Weinrank in https://github.com/sctplab/usrsctp/issues/375 for the userland stack and found by running a fuzz tester. Modified: stable/11/sys/netinet/sctp_indata.c stable/11/sys/netinet/sctp_input.c stable/11/sys/netinet/sctputil.c stable/11/sys/netinet/sctputil.h Directory Properties: stable/11/ (props changed) Modified: stable/11/sys/netinet/sctp_indata.c ============================================================================== --- stable/11/sys/netinet/sctp_indata.c Thu May 7 01:31:21 2020 (r360734) +++ stable/11/sys/netinet/sctp_indata.c Thu May 7 01:34:41 2020 (r360735) @@ -3106,13 +3106,12 @@ sctp_process_segment_range(struct sctp_tcb *stcb, stru * update RTO too ? */ if (tp1->do_rtt) { - if (*rto_ok) { - tp1->whoTo->RTO = - sctp_calculate_rto(stcb, - &stcb->asoc, - tp1->whoTo, - &tp1->sent_rcv_time, - SCTP_RTT_FROM_DATA); + if (*rto_ok && + sctp_calculate_rto(stcb, + &stcb->asoc, + tp1->whoTo, + &tp1->sent_rcv_time, + SCTP_RTT_FROM_DATA)) { *rto_ok = 0; } if (tp1->whoTo->rto_needed == 0) { @@ -4084,16 +4083,12 @@ sctp_express_handle_sack(struct sctp_tcb *stcb, uint32 /* update RTO too? */ if (tp1->do_rtt) { - if (rto_ok) { - tp1->whoTo->RTO = - /* - * sa_ignore - * NO_NULL_CHK - */ - sctp_calculate_rto(stcb, - asoc, tp1->whoTo, - &tp1->sent_rcv_time, - SCTP_RTT_FROM_DATA); + if (rto_ok && + sctp_calculate_rto(stcb, + &stcb->asoc, + tp1->whoTo, + &tp1->sent_rcv_time, + SCTP_RTT_FROM_DATA)) { rto_ok = 0; } if (tp1->whoTo->rto_needed == 0) { @@ -4704,12 +4699,12 @@ hopeless_peer: /* update RTO too? */ if (tp1->do_rtt) { - if (rto_ok) { - tp1->whoTo->RTO = - sctp_calculate_rto(stcb, - asoc, tp1->whoTo, - &tp1->sent_rcv_time, - SCTP_RTT_FROM_DATA); + if (rto_ok && + sctp_calculate_rto(stcb, + &stcb->asoc, + tp1->whoTo, + &tp1->sent_rcv_time, + SCTP_RTT_FROM_DATA)) { rto_ok = 0; } if (tp1->whoTo->rto_needed == 0) { Modified: stable/11/sys/netinet/sctp_input.c ============================================================================== --- stable/11/sys/netinet/sctp_input.c Thu May 7 01:31:21 2020 (r360734) +++ stable/11/sys/netinet/sctp_input.c Thu May 7 01:34:41 2020 (r360735) @@ -547,7 +547,7 @@ sctp_process_init_ack(struct mbuf *m, int iphlen, int asoc->primary_destination, SCTP_FROM_SCTP_INPUT + SCTP_LOC_3); /* calculate the RTO */ - net->RTO = sctp_calculate_rto(stcb, asoc, net, &asoc->time_entered, + sctp_calculate_rto(stcb, asoc, net, &asoc->time_entered, SCTP_RTT_FROM_NON_DATA); retval = sctp_send_cookie_echo(m, offset, initack_limit, stcb, net); return (retval); @@ -647,7 +647,7 @@ sctp_handle_heartbeat_ack(struct sctp_heartbeat_chunk tv.tv_sec = cp->heartbeat.hb_info.time_value_1; tv.tv_usec = cp->heartbeat.hb_info.time_value_2; /* Now lets do a RTO with this */ - r_net->RTO = sctp_calculate_rto(stcb, &stcb->asoc, r_net, &tv, + sctp_calculate_rto(stcb, &stcb->asoc, r_net, &tv, SCTP_RTT_FROM_NON_DATA); if (!(r_net->dest_state & SCTP_ADDR_REACHABLE)) { r_net->dest_state |= SCTP_ADDR_REACHABLE; @@ -1676,8 +1676,7 @@ sctp_process_cookie_existing(struct mbuf *m, int iphle old.tv_sec = cookie->time_entered.tv_sec; old.tv_usec = cookie->time_entered.tv_usec; net->hb_responded = 1; - net->RTO = sctp_calculate_rto(stcb, asoc, net, - &old, + sctp_calculate_rto(stcb, asoc, net, &old, SCTP_RTT_FROM_NON_DATA); if (stcb->asoc.sctp_autoclose_ticks && @@ -2401,8 +2400,7 @@ sctp_process_cookie_new(struct mbuf *m, int iphlen, in /* calculate the RTT and set the encaps port */ old.tv_sec = cookie->time_entered.tv_sec; old.tv_usec = cookie->time_entered.tv_usec; - (*netp)->RTO = sctp_calculate_rto(stcb, asoc, *netp, - &old, SCTP_RTT_FROM_NON_DATA); + sctp_calculate_rto(stcb, asoc, *netp, &old, SCTP_RTT_FROM_NON_DATA); } /* respond with a COOKIE-ACK */ sctp_send_cookie_ack(stcb); @@ -2978,8 +2976,7 @@ sctp_handle_cookie_ack(struct sctp_cookie_ack_chunk *c SCTP_STAT_INCR_COUNTER32(sctps_activeestab); SCTP_STAT_INCR_GAUGE32(sctps_currestab); if (asoc->overall_error_count == 0) { - net->RTO = sctp_calculate_rto(stcb, asoc, net, - &asoc->time_entered, + sctp_calculate_rto(stcb, asoc, net, &asoc->time_entered, SCTP_RTT_FROM_NON_DATA); } (void)SCTP_GETTIME_TIMEVAL(&asoc->time_entered); Modified: stable/11/sys/netinet/sctputil.c ============================================================================== --- stable/11/sys/netinet/sctputil.c Thu May 7 01:31:21 2020 (r360734) +++ stable/11/sys/netinet/sctputil.c Thu May 7 01:34:41 2020 (r360735) @@ -2466,25 +2466,24 @@ sctp_mtu_size_reset(struct sctp_inpcb *inp, /* - * given an association and starting time of the current RTT period return - * RTO in number of msecs net should point to the current network + * Given an association and starting time of the current RTT period, update + * RTO in number of msecs. net should point to the current network. + * Return 1, if an RTO update was performed, return 0 if no update was + * performed due to invalid starting point. */ -uint32_t +int sctp_calculate_rto(struct sctp_tcb *stcb, struct sctp_association *asoc, struct sctp_nets *net, struct timeval *old, int rtt_from_sack) { - /*- - * given an association and the starting time of the current RTT - * period (in value1/value2) return RTO in number of msecs. - */ + struct timeval now; + uint64_t rtt_us; /* RTT in us */ int32_t rtt; /* RTT in ms */ uint32_t new_rto; int first_measure = 0; - struct timeval now; /************************/ /* 1. calculate new RTT */ @@ -2495,10 +2494,19 @@ sctp_calculate_rto(struct sctp_tcb *stcb, } else { (void)SCTP_GETTIME_TIMEVAL(&now); } + if ((old->tv_sec > now.tv_sec) || + ((old->tv_sec == now.tv_sec) && (old->tv_sec > now.tv_sec))) { + /* The starting point is in the future. */ + return (0); + } timevalsub(&now, old); + rtt_us = (uint64_t)1000000 * (uint64_t)now.tv_sec + (uint64_t)now.tv_usec; + if (rtt_us > SCTP_RTO_UPPER_BOUND * 1000) { + /* The RTT is larger than a sane value. */ + return (0); + } /* store the current RTT in us */ - net->rtt = (uint64_t)1000000 * (uint64_t)now.tv_sec + - (uint64_t)now.tv_usec; + net->rtt = rtt_us; /* compute rtt in ms */ rtt = (int32_t)(net->rtt / 1000); if ((asoc->cc_functions.sctp_rtt_calculated) && (rtt_from_sack == SCTP_RTT_FROM_DATA)) { @@ -2530,7 +2538,7 @@ sctp_calculate_rto(struct sctp_tcb *stcb, * Paper "Congestion Avoidance and Control", Annex A. * * (net->lastsa >> SCTP_RTT_SHIFT) is the srtt - * (net->lastsa >> SCTP_RTT_VAR_SHIFT) is the rttvar + * (net->lastsv >> SCTP_RTT_VAR_SHIFT) is the rttvar */ if (net->RTO_measured) { rtt -= (net->lastsa >> SCTP_RTT_SHIFT); @@ -2571,8 +2579,8 @@ sctp_calculate_rto(struct sctp_tcb *stcb, if (new_rto > stcb->asoc.maxrto) { new_rto = stcb->asoc.maxrto; } - /* we are now returning the RTO */ - return (new_rto); + net->RTO = new_rto; + return (1); } /* Modified: stable/11/sys/netinet/sctputil.h ============================================================================== --- stable/11/sys/netinet/sctputil.h Thu May 7 01:31:21 2020 (r360734) +++ stable/11/sys/netinet/sctputil.h Thu May 7 01:34:41 2020 (r360735) @@ -131,7 +131,7 @@ uint32_t sctp_get_next_mtu(uint32_t); void sctp_timeout_handler(void *); -uint32_t +int sctp_calculate_rto(struct sctp_tcb *, struct sctp_association *, struct sctp_nets *, struct timeval *, int);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202005070134.0471Yfg4061271>