From: Alexander Bluhm Subject: Re: TCP window shifts unsinged long To: Theo de Raadt Cc: Mark Kettenis , tech@openbsd.org Date: Mon, 11 Aug 2025 16:43:27 +0200 The source of the u_long types are in the socket buffer. struct sockbuf { u_long sb_cc; /* [m] actual chars in buffer */ u_long sb_datacc; /* [m] data only chars in buffer */ u_long sb_hiwat; /* [m] max actual char count */ u_long sb_wat; /* [m] default watermark */ u_long sb_mbcnt; /* [m] chars of mbufs used */ u_long sb_mbmax; /* [m] max chars of mbufs to use */ } The were converted from u_short to u_long here: commit 4924467d92d8f1e83e3d218800f0804c18ec2f95 Author: Mike Karels Date: Wed Jun 17 22:08:30 1987 -0700 use longs for sockbuf counters, it fits! SCCS-vsn: 7.2 Are you really suggesting to convert them to u_int? This would need a review of all comparisons in all domains and protocols. I doubt that this is feasible. All I want is getting some comparisons in TCP less complicated. The easiest way to achieve this, is to convert the size calculations to u_long. Then buffer length, window size, and sizeof have the same integer rank. I am pretty sure that 32 bit are sufficent. But in my opinion, long is the only type to which we can convert the existing code. Unless you want everthing in unsinged long long, but that is overkill, would hurt 32 bit platforms, and access is not MP atomic. The longer I look into these conversions, the more complicated it gets. Below is a diff where I convert all window sizes to u_long. Also I removed some more needless casts. At least that makes it consistent. I you don't like it, I will drop the diff. bluhm 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 11 Aug 2025 13:19:44 -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.458 tcp_input.c --- netinet/tcp_input.c 5 Aug 2025 09:51:12 -0000 1.458 +++ netinet/tcp_input.c 11 Aug 2025 14:33:26 -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; @@ -990,7 +990,7 @@ findpcb: #endif (!opti.ts_present || TSTMP_GEQ(opti.ts_val, tp->ts_recent)) && th->th_seq == tp->rcv_nxt && - tiwin && tiwin == tp->snd_wnd && + tiwin > 0 && tiwin == tp->snd_wnd && tp->snd_nxt == tp->snd_max) { /* @@ -1156,12 +1156,10 @@ findpcb: * but not less than advertised window. */ { - int win; + u_long win; - win = sbspace(&so->so_rcv); - if (win < 0) - win = 0; - tp->rcv_wnd = imax(win, (int)(tp->rcv_adv - tp->rcv_nxt)); + win = lmax(sbspace(&so->so_rcv), 0); + tp->rcv_wnd = lmax(win, (int)(tp->rcv_adv - tp->rcv_nxt)); } switch (tp->t_state) { @@ -1551,7 +1549,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 +1580,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; @@ -3881,7 +3881,7 @@ syn_cache_add(struct sockaddr *src, stru struct tcp_opt_info *oi, tcp_seq *issp, uint64_t now, int do_ecn) { struct tcpcb tb, *tp; - long win; + u_long win; struct syn_cache *sc; struct syn_cache_head *scp; struct mbuf *ipopts; @@ -3900,7 +3900,7 @@ syn_cache_add(struct sockaddr *src, stru /* * Initialize some local state. */ - win = sbspace(&so->so_rcv); + win = lmax(sbspace(&so->so_rcv), 0); if (win > TCP_MAXWIN) win = TCP_MAXWIN; 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 11 Aug 2025 14:21:53 -0000 @@ -190,7 +190,8 @@ int tcp_output(struct tcpcb *tp) { struct socket *so = tp->t_inpcb->inp_socket; - long len, win, rcv_hiwat, txmaxseg; + u_long win; + long len, rcv_hiwat, txmaxseg; int off, flags, error; struct mbuf *m; struct tcphdr *th; @@ -374,7 +375,7 @@ again: flags &= ~TH_FIN; mtx_enter(&so->so_rcv.sb_mtx); - win = sbspace_locked(&so->so_rcv); + win = lmax(sbspace_locked(&so->so_rcv), 0); rcv_hiwat = (long) so->so_rcv.sb_hiwat; mtx_leave(&so->so_rcv.sb_mtx); @@ -418,7 +419,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 = ulmin(win, TCP_MAXWIN << tp->rcv_scale) - (tp->rcv_adv - tp->rcv_nxt); if (adv >= (long) (2 * tp->t_maxseg)) @@ -857,15 +858,15 @@ send: * Calculate receive window. Don't shrink window, * but avoid silly window syndrome. */ - if (win < (rcv_hiwat / 4) && win < (long)tp->t_maxseg) + if (win < (rcv_hiwat / 4) && win < 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(win >> tp->rcv_scale); if (th->th_win == 0) tp->t_sndzerowin++; if (SEQ_GT(tp->snd_up, tp->snd_nxt)) { @@ -1164,7 +1165,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 11 Aug 2025 14:35:11 -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; + u_long win = 0; struct mbuf *m = NULL; struct tcphdr *th; struct ip *ip; @@ -305,7 +305,7 @@ tcp_respond(struct tcpcb *tp, caddr_t te if (tp) { struct socket *so = tp->t_inpcb->inp_socket; - win = sbspace(&so->so_rcv); + win = lmax(sbspace(&so->so_rcv), 0); /* * If this is called with an unconnected * socket/tp/pcb (tp->pf is 0), we lose. @@ -364,7 +364,7 @@ tcp_respond(struct tcpcb *tp, caddr_t te win >>= tp->rcv_scale; if (win > TCP_MAXWIN) win = TCP_MAXWIN; - th->th_win = htons((u_int16_t)win); + th->th_win = htons(win); th->th_urp = 0; if (tp && (tp->t_flags & (TF_REQ_TSTMP|TF_NOOPT)) == TF_REQ_TSTMP && Index: netinet/tcp_var.h =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_var.h,v diff -u -p -r1.195 tcp_var.h --- netinet/tcp_var.h 18 Jun 2025 16:15:46 -0000 1.195 +++ netinet/tcp_var.h 11 Aug 2025 14:08:40 -0000 @@ -248,7 +248,7 @@ struct syn_cache { struct refcnt sc_refcnt; /* ref count list and timer */ struct timeout sc_timer; /* rexmt timer */ struct route sc_route; /* [s] cached route */ - long sc_win; /* [I] advertised window */ + u_long sc_win; /* [I] advertised window */ struct syn_cache_head *sc_buckethead; /* [S] our bucket index */ struct syn_cache_set *sc_set; /* [S] our syn cache set */ u_int64_t sc_timestamp; /* [s] timestamp from SYN */