Re: [PATCH net RESEND 1/1] ipv4: reject RTAX_ADVMSS values below TCP_MIN_MSS

On Mon, Aug 17, 2026 at 10:28 AM Ido Schimmel <[email protected]> wrote:
>
> On Fri, Aug 14, 2026 at 01:05:34AM +0800, Ren Wei wrote:
> > From: Yong Wang <[email protected]>
> >
> > 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..5ab5f2691ad8af2d27f6019f092adc1c02f8bf08
100644
--- 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 void
dst_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, const
int 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. */