From: Alexander Bluhm <bluhm@openbsd.org>
Subject: TCP window shifts unsinged long
To: tech@openbsd.org
Date: Tue, 5 Aug 2025 14:15:32 +0200

Hi,

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?

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;