Re: [PATCH net 1/1] ipv4: reject RTAX_ADVMSS values below TCP_MIN_MSS
Jakub Kicinski <[email protected]>
| Newsgroups | gmane.linux.network |
|---|---|
| Message-ID | <[email protected]> |
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 <[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, 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/[email protected]/ > > Thanks > > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > Cc: [email protected] > > Reported-by: Vega <[email protected]> > > Assisted-by: Codex:GPT-5.4 > > Signed-off-by: Yong Wang <[email protected]> > > Signed-off-by: Ren Wei <[email protected]> > > --- > > 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 >