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

[Jiayuan Chen]

On 9/30/26 7:21 PM, Eric Dumazet wrote:
> On Wed, Sep 30, 2026 at 12:10 PM Jiayuan Chen <jiayuan.chen@xxxxxxxxx> wrote:
> > beta_scale and cube_factor are computed once at module init, and they
> > need 1024 - beta and bic_scale * 10 to be positive: beta == 1024 or
> > bic_scale == 0 crash right there, beta > 1024 or a negative bic_scale
> > gives garbage or wraps to 0. Negative beta can also make beta_scale 0.
> > Reject them.
> >
> > A small beta also gives a small beta_scale, and (cwnd * scale) >> 3
> > truncates to 0 for a tiny cwnd (e.g. 2), so the TCP friendliness loop
> > never ends. Clamp delta to 1.
> >
> > Fixes: df3271f3361b ("[TCP] BIC: CUBIC window growth (2.0)")
> > Signed-off-by: Jiayuan Chen <jiayuan.chen@xxxxxxxxx>
> > ---
> > Target net-next since it is not a big problem.
> > ---
> >   net/ipv4/tcp_cubic.c | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
> > index 119bf8cbb007c..2f04dca5be095 100644
> > --- a/net/ipv4/tcp_cubic.c
> > +++ b/net/ipv4/tcp_cubic.c
> > @@ -298,7 +298,7 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd, u32 acked)
> >          if (tcp_friendliness) {
> >                  u32 scale = beta_scale;
> >
> > -               delta = (cwnd * scale) >> 3;
> I would prefer not adding a test in the fast path to work around
> silly module parameters.
>
> beta_scale is computed once at module init, we can make sure it is >= 8
> there. tcp_snd_cwnd() is >= 1, so delta would be >= 1.


Agreed. beta_scale is 8 / alpha_cubic, so >= 8 just caps alpha_cubic
at 1


> With the integer divisions, beta_scale >= 8 iff beta >= 512,
> so the default beta (717 -> beta_scale = 15) is not affected.
>
>
> > +               delta = max((cwnd * scale) >> 3, 1U);
> >                  while (ca->ack_cnt > delta) {           /* update tcp cwnd */
> >                          ca->ack_cnt -= delta;
> >                          ca->tcp_cwnd++;
> > @@ -504,6 +504,11 @@ 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 * 10 can overflow if bic_scale > INT_MAX / 10


Right, will add it.


> > +               pr_err("tcp_cubic: invalid beta %d or bic_scale %d\n", beta, bic_scale);
> > +               return -EINVAL;
> > +       }
> > +
> Something like this (untested) :
>
> 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);
>
> Thanks.