[PATCH net v5] net: ipv4: add zero check for snd_cwnd_clamp

[PATCH net v5] net: ipv4: add zero check for snd_cwnd_clamp

[Kaho Jiang]

User can pass zero to snd_cwnd_clamp via RTAX_CWND type.
This could lead to division of zero error if not checked properly.

Oops: divide error: 0000 [#1] SMP KASAN NOPTI
Call Trace:
 <IRQ>
 tcp_reno_cong_avoid (net/ipv4/tcp_cong.c:510 (discriminator 1))
 tcp_ack (net/ipv4/tcp_input.c:3517 net/ipv4/tcp_input.c:3873 net/ipv4/tcp_input.c:4440)
 tcp_rcv_established (net/ipv4/tcp_input.c:6685)
 tcp_v4_do_rcv (net/ipv4/tcp_ipv4.c:1854)
 tcp_v4_rcv (net/ipv4/tcp_ipv4.c:2248)
 ip_protocol_deliver_rcu (net/ipv4/ip_input.c:207 (discriminator 1))
 ip_local_deliver_finish (net/ipv4/ip_input.c:241 (discriminator 1))
 ip_local_deliver (./include/linux/netfilter.h:325 ./include/linux/netfilter.h:319 net/ipv4/ip_input.c:262)
 ip_rcv (./include/net/dst.h:481 (discriminator 6) net/ipv4/ip_input.c:492 (discriminator 6) ./include/linux/netfilter.h:325 (discriminator 6) ./include/li)
 __netif_receive_skb_one_core (net/core/dev.c:6297 (discriminator 4))
 __netif_receive_skb (net/core/dev.c:6410)
 process_backlog (net/core/dev.c:6761)
 __napi_poll.constprop.0 (net/core/dev.c:7830)
 net_rx_action (net/core/dev.c:7893 net/core/dev.c:8050)
 handle_softirqs (kernel/softirq.c:645)
 __irq_exit_rcu (kernel/softirq.c:679 kernel/softirq.c:519 kernel/softirq.c:758)
 irq_exit_rcu (kernel/softirq.c:775)
 sysvec_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:1062 (discriminator 35) arch/x86/kernel/apic/apic.c:1062 (discriminator 35))
RIP: 0010:tcp_cong_avoid_ai (net/ipv4/tcp_cong.c:482)

Fix by checking if user pass in zero as parameter,
if so reject the value zero.

Fixes: 814d488c6126 ("tcp: fix the timid additive increase on stretch ACKs")
Cc: stable@vger.kernel.org
Reported-by: co+71b6509cb791e531@bugs.sh
Assisted-by: LLM
Signed-off-by: Kaho Jiang <kj2864@nyu.edu>
---
 include/net/tcp.h  | 2 +-
 net/ipv4/metrics.c | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 4416cdf9bf30..9f605d9ecb35 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1522,7 +1522,7 @@ static inline u32 tcp_snd_cwnd(const struct tcp_sock *tp)
 static inline void tcp_snd_cwnd_set(struct tcp_sock *tp, u32 val)
 {
 	WARN_ON_ONCE((int)val <= 0);
-	WRITE_ONCE(tp->snd_cwnd, val);
+	WRITE_ONCE(tp->snd_cwnd, max(val, 1U));
 }
 
 static inline bool tcp_in_slow_start(const struct tcp_sock *tp)
diff --git a/net/ipv4/metrics.c b/net/ipv4/metrics.c
index ad40762a8b38..cb36686973e5 100644
--- a/net/ipv4/metrics.c
+++ b/net/ipv4/metrics.c
@@ -54,6 +54,10 @@ static int ip_metrics_convert(struct nlattr *fc_mx,
 			NL_SET_ERR_MSG(extack, "Unknown flag set in feature mask in metrics attribute");
 			return -EINVAL;
 		}
+		if (type == RTAX_CWND && !val) {
+			NL_SET_ERR_MSG(extack, "RTAX_CWND must be greater than zero");
+			return -EINVAL;
+		}
 		metrics[type - 1] = val;
 	}
 
-- 
2.43.0

Re: [PATCH net v5] net: ipv4: add zero check for snd_cwnd_clamp

[Eric Dumazet]

On 10/20/26 00:25, Kaho Jiang wrote:
> User can pass zero to snd_cwnd_clamp via RTAX_CWND type.
> This could lead to division of zero error if not checked properly.
> 
> Oops: divide error: 0000 [#1] SMP KASAN NOPTI
> Call Trace:
>   <IRQ>
>   tcp_reno_cong_avoid (net/ipv4/tcp_cong.c:510 (discriminator 1))
>   tcp_ack (net/ipv4/tcp_input.c:3517 net/ipv4/tcp_input.c:3873 net/ipv4/tcp_input.c:4440)
>   tcp_rcv_established (net/ipv4/tcp_input.c:6685)
>   tcp_v4_do_rcv (net/ipv4/tcp_ipv4.c:1854)
>   tcp_v4_rcv (net/ipv4/tcp_ipv4.c:2248)
>   ip_protocol_deliver_rcu (net/ipv4/ip_input.c:207 (discriminator 1))
>   ip_local_deliver_finish (net/ipv4/ip_input.c:241 (discriminator 1))
>   ip_local_deliver (./include/linux/netfilter.h:325 ./include/linux/netfilter.h:319 net/ipv4/ip_input.c:262)
>   ip_rcv (./include/net/dst.h:481 (discriminator 6) net/ipv4/ip_input.c:492 (discriminator 6) ./include/linux/netfilter.h:325 (discriminator 6) ./include/li)
>   __netif_receive_skb_one_core (net/core/dev.c:6297 (discriminator 4))
>   __netif_receive_skb (net/core/dev.c:6410)
>   process_backlog (net/core/dev.c:6761)
>   __napi_poll.constprop.0 (net/core/dev.c:7830)
>   net_rx_action (net/core/dev.c:7893 net/core/dev.c:8050)
>   handle_softirqs (kernel/softirq.c:645)
>   __irq_exit_rcu (kernel/softirq.c:679 kernel/softirq.c:519 kernel/softirq.c:758)
>   irq_exit_rcu (kernel/softirq.c:775)
>   sysvec_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:1062 (discriminator 35) arch/x86/kernel/apic/apic.c:1062 (discriminator 35))
> RIP: 0010:tcp_cong_avoid_ai (net/ipv4/tcp_cong.c:482)
> 
> Fix by checking if user pass in zero as parameter,
> if so reject the value zero.
> 
> Fixes: 814d488c6126 ("tcp: fix the timid additive increase on stretch ACKs")
> Cc: stable@vger.kernel.org
> Reported-by: co+71b6509cb791e531@bugs.sh
> Assisted-by: LLM
> Signed-off-by: Kaho Jiang <kj2864@nyu.edu>
> ---
>   include/net/tcp.h  | 2 +-
>   net/ipv4/metrics.c | 4 ++++
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 4416cdf9bf30..9f605d9ecb35 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1522,7 +1522,7 @@ static inline u32 tcp_snd_cwnd(const struct tcp_sock *tp)
>   static inline void tcp_snd_cwnd_set(struct tcp_sock *tp, u32 val)
>   {
>   	WARN_ON_ONCE((int)val <= 0);
> -	WRITE_ONCE(tp->snd_cwnd, val);
> +	WRITE_ONCE(tp->snd_cwnd, max(val, 1U));

No. Please do not hide bugs.

I repeat : Do not change tcp_snd_cwnd_set()

>   }
>   
>   static inline bool tcp_in_slow_start(const struct tcp_sock *tp)
> diff --git a/net/ipv4/metrics.c b/net/ipv4/metrics.c
> index ad40762a8b38..cb36686973e5 100644
> --- a/net/ipv4/metrics.c
> +++ b/net/ipv4/metrics.c
> @@ -54,6 +54,10 @@ static int ip_metrics_convert(struct nlattr *fc_mx,
>   			NL_SET_ERR_MSG(extack, "Unknown flag set in feature mask in metrics attribute");
>   			return -EINVAL;
>   		}
> +		if (type == RTAX_CWND && !val) {
> +			NL_SET_ERR_MSG(extack, "RTAX_CWND must be greater than zero");
> +			return -EINVAL;
> +		}
>   		metrics[type - 1] = val;
>   	}

But is it really fixing anything ? I guess this is why you added the 
max() in tcp_snd_cwnd_set() because you did not understand the issue.

An unlocked CWND of zero is harmless.

I think a better fix would be:

diff --git a/net/ipv4/metrics.c b/net/ipv4/metrics.c
index 
ad40762a8b38379e4ccdc505e3885adb8dfff763..b7f8c47f72f9707adb4ef4dde5558af23b46a502 
100644
--- a/net/ipv4/metrics.c
+++ b/net/ipv4/metrics.c
@@ -60,6 +60,12 @@ static int ip_metrics_convert(struct nlattr *fc_mx,
         if (ecn_ca)
                 metrics[RTAX_FEATURES - 1] |= DST_FEATURE_ECN_CA;

+       if ((metrics[RTAX_LOCK - 1] & (1 << RTAX_CWND)) &&
+           !metrics[RTAX_CWND - 1]) {
+               NL_SET_ERR_MSG(extack, "Locked cwnd metric must be non 
zero");
+               return -EINVAL;
+       }
+
         return 0;
  }




Note that my email address is edumazet@kernel.org

Make sure to refresh your git tree, because git should have handled the 
change automatically.

pw-bot: cr

Re: [PATCH net v5] net: ipv4: add zero check for snd_cwnd_clamp

[Jerry Jiang]

My apologies, I didn't fully understand the issue and naively
implemented a fix in tcp_snd_cwnd_set. I've tested the patch you've
given and I believe it should fix the underlying issue, thank you for
the support to help me understand the true issue.

Here is the v6 proposal:
https://lore.kernel.org/all/20261021235732.1313305-2-kj2864@nyu.edu/



On Tue, Oct 20, 2026 at 6:31 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
>
>
> On 10/20/26 00:25, Kaho Jiang wrote:
> > User can pass zero to snd_cwnd_clamp via RTAX_CWND type.
> > This could lead to division of zero error if not checked properly.
> >
> > Oops: divide error: 0000 [#1] SMP KASAN NOPTI
> > Call Trace:
> >   <IRQ>
> >   tcp_reno_cong_avoid (net/ipv4/tcp_cong.c:510 (discriminator 1))
> >   tcp_ack (net/ipv4/tcp_input.c:3517 net/ipv4/tcp_input.c:3873 net/ipv4/tcp_input.c:4440)
> >   tcp_rcv_established (net/ipv4/tcp_input.c:6685)
> >   tcp_v4_do_rcv (net/ipv4/tcp_ipv4.c:1854)
> >   tcp_v4_rcv (net/ipv4/tcp_ipv4.c:2248)
> >   ip_protocol_deliver_rcu (net/ipv4/ip_input.c:207 (discriminator 1))
> >   ip_local_deliver_finish (net/ipv4/ip_input.c:241 (discriminator 1))
> >   ip_local_deliver (./include/linux/netfilter.h:325 ./include/linux/netfilter.h:319 net/ipv4/ip_input.c:262)
> >   ip_rcv (./include/net/dst.h:481 (discriminator 6) net/ipv4/ip_input.c:492 (discriminator 6) ./include/linux/netfilter.h:325 (discriminator 6) ./include/li)
> >   __netif_receive_skb_one_core (net/core/dev.c:6297 (discriminator 4))
> >   __netif_receive_skb (net/core/dev.c:6410)
> >   process_backlog (net/core/dev.c:6761)
> >   __napi_poll.constprop.0 (net/core/dev.c:7830)
> >   net_rx_action (net/core/dev.c:7893 net/core/dev.c:8050)
> >   handle_softirqs (kernel/softirq.c:645)
> >   __irq_exit_rcu (kernel/softirq.c:679 kernel/softirq.c:519 kernel/softirq.c:758)
> >   irq_exit_rcu (kernel/softirq.c:775)
> >   sysvec_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:1062 (discriminator 35) arch/x86/kernel/apic/apic.c:1062 (discriminator 35))
> > RIP: 0010:tcp_cong_avoid_ai (net/ipv4/tcp_cong.c:482)
> >
> > Fix by checking if user pass in zero as parameter,
> > if so reject the value zero.
> >
> > Fixes: 814d488c6126 ("tcp: fix the timid additive increase on stretch ACKs")
> > Cc: stable@vger.kernel.org
> > Reported-by: co+71b6509cb791e531@bugs.sh
> > Assisted-by: LLM
> > Signed-off-by: Kaho Jiang <kj2864@nyu.edu>
> > ---
> >   include/net/tcp.h  | 2 +-
> >   net/ipv4/metrics.c | 4 ++++
> >   2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index 4416cdf9bf30..9f605d9ecb35 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -1522,7 +1522,7 @@ static inline u32 tcp_snd_cwnd(const struct tcp_sock *tp)
> >   static inline void tcp_snd_cwnd_set(struct tcp_sock *tp, u32 val)
> >   {
> >       WARN_ON_ONCE((int)val <= 0);
> > -     WRITE_ONCE(tp->snd_cwnd, val);
> > +     WRITE_ONCE(tp->snd_cwnd, max(val, 1U));
>
> No. Please do not hide bugs.
>
> I repeat : Do not change tcp_snd_cwnd_set()
>
> >   }
> >
> >   static inline bool tcp_in_slow_start(const struct tcp_sock *tp)
> > diff --git a/net/ipv4/metrics.c b/net/ipv4/metrics.c
> > index ad40762a8b38..cb36686973e5 100644
> > --- a/net/ipv4/metrics.c
> > +++ b/net/ipv4/metrics.c
> > @@ -54,6 +54,10 @@ static int ip_metrics_convert(struct nlattr *fc_mx,
> >                       NL_SET_ERR_MSG(extack, "Unknown flag set in feature mask in metrics attribute");
> >                       return -EINVAL;
> >               }
> > +             if (type == RTAX_CWND && !val) {
> > +                     NL_SET_ERR_MSG(extack, "RTAX_CWND must be greater than zero");
> > +                     return -EINVAL;
> > +             }
> >               metrics[type - 1] = val;
> >       }
>
> But is it really fixing anything ? I guess this is why you added the
> max() in tcp_snd_cwnd_set() because you did not understand the issue.
>
> An unlocked CWND of zero is harmless.
>
> I think a better fix would be:
>
> diff --git a/net/ipv4/metrics.c b/net/ipv4/metrics.c
> index
> ad40762a8b38379e4ccdc505e3885adb8dfff763..b7f8c47f72f9707adb4ef4dde5558af23b46a502
> 100644
> --- a/net/ipv4/metrics.c
> +++ b/net/ipv4/metrics.c
> @@ -60,6 +60,12 @@ static int ip_metrics_convert(struct nlattr *fc_mx,
>          if (ecn_ca)
>                  metrics[RTAX_FEATURES - 1] |= DST_FEATURE_ECN_CA;
>
> +       if ((metrics[RTAX_LOCK - 1] & (1 << RTAX_CWND)) &&
> +           !metrics[RTAX_CWND - 1]) {
> +               NL_SET_ERR_MSG(extack, "Locked cwnd metric must be non
> zero");
> +               return -EINVAL;
> +       }
> +
>          return 0;
>   }
>
>
>
>
> Note that my email address is edumazet@kernel.org
>
> Make sure to refresh your git tree, because git should have handled the
> change automatically.
>
> pw-bot: cr
>

Re: [PATCH net v5] net: ipv4: add zero check for snd_cwnd_clamp

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 2 (1) · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch does not close the bug its commit message describes:
  userspace setting tp->snd_cwnd_clamp to 0 through route metrics.
- [Medium] The patch changes the core inline helper tcp_snd_cwnd_set()
  (include/net/tcp.h:1522-1526) to store max(val, 1U) instead of val.
- [Medium] ip_metrics_convert() now fails any IPv4 or IPv6 route
  add/replace whose RTA_METRICS carries RTAX_CWND = 0, with -EINVAL,…
- [Low] The new error path in ip_metrics_convert() uses a plain-text
  NL_SET_ERR_MSG(extack, "RTAX_CWND must be greater than zero") and does…

Pre-existing issues:
- [Medium] Even when no route sends a zero RTAX_CWND, tp->snd_cwnd_clamp
  can still become 0 through a lockless race in the TCP metrics cache.

--

net: ipv4: add zero check for snd_cwnd_clamp

This patch makes ip_metrics_convert() reject a zero RTAX_CWND value, so
userspace cannot set tp->snd_cwnd_clamp to 0 through route metrics. It
also changes tcp_snd_cwnd_set() to store max(val, 1U) instead of val.

> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 4416cdf9bf30..9f605d9ecb35 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1522,7 +1522,7 @@ static inline u32 tcp_snd_cwnd(const struct tcp_sock *tp)
>  static inline void tcp_snd_cwnd_set(struct tcp_sock *tp, u32 val)
>  {
>  	WARN_ON_ONCE((int)val <= 0);
> -	WRITE_ONCE(tp->snd_cwnd, val);
> +	WRITE_ONCE(tp->snd_cwnd, max(val, 1U));

[Severity: Medium]
The commit message only describes the netlink check:

  Fix by checking if user pass in zero as parameter,
  if so reject the value zero.

Should this hunk be described in the changelog too? tcp_snd_cwnd_set()
is used by every congestion control module, by tcp_input.c and
tcp_output.c, by the TCP_BPF_IW path in net/core/filter.c, and by
struct_ops congestion control through the tcp_slow_start() and
tcp_cong_avoid_ai() kfuncs.

With this change, a computed cwnd of 0 is quietly raised to 1 while the
WARN still fires. That can hide caller bugs and leave connections stuck
at cwnd 1.

The clamp also doesn't match the WARN condition next to it. Values of
0x80000000 and above trigger the WARN but are stored unchanged.

This hunk is also what actually prevents the divide error on the
RTAX_LOCK-only path described below. If a stable backport drops it as
unrelated, would the divide error come back?

>  }
>  
>  static inline bool tcp_in_slow_start(const struct tcp_sock *tp)
> diff --git a/net/ipv4/metrics.c b/net/ipv4/metrics.c
> index ad40762a8b38..cb36686973e5 100644
> --- a/net/ipv4/metrics.c
> +++ b/net/ipv4/metrics.c
> @@ -54,6 +54,10 @@ static int ip_metrics_convert(struct nlattr *fc_mx,
>  			NL_SET_ERR_MSG(extack, "Unknown flag set in feature mask in metrics attribute");
>  			return -EINVAL;
>  		}
> +		if (type == RTAX_CWND && !val) {
> +			NL_SET_ERR_MSG(extack, "RTAX_CWND must be greater than zero");

[Severity: Low]
This isn't a bug, but could this use NL_SET_ERR_MSG_ATTR(extack, nla, ...)
like the length check earlier in ip_metrics_convert()? That way userspace
learns which attribute was rejected.

> +			return -EINVAL;
> +		}

[Severity: High]
Does this check close the path the commit message describes? It only
runs when an RTAX_CWND attribute is present, and it never looks at
RTAX_LOCK.

A request can set the CWND bit in RTAX_LOCK and leave RTAX_CWND out
entirely. ip_fib_metrics_init() allocates the array with kzalloc_obj(),
so the CWND slot stays 0. dst_metric_locked() only checks the RTAX_LOCK
mask.

>From there:

tcpm_suck_dst()
    copies the CWND lock bit and dst_metric_raw(dst, RTAX_CWND) == 0
tcp_init_metrics()
    if (tcp_metric_locked(tm, TCP_METRIC_CWND))
        tp->snd_cwnd_clamp = tcp_metric_get(tm, TCP_METRIC_CWND);

When an ssthresh metric is cached, tcp_init_metrics() then also clamps
snd_ssthresh to 0. tcp_update_metrics() never overwrites a locked CWND,
so the 0 stays for later connections.

Next, tcp_init_transfer() calls tcp_snd_cwnd_set() with tcp_init_cwnd(),
which is min(cwnd, 0) == 0. tcp_slow_start() and tcp_cong_avoid_ai() then
pass min(cwnd, snd_cwnd_clamp) == 0 on every ACK.

The divide error is now avoided only by the max(val, 1U) in
tcp_snd_cwnd_set(). WARN_ON_ONCE((int)val <= 0) still runs before that
clamp. Anyone with CAP_NET_ADMIN in a user namespace can still trigger
the warning, which panics with panic_on_warn. The connection also stays
at cwnd 1.

For comparison, TCP_BPF_SNDCWND_CLAMP in net/core/filter.c rejects
val <= 0.

Would it be better to fix this in tcp_init_metrics(), applying a locked
CWND only when it is non-zero? Another option is to check the final
metrics array against RTAX_LOCK after the parse loop.

[Severity: Medium]
This check also changes rtnetlink behavior for both IPv4 and IPv6 routes.
An RTM_NEWROUTE with RTAX_CWND = 0 in RTA_METRICS, for example
"ip route replace ... cwnd 0", used to succeed. It now fails with -EINVAL
whether or not the CWND lock bit is set.

An unlocked CWND of 0 is the same as leaving the metric out, since
dst_metric_raw() returns 0 either way. tcp_init_metrics() only reads
TCP_METRIC_CWND when it is locked, and tcp_update_metrics() only updates
it when it is unlocked.

Is rejecting the unlocked case intended? The commit message doesn't
mention it, and a fix in tcp_init_metrics() would leave what rtnetlink
accepts unchanged.

>  		metrics[type - 1] = val;
>  	}
>  

[Severity: Medium]
This is a pre-existing issue, not one introduced by this patch, but
tp->snd_cwnd_clamp can still become 0 without any route carrying a zero
RTAX_CWND.

tcp_get_metrics() calls tcpm_check_stamp() with only rcu_read_lock held.
After TCP_METRICS_TIMEOUT, that can call tcpm_suck_dst(), which rewrites
a live entry in place:

net/ipv4/tcp_metrics.c:tcpm_suck_dst() {
    ...
	WRITE_ONCE(tm->tcpm_lock, val);
    ...
	tcp_metric_set(tm, TCP_METRIC_CWND,
		       dst_metric_raw(dst, RTAX_CWND));
    ...
}

tcp_init_metrics() reads the lock bit and the CWND value as two separate
loads. Take an entry built from a dst with no cwnd metric (lock 0,
value 0) that gets refreshed from a dst with "cwnd lock N". Can a
concurrent reader see the new lock bit with the old value 0 and set
snd_cwnd_clamp to 0?

The reverse transition has the same window. The reclaim path in
tcpm_new() also rewrites entries that RCU readers can still see. With
policy routing, two concurrent tcpm_suck_dst() calls could also leave an
entry locked with value 0 until the next refresh.

The new netlink check doesn't cover this case. Only the max(val, 1U) in
tcp_snd_cwnd_set() avoids the divide, and the WARN_ON_ONCE still fires.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261019222531.1073-1-kj2864%40nyu.edu