Download raw body.
TCP window shifts unsinged long
On Tue, Aug 05, 2025 at 02:15:32PM +0200, Alexander Bluhm wrote:
> Coverity complains about signed integer shift in
> tiwin = th->th_win << tp->snd_scale
> I think we are safe as TCP_MAX_WINSHIFT 14 prevents us from using
> potential sign bits.
>
> However, the type of the TCP window is inconsistent. I want to
> convert it to long. Maybe u_long would be even better, but let's
> start with the integer size. Converting to unsigned everywhere can
> be done later.
>
> ok?
ok jan@
> Index: netinet/tcp.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp.h,v
> diff -u -p -r1.24 tcp.h
> --- netinet/tcp.h 19 May 2023 01:04:39 -0000 1.24
> +++ netinet/tcp.h 2 Aug 2025 22:25:50 -0000
> @@ -113,7 +113,7 @@ struct tcphdr {
> */
> #define TCP_MSS 512
>
> -#define TCP_MAXWIN 65535 /* largest value for (unscaled) window */
> +#define TCP_MAXWIN 65535UL /* largest value for (unscaled) window */
>
> #define TCP_MAX_WINSHIFT 14 /* maximum window shift */
>
> Index: netinet/tcp_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
> diff -u -p -r1.457 tcp_input.c
> --- netinet/tcp_input.c 24 Jul 2025 21:34:07 -0000 1.457
> +++ netinet/tcp_input.c 2 Aug 2025 22:28:22 -0000
> @@ -690,7 +690,7 @@ findpcb:
>
> /* Unscale the window into a 32-bit value. */
> if ((tiflags & TH_SYN) == 0)
> - tiwin = th->th_win << tp->snd_scale;
> + tiwin = (u_long)th->th_win << tp->snd_scale;
> else
> tiwin = th->th_win;
>
> @@ -1156,12 +1156,12 @@ findpcb:
> * but not less than advertised window.
> */
> {
> - int win;
> + long win;
>
> win = sbspace(&so->so_rcv);
> if (win < 0)
> win = 0;
> - tp->rcv_wnd = imax(win, (int)(tp->rcv_adv - tp->rcv_nxt));
> + tp->rcv_wnd = lmax(win, (int)(tp->rcv_adv - tp->rcv_nxt));
> }
>
> switch (tp->t_state) {
> @@ -1551,7 +1551,7 @@ trimthenstep6:
> (TF_RCVD_SCALE|TF_REQ_SCALE)) {
> tp->snd_scale = tp->requested_s_scale;
> tp->rcv_scale = tp->request_r_scale;
> - tiwin = th->th_win << tp->snd_scale;
> + tiwin = (u_long)th->th_win << tp->snd_scale;
> }
> tcp_flush_queue(tp);
> tp->snd_wl1 = th->th_seq - 1;
> @@ -1582,11 +1582,13 @@ trimthenstep6:
> if (do_ecn && (tiflags & TH_ECE)) {
> if ((tp->t_flags & TF_ECN_PERMIT) &&
> SEQ_GEQ(tp->snd_una, tp->snd_last)) {
> - u_int win;
> + u_long win;
>
> - win = min(tp->snd_wnd, tp->snd_cwnd) / tp->t_maxseg;
> + win = ulmin(tp->snd_wnd, tp->snd_cwnd) /
> + tp->t_maxseg;
> if (win > 1) {
> - tp->snd_ssthresh = win / 2 * tp->t_maxseg;
> + tp->snd_ssthresh = win / 2 *
> + tp->t_maxseg;
> tp->snd_cwnd = tp->snd_ssthresh;
> tp->snd_last = tp->snd_max;
> tp->t_flags |= TF_SEND_CWR;
> Index: netinet/tcp_output.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_output.c,v
> diff -u -p -r1.156 tcp_output.c
> --- netinet/tcp_output.c 8 Jul 2025 00:47:41 -0000 1.156
> +++ netinet/tcp_output.c 2 Aug 2025 22:23:04 -0000
> @@ -418,7 +418,7 @@ again:
> * taking into account that we are limited by
> * TCP_MAXWIN << tp->rcv_scale.
> */
> - long adv = lmin(win, (long)TCP_MAXWIN << tp->rcv_scale) -
> + long adv = lmin(win, TCP_MAXWIN << tp->rcv_scale) -
> (tp->rcv_adv - tp->rcv_nxt);
>
> if (adv >= (long) (2 * tp->t_maxseg))
> @@ -859,13 +859,13 @@ send:
> */
> if (win < (rcv_hiwat / 4) && win < (long)tp->t_maxseg)
> win = 0;
> - if (win > (long)TCP_MAXWIN << tp->rcv_scale)
> - win = (long)TCP_MAXWIN << tp->rcv_scale;
> - if (win < (long)(int32_t)(tp->rcv_adv - tp->rcv_nxt))
> - win = (long)(int32_t)(tp->rcv_adv - tp->rcv_nxt);
> + if (win > TCP_MAXWIN << tp->rcv_scale)
> + win = TCP_MAXWIN << tp->rcv_scale;
> + if (win < (int32_t)(tp->rcv_adv - tp->rcv_nxt))
> + win = (int32_t)(tp->rcv_adv - tp->rcv_nxt);
> if (flags & TH_RST)
> win = 0;
> - th->th_win = htons((u_int16_t) (win>>tp->rcv_scale));
> + th->th_win = htons((u_int16_t)(win >> tp->rcv_scale));
> if (th->th_win == 0)
> tp->t_sndzerowin++;
> if (SEQ_GT(tp->snd_up, tp->snd_nxt)) {
> @@ -1164,7 +1164,7 @@ out:
> * then remember the size of the advertised window.
> * Any pending ACK has now been sent.
> */
> - if (win > 0 && SEQ_GT(tp->rcv_nxt+win, tp->rcv_adv))
> + if (win > 0 && SEQ_GT(tp->rcv_nxt + win, tp->rcv_adv))
> tp->rcv_adv = tp->rcv_nxt + win;
> tp->last_ack_sent = tp->rcv_nxt;
> tp->t_sndacktime = now;
> Index: netinet/tcp_subr.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_subr.c,v
> diff -u -p -r1.216 tcp_subr.c
> --- netinet/tcp_subr.c 18 Jul 2025 08:39:14 -0000 1.216
> +++ netinet/tcp_subr.c 2 Aug 2025 22:11:34 -0000
> @@ -294,7 +294,7 @@ tcp_respond(struct tcpcb *tp, caddr_t te
> tcp_seq ack, tcp_seq seq, int flags, u_int rtableid, uint64_t now)
> {
> int tlen;
> - int win = 0;
> + long win = 0;
> struct mbuf *m = NULL;
> struct tcphdr *th;
> struct ip *ip;
>
TCP window shifts unsinged long