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

[Jakub Kicinski <kuba@kernel.org>]

Hi Eric!

Welcome back! This patch is waiting on your input, when you have a sec..

On Thu, 30 Jul 2026 16:35:46 +0300 Ido Schimmel wrote:
> On Tue, Jul 28, 2026 at 01:08:17PM +0800, Ren Wei wrote:
> > From: Yong Wang <edragain@163.com>
> > 
> > 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, the comment above tcp_select_initial_window() says:
> 
> "[...]. We assume here that mss >= 1. This MUST be enforced by all
> callers".
> 
> AFAICT, tcp_openreq_init_rwin() and tcp_connect_init() are the only
> callers that subtract the size of the timestamp option from the MSS
> without validating the result.
> 
> Fixing it there also takes care of the comment from Sashiko regarding
> RTAX_MTU:
> 
> "If an unprivileged user sets the namespace specific sysctl
> net.ipv4.route.min_adv_mss to 0 (which is accessible due to an exporting
> flaw) and adds a route with an MTU of 52, the IPv4 stack evaluates the
> default advmss as max(MTU - 40, min_adv_mss), yielding 12.
> 
> [...]
> 
> Should a similar lower bound check be enforced for RTAX_MTU during
> netlink conversion to prevent this bypass?"
> 
> Do you prefer to fix this in TCP?
> 
> Sashiko link:
> 
> https://sashiko.dev/#/patchset/a2e93ae9003f33bf49b789dbd537f4a6c10f26fa.1784972917.git.edragain%40163.com
> 
> Patch link:
> 
> https://lore.kernel.org/netdev/cover.1784972917.git.edragain@163.com/
> 
> Thanks
> 
> > 
> > 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: Yong Wang <edragain@163.com>
> > Signed-off-by: Ren Wei <enjou1224z@gmail.com>
> > ---
> >  net/ipv4/metrics.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/net/ipv4/metrics.c b/net/ipv4/metrics.c
> > index ad40762a8b38..b9b97a0a5126 100644
> > --- a/net/ipv4/metrics.c
> > +++ b/net/ipv4/metrics.c
> > @@ -44,6 +44,12 @@ static int ip_metrics_convert(struct nlattr *fc_mx,
> >  			}
> >  			val = nla_get_u32(nla);
> >  		}
> > +		if (type == RTAX_ADVMSS && val && val < TCP_MIN_MSS) {
> > +			NL_SET_ERR_MSG_ATTR_FMT(extack, nla,
> > +						"Invalid advmss, must be 0 or >= %u",
> > +						TCP_MIN_MSS);
> > +			return -EINVAL;
> > +		}
> >  		if (type == RTAX_ADVMSS && val > 65535 - 40)
> >  			val = 65535 - 40;
> >  		if (type == RTAX_MTU && val > 65535 - 15)
> > -- 
> > 2.53.0  
>