[PATCH net-next v2 0/3] tcp: fix a few divide-by-zero in congestion control modules

[PATCH net-next v2 0/3] tcp: fix a few divide-by-zero in congestion control modules

[Jiayuan Chen]

Patch 1 and 2 were reported by VEGA <vega@nebusec.ai>.

Patch 3 was found while reviewing the other TCP congestion control
modules for the same kind of divide-by-zero on module parameters.

v1 -> v2:
- Patch 3: rework as suggested by Eric.
- Patch 1 and 2: add Eric's Reviewed-by.
v1: https://lore.kernel.org/netdev/20260930100937.206377-1-jiayuan.chen@linux.dev/

Jiayuan Chen (3):
  tcp_bic: fix divide by zero on max_increment == 0
  tcp_hybla: fix divide by zero on rtt0 == 0
  tcp_cubic: fix divide by zero and endless loop on bad module params

 net/ipv4/tcp_bic.c   | 14 ++++++++++++--
 net/ipv4/tcp_cubic.c |  9 +++++++++
 net/ipv4/tcp_hybla.c | 16 ++++++++++++++--
 3 files changed, 35 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH net-next v2 1/3] tcp_bic: fix divide by zero on max_increment == 0

[Jiayuan Chen]

max_increment is how many packets cwnd can grow per RTT, so 0 makes
no sense, and bictcp_update() divides by it.

Reject values below 1 when the parameter is written.

Fixes: 83803034f423 ("[TCP]: Add TCP BIC congestion control module.")
Reported-by: VEGA <vega@nebusec.ai>
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Reviewed-by: Eric Dumazet <edumazet@kernel.org>
---
Target net-next since it is not a big problem.
---
 net/ipv4/tcp_bic.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_bic.c b/net/ipv4/tcp_bic.c
index 65444ff142413..27d2e38ff7932 100644
--- a/net/ipv4/tcp_bic.c
+++ b/net/ipv4/tcp_bic.c
@@ -27,15 +27,25 @@
 					  */
 
 static int fast_convergence = 1;
-static int max_increment = 16;
+static unsigned int max_increment = 16;
 static int low_window = 14;
 static int beta = 819;		/* = 819/1024 (BICTCP_BETA_SCALE) */
 static int initial_ssthresh;
 static int smooth_part = 20;
 
+static int max_increment_set(const char *val, const struct kernel_param *kp)
+{
+	return param_set_uint_minmax(val, kp, 1, INT_MAX);
+}
+
+static const struct kernel_param_ops max_increment_ops = {
+	.set = max_increment_set,
+	.get = param_get_uint,
+};
+
 module_param(fast_convergence, int, 0644);
 MODULE_PARM_DESC(fast_convergence, "turn on/off fast convergence");
-module_param(max_increment, int, 0644);
+module_param_cb(max_increment, &max_increment_ops, &max_increment, 0644);
 MODULE_PARM_DESC(max_increment, "Limit on increment allowed during binary search");
 module_param(low_window, int, 0644);
 MODULE_PARM_DESC(low_window, "lower bound on congestion window (for TCP friendliness)");
-- 
2.43.0

[PATCH net-next v2 2/3] tcp_hybla: fix divide by zero on rtt0 == 0

[Jiayuan Chen]

rtt0 is the reference RTT in ms, so 0 makes no sense, and
hybla_recalc_param() divides by it right from hybla_init().

Reject values below 1 when the parameter is written. Also cap it at
U32_MAX / USEC_PER_MSEC, since rtt0 * USEC_PER_MSEC can wrap to 0 on
32-bit. An rtt0 above that (about 71 minutes) makes no sense anyway.

Fixes: 835b3f0c0d7e ("[TCP]: Add TCP Hybla congestion control module.")
Fixes: 740b0f1841f6 ("tcp: switch rtt estimations to usec resolution")
Reported-by: VEGA <vega@nebusec.ai>
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Reviewed-by: Eric Dumazet <edumazet@kernel.org>
---
Target net-next since it is not a big problem.
---
 net/ipv4/tcp_hybla.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_hybla.c b/net/ipv4/tcp_hybla.c
index abd7d91807e54..b9b482180d30b 100644
--- a/net/ipv4/tcp_hybla.c
+++ b/net/ipv4/tcp_hybla.c
@@ -26,8 +26,20 @@ struct hybla {
 };
 
 /* Hybla reference round trip time (default= 1/40 sec = 25 ms), in ms */
-static int rtt0 = 25;
-module_param(rtt0, int, 0644);
+static unsigned int rtt0 = 25;
+
+static int rtt0_set(const char *val, const struct kernel_param *kp)
+{
+	/* avoid rtt0 * USEC_PER_MSEC overflow */
+	return param_set_uint_minmax(val, kp, 1, U32_MAX / USEC_PER_MSEC);
+}
+
+static const struct kernel_param_ops rtt0_ops = {
+	.set = rtt0_set,
+	.get = param_get_uint,
+};
+
+module_param_cb(rtt0, &rtt0_ops, &rtt0, 0644);
 MODULE_PARM_DESC(rtt0, "reference rout trip time (ms)");
 
 /* This is called to refresh values for hybla parameters */
-- 
2.43.0

Re: [PATCH net-next v2 2/3] tcp_hybla: fix divide by zero on rtt0 == 0

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In net/ipv4/tcp_hybla.c, the patch replaces `module_param(rtt0,
  int, 0644)` with `module_param_cb(rtt0, &rtt0_ops, &rtt0, 0644)`.

--

tcp_hybla: fix divide by zero on rtt0 == 0

This commit changes the rtt0 module parameter from int to unsigned int.
It also registers rtt0 through module_param_cb() with a setter that only
accepts values from 1 to U32_MAX / USEC_PER_MSEC. That stops
hybla_recalc_param() from dividing by zero, and stops rtt0 * USEC_PER_MSEC
from wrapping on 32-bit.

> diff --git a/net/ipv4/tcp_hybla.c b/net/ipv4/tcp_hybla.c
> index abd7d91807e54..b9b482180d30b 100644
> --- a/net/ipv4/tcp_hybla.c
> +++ b/net/ipv4/tcp_hybla.c
> @@ -26,8 +26,20 @@ struct hybla {

[ ... ]

> +static const struct kernel_param_ops rtt0_ops = {
> +	.set = rtt0_set,
> +	.get = param_get_uint,
> +};
> +
> +module_param_cb(rtt0, &rtt0_ops, &rtt0, 0644);
>  MODULE_PARM_DESC(rtt0, "reference rout trip time (ms)");

[Severity: Low]
Does this drop the parmtype entry for rtt0 from the .modinfo section of
tcp_hybla.ko?

The old module_param() line expanded to module_param_named(), which emits
the type tag:

include/linux/moduleparam.h:
#define module_param_named(name, value, type, perm)			   \
	param_check_##type(name, &(value));				   \
	module_param_cb(name, &param_ops_##type, &value, perm);		   \
	__MODULE_PARM_TYPE(name, #type)

module_param_cb() only expands to __module_param_call():

#define module_param_cb(name, ops, arg, perm)				      \
	__module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1, 0)

MODULE_PARM_DESC(rtt0, ...) is still there, but nothing provides a type.
After this change, modinfo tcp_hybla would show a description for rtt0
with no type suffix.

Since the storage is now unsigned int and it is read back with
param_get_uint, could a line like this be added after module_param_cb()?

__MODULE_PARM_TYPE(rtt0, "uint");

Other module_param_cb() users already do this, for example
arch/x86/kvm/mmu/mmu.c, fs/fuse/inode.c and
drivers/scsi/fcoe/fcoe_transport.c.

>  
>  /* This is called to refresh values for hybla parameters */

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001112948.322463-1-jiayuan.chen%40linux.dev

[PATCH net-next v2 3/3] tcp_cubic: fix divide by zero and endless loop on bad module params

[Jiayuan Chen]

beta_scale and cube_factor are computed once at module init. They need
1024 - beta to be positive and bic_scale * 10 to neither be 0 nor
overflow: beta == 1024 or bic_scale == 0 crash right there, and other
out of range values give garbage. Negative beta can also make
beta_scale 0. Reject them.

A small beta also gives a small beta_scale, and (cwnd * beta_scale) >> 3
truncates to 0 for a tiny cwnd, so the TCP friendliness loop never
ends. Make sure beta_scale is at least 8 at init, so this is always
>= 1 with cwnd >= 1 and the fast path is untouched.
This also caps alpha_cubic at 1 for beta < 512, which only affects
such unusual settings.

Fixes: df3271f3361b ("[TCP] BIC: CUBIC window growth (2.0)")
Suggested-by: Eric Dumazet <edumazet@kernel.org>
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
Target net-next since it is not a big problem.
---
 net/ipv4/tcp_cubic.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 119bf8cbb007c..33b383e1307fb 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -504,12 +504,21 @@ static int __init cubictcp_register(void)
 
 	BUILD_BUG_ON(sizeof(struct bictcp) > ICSK_CA_PRIV_SIZE);
 
+	if (beta < 0 || beta >= BICTCP_BETA_SCALE ||
+	    bic_scale <= 0 || bic_scale > INT_MAX / 10) {
+		pr_err("tcp_cubic: invalid beta %d or bic_scale %d\n",
+		       beta, bic_scale);
+		return -EINVAL;
+	}
+
 	/* Precompute a bunch of the scaling factors that are used per-packet
 	 * based on SRTT of 100ms
 	 */
 
 	beta_scale = 8*(BICTCP_BETA_SCALE+beta) / 3
 		/ (BICTCP_BETA_SCALE - beta);
+	/* bictcp_update() needs (cwnd * beta_scale) >> 3 to be >= 1 */
+	beta_scale = max(beta_scale, 8U);
 
 	cube_rtt_scale = (bic_scale * 10);	/* 1024*c/rtt */
 
-- 
2.43.0

Re: [PATCH net-next v2 3/3] tcp_cubic: fix divide by zero and endless loop on bad module params

[Eric Dumazet]

On Thu, Oct 1, 2026 at 1:30 PM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> beta_scale and cube_factor are computed once at module init. They need
> 1024 - beta to be positive and bic_scale * 10 to neither be 0 nor
> overflow: beta == 1024 or bic_scale == 0 crash right there, and other
> out of range values give garbage. Negative beta can also make
> beta_scale 0. Reject them.
>
> A small beta also gives a small beta_scale, and (cwnd * beta_scale) >> 3
> truncates to 0 for a tiny cwnd, so the TCP friendliness loop never
> ends. Make sure beta_scale is at least 8 at init, so this is always
> >= 1 with cwnd >= 1 and the fast path is untouched.
> This also caps alpha_cubic at 1 for beta < 512, which only affects
> such unusual settings.
>
> Fixes: df3271f3361b ("[TCP] BIC: CUBIC window growth (2.0)")
> Suggested-by: Eric Dumazet <edumazet@kernel.org>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
> Target net-next since it is not a big problem.
> ---
>  net/ipv4/tcp_cubic.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
> index 119bf8cbb007c..33b383e1307fb 100644
> --- a/net/ipv4/tcp_cubic.c
> +++ b/net/ipv4/tcp_cubic.c
> @@ -504,12 +504,21 @@ static int __init cubictcp_register(void)
>
>         BUILD_BUG_ON(sizeof(struct bictcp) > ICSK_CA_PRIV_SIZE);
>

A READ_ONCE(beta) is needed to close a race when/if TCP_CONG_CUBIC is a module.

This can be done later as a follow up, the window is extremely tiny
and most builds
have CONFIG_TCP_CONG_CUBIC=y (and CONFIG_DEFAULT_CUBIC=y)

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

Re: [PATCH net-next v2 3/3] tcp_cubic: fix divide by zero and endless loop on bad module params

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 0 (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).

- [Medium] The commit message says that after the floor `beta_scale =
  max(beta_scale, 8U)`, `(cwnd * beta_scale) >> 3` "is always >= 1 with…

Pre-existing issues:
- [Low] The patch checks `beta` only once, in the __init function
  cubictcp_register() (net/ipv4/tcp_cubic.c:507-512).

--

tcp_cubic: fix divide by zero and endless loop on bad module params

This patch makes cubictcp_register() check the beta and bic_scale module
parameters at init time. It rejects values that would cause a divide by
zero or an overflow when beta_scale and cube_factor are computed. It also
raises beta_scale to at least 8 so the TCP friendliness loop in
bictcp_update() never sees a zero delta.

> diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
> index 119bf8cbb007c..33b383e1307fb 100644
> --- a/net/ipv4/tcp_cubic.c
> +++ b/net/ipv4/tcp_cubic.c
> @@ -504,12 +504,21 @@ static int __init cubictcp_register(void)
>  
>  	BUILD_BUG_ON(sizeof(struct bictcp) > ICSK_CA_PRIV_SIZE);
>  
> +	if (beta < 0 || beta >= BICTCP_BETA_SCALE ||
> +	    bic_scale <= 0 || bic_scale > INT_MAX / 10) {
> +		pr_err("tcp_cubic: invalid beta %d or bic_scale %d\n",
> +		       beta, bic_scale);
> +		return -EINVAL;
> +	}

[Severity: Low]
This is a pre-existing issue and this patch doesn't introduce it. The
check only runs once, in the __init function cubictcp_register(), but
beta is still registered as writable:

    /* Note parameters that are used for precomputing scale factors are read-only */
    module_param(beta, int, 0644);

So root can later write any int to /sys/module/tcp_cubic/parameters/beta,
including negative values or values >= 1024. The write goes through the
generic param_set_int(), which does no range check.

cubictcp_recalc_ssthresh() reads the live beta on every loss event:

    ca->last_max_cwnd = (tcp_snd_cwnd(tp) * (BICTCP_BETA_SCALE + beta))
            / (2 * BICTCP_BETA_SCALE);
    ...
    return max((tcp_snd_cwnd(tp) * beta) / BICTCP_BETA_SCALE, 2U);

If beta is set out of range at runtime, can this u32 arithmetic produce a
garbage ssthresh and last_max_cwnd? beta_scale is also never recomputed,
so it would stop matching beta.

The comment above the module_param() calls says the precompute parameters
are read-only, which isn't true for beta.

Runtime writes don't bring back the divide by zero or the endless loop,
because beta_scale and cube_factor are only computed at init. Still, the
new range only holds at load time. Should beta be 0444 like bic_scale, or
use a custom setter that enforces the same bounds?

> +
>  	/* Precompute a bunch of the scaling factors that are used per-packet
>  	 * based on SRTT of 100ms
>  	 */
>  
>  	beta_scale = 8*(BICTCP_BETA_SCALE+beta) / 3
>  		/ (BICTCP_BETA_SCALE - beta);
> +	/* bictcp_update() needs (cwnd * beta_scale) >> 3 to be >= 1 */
> +	beta_scale = max(beta_scale, 8U);

[Severity: Medium]
The commit message says this makes (cwnd * beta_scale) >> 3 "always >= 1
with cwnd >= 1". Does that still hold when the u32 product in
bictcp_update() wraps?

net/ipv4/tcp_cubic.c:bictcp_update() {
    ...
	u32 scale = beta_scale;

	delta = (cwnd * scale) >> 3;
	while (ca->ack_cnt > delta) {		/* update tcp cwnd */
		ca->ack_cnt -= delta;
		ca->tcp_cwnd++;
	}
    ...
}

If the wrapped product is less than 8, delta is 0 and this loop never
ends during ACK processing.

With beta=0, the raw beta_scale is 8*1024/3/1024 = 2, and this clamp
raises it to 8. At cwnd = 1 << 29 the product is 2^32, which wraps to 0.
Without the clamp, delta would have been 2^27 there. The same happens for
any beta whose raw beta_scale is below 8 (roughly beta < 512), at
cwnd = k * 2^29.

There is also a case that predates this patch. beta=983 gives
beta_scale=130, and 33038210 * 130 = 4294967300 wraps to 4, so delta is
again 0.

All of these cwnd values are far above the documented design limit of
cwnd < 1 million packets. With the default beta=717, a zero delta needs a
cwnd above INT_MAX, so default settings are not affected. The normal TCP
state machine doesn't look able to reach the 2^29 cases. The ~33M case
might only be reachable with extreme tuning.

Would it be more robust to guard delta itself, for example by computing
it in 64 bits or forcing it to at least 1? If not, should the commit
message and comment say that the guarantee assumes the product doesn't
overflow?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001112948.322463-1-jiayuan.chen%40linux.dev

Re: [PATCH net-next v2 0/3] tcp: fix a few divide-by-zero in congestion control modules

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.