Re: [PATCH net RESEND 1/1] ipv4: reject RTAX_ADVMSS values below TCP_MIN_MSS
From: Eric Dumazet <edumazet@google.com>
Date: 2026-08-17 08:35:47
Subsystem:
networking [general], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
On Mon, Aug 17, 2026 at 10:28 AM Ido Schimmel [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On Fri, Aug 14, 2026 at 01:05:34AM +0800, Ren Wei wrote:quoted
From: Yong Wang <redacted> ip_metrics_convert() only caps RTAX_ADVMSS at the upper bound and still accepts undersized non-zero values from userspace. A route installed with "advmss 12" can later reach the passive TCP open path. When SYN timestamps are enabled, tcp_openreq_init_rwin() subtracts TCPOLEN_TSTAMP_ALIGNED from the route advmss before calling tcp_select_initial_window(). This can reduce the effective MSS to zero and trigger a divide-by-zero in the rounddown(space, mss) path. Reject non-zero RTAX_ADVMSS values smaller than TCP_MIN_MSS while keeping the existing "0 means use default advmss" behavior intact. This matches the existing TCP_MIN_MSS based validation used for TCP_MAXSEG and fixes the bug at the route metric input point rather than adding a redundant guard deeper in the TCP stack.Eric / Neal, Both sashiko instances [1][2] claim that this patch doesn't completely fix the divide-by-zero issue: it is still reachable by lowering net.ipv4.route.min_adv_mss to 0 and configuring a route with an MTU metric of 52. Given the above and the "We assume here that mss >= 1. This MUST be enforced by all callers" comment above tcp_select_initial_window(), do you prefer to fix this in TCP by enforcing a minimum MSS value? Something like [3]. Thanks [1] https://sashiko.dev/#/patchset/2c3901162c65a1d85cc1756a83a458db834d70c1.1786610865.git.edragain%40163.com [2] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/2c3901162c65a1d85cc1756a83a458db834d70c1.1786610865.git.edragain%40163.com [3]diff --git a/include/net/tcp.h b/include/net/tcp.h index 2c5b889530b5..670c20876f26 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h@@ -1782,6 +1782,11 @@ static inline int tcp_full_space(const struct sock *sk) return tcp_win_from_space(sk, READ_ONCE(sk->sk_rcvbuf)); } +static inline u32 tcp_dst_advmss(const struct dst_entry *dst) +{ + return max_t(u32, dst_metric_advmss(dst), TCP_MIN_MSS); +}
Sounds good, although we probably need READ_ONCE()/WRITE_ONCE() annotations.
diff --git a/include/net/dst.h b/include/net/dst.h
index 307073eae7f83456aa80dfa8686f839b302ca004..5ab5f2691ad8af2d27f6019f092adc1c02f8bf08100644
--- a/include/net/dst.h
+++ b/include/net/dst.h@@ -110,7 +110,7 @@ u32 *dst_cow_metrics_generic(struct dst_entry*dst, unsigned long old);
#define DST_METRICS_FLAGS 0x3UL
#define __DST_METRICS_PTR(Y) \
((u32 *)((Y) & ~DST_METRICS_FLAGS))
-#define DST_METRICS_PTR(X) __DST_METRICS_PTR((X)->_metrics)
+#define DST_METRICS_PTR(X) __DST_METRICS_PTR(READ_ONCE((X)->_metrics))
static inline bool dst_metrics_read_only(const struct dst_entry *dst)
{@@ -128,7 +128,7 @@ static inline voiddst_destroy_metrics_generic(struct dst_entry *dst)
static inline u32 *dst_metrics_write_ptr(struct dst_entry *dst)
{
- unsigned long p = dst->_metrics;
+ unsigned long p = READ_ONCE(dst->_metrics);
BUG_ON(!p);
@@ -169,7 +169,7 @@ dst_metric_raw(const struct dst_entry *dst, constint metric)
{
u32 *p = DST_METRICS_PTR(dst);
- return p[metric-1];
+ return READ_ONCE(p[metric-1]);
}
static inline u32@@ -197,7 +197,7 @@ static inline void dst_metric_set(struct dst_entry*dst, int metric, u32 val)
u32 *p = dst_metrics_write_ptr(dst);
if (p)
- p[metric-1] = val;
+ WRITE_ONCE(p[metric-1], val);
}
/* Kernel-internal feature bits that are unallocated in user space. */