Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Jun 2009 20:52:31 +0100 (BST)
From:      Robert Watson <rwatson@FreeBSD.org>
To:        John Baldwin <jhb@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r193941 - head/sys/netinet
Message-ID:  <alpine.BSF.2.00.0906102051570.45017@fledge.watson.org>
In-Reply-To: <200906101827.n5AIRFoR022115@svn.freebsd.org>
References:  <200906101827.n5AIRFoR022115@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

On Wed, 10 Jun 2009, John Baldwin wrote:

>  Change a few members of tcpcb that store cached copies of ticks to be ints
>  instead of unsigned longs.  This fixes a few overflow edge cases on 64-bit
>  platforms.  Specifically, if an idle connection receives a packet shortly
>  before 2^31 clock ticks of uptime (about 25 days with hz=1000) and the keep
>  alive timer fires after 2^31 clock ticks, the keep alive timer will think
>  that the connection has been idle for a very long time and will immediately
>  drop the connection instead of sending a keep alive probe.
>
>  Reviewed by:	silby, gnn, lstewart
>  MFC after:	1 week

That's pretty subtle even as TCP bugs go, nice work :-).

Robert N M Watson
Computer Laboratory
University of Cambridge

>
> Modified:
>  head/sys/netinet/tcp_input.c
>  head/sys/netinet/tcp_usrreq.c
>  head/sys/netinet/tcp_var.h
>
> Modified: head/sys/netinet/tcp_input.c
> ==============================================================================
> --- head/sys/netinet/tcp_input.c	Wed Jun 10 18:26:02 2009	(r193940)
> +++ head/sys/netinet/tcp_input.c	Wed Jun 10 18:27:15 2009	(r193941)
> @@ -1778,7 +1778,7 @@ tcp_do_segment(struct mbuf *m, struct tc
> 	    TSTMP_LT(to.to_tsval, tp->ts_recent)) {
>
> 		/* Check to see if ts_recent is over 24 days old.  */
> -		if ((int)(ticks - tp->ts_recent_age) > TCP_PAWS_IDLE) {
> +		if ((ticks - tp->ts_recent_age) > TCP_PAWS_IDLE) {
> 			/*
> 			 * Invalidate ts_recent.  If this segment updates
> 			 * ts_recent, the age will be reset later and ts_recent
>
> Modified: head/sys/netinet/tcp_usrreq.c
> ==============================================================================
> --- head/sys/netinet/tcp_usrreq.c	Wed Jun 10 18:26:02 2009	(r193940)
> +++ head/sys/netinet/tcp_usrreq.c	Wed Jun 10 18:27:15 2009	(r193941)
> @@ -1823,7 +1823,7 @@ db_print_tcpcb(struct tcpcb *tp, const c
> 	    tp->snd_recover);
>
> 	db_print_indent(indent);
> -	db_printf("t_maxopd: %u   t_rcvtime: %lu   t_startime: %lu\n",
> +	db_printf("t_maxopd: %u   t_rcvtime: %u   t_startime: %u\n",
> 	    tp->t_maxopd, tp->t_rcvtime, tp->t_starttime);
>
> 	db_print_indent(indent);
> @@ -1854,7 +1854,7 @@ db_print_tcpcb(struct tcpcb *tp, const c
> 	    tp->snd_scale, tp->rcv_scale, tp->request_r_scale);
>
> 	db_print_indent(indent);
> -	db_printf("ts_recent: %u   ts_recent_age: %lu\n",
> +	db_printf("ts_recent: %u   ts_recent_age: %u\n",
> 	    tp->ts_recent, tp->ts_recent_age);
>
> 	db_print_indent(indent);
> @@ -1863,7 +1863,7 @@ db_print_tcpcb(struct tcpcb *tp, const c
>
> 	db_print_indent(indent);
> 	db_printf("snd_ssthresh_prev: %lu   snd_recover_prev: 0x%08x   "
> -	    "t_badrxtwin: %lu\n", tp->snd_ssthresh_prev,
> +	    "t_badrxtwin: %u\n", tp->snd_ssthresh_prev,
> 	    tp->snd_recover_prev, tp->t_badrxtwin);
>
> 	db_print_indent(indent);
>
> Modified: head/sys/netinet/tcp_var.h
> ==============================================================================
> --- head/sys/netinet/tcp_var.h	Wed Jun 10 18:26:02 2009	(r193940)
> +++ head/sys/netinet/tcp_var.h	Wed Jun 10 18:27:15 2009	(r193941)
> @@ -139,8 +139,8 @@ struct tcpcb {
>
> 	u_int	t_maxopd;		/* mss plus options */
>
> -	u_long	t_rcvtime;		/* inactivity time */
> -	u_long	t_starttime;		/* time connection was established */
> +	int	t_rcvtime;		/* inactivity time */
> +	int	t_starttime;		/* time connection was established */
> 	int	t_rtttime;		/* round trip time */
> 	tcp_seq	t_rtseq;		/* sequence number being timed */
>
> @@ -167,7 +167,7 @@ struct tcpcb {
> 	u_char	rcv_scale;		/* window scaling for recv window */
> 	u_char	request_r_scale;	/* pending window scaling */
> 	u_int32_t  ts_recent;		/* timestamp echo data */
> -	u_long	ts_recent_age;		/* when last updated */
> +	int	ts_recent_age;		/* when last updated */
> 	u_int32_t  ts_offset;		/* our timestamp offset */
>
> 	tcp_seq	last_ack_sent;
> @@ -175,7 +175,7 @@ struct tcpcb {
> 	u_long	snd_cwnd_prev;		/* cwnd prior to retransmit */
> 	u_long	snd_ssthresh_prev;	/* ssthresh prior to retransmit */
> 	tcp_seq	snd_recover_prev;	/* snd_recover prior to retransmit */
> -	u_long	t_badrxtwin;		/* window for retransmit recovery */
> +	int	t_badrxtwin;		/* window for retransmit recovery */
> 	u_char	snd_limited;		/* segments limited transmitted */
> /* SACK related state */
> 	int	snd_numholes;		/* number of holes seen by sender */
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.0906102051570.45017>