Re: [PATCH v2 net-next 1/1] tcp: Replace min_tso_segs() with tso_segs() CC callback for TCP Prague

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] BPF programs can cause a kernel panic by passing `mss_now == 0` to the `bbr_tso_segs` kfunc, leading to a division by zero.
- [Medium] The `tcp_tso_autosize` function is not registered as a BPF kfunc, preventing BPF congestion control algorithms from reusing default TSO autosizing logic.
--

commit 39a9bf39a3860ba9dd1b2bed8669b97491df87c6
Author: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>

tcp: Replace min_tso_segs() with tso_segs() CC callback for TCP Prague

This patch replaces existing min_tso_segs() with tso_segs() CC callbak
for CC algorithm to provides explicit tso segment number of each data
burst and overrides tcp_tso_autosize().

> diff --git a/net/ipv4/tcp_bbr.c b/net/ipv4/tcp_bbr.c
> --- a/net/ipv4/tcp_bbr.c
> +++ b/net/ipv4/tcp_bbr.c
[ ... ]
> @@ -297,11 +297,19 @@ static void bbr_set_pacing_rate(struct sock *sk, u32 bw, int gain)
>  }
>  
>  /* override sysctl_tcp_min_tso_segs */
> -__bpf_kfunc static u32 bbr_min_tso_segs(struct sock *sk)
> +static u32 bbr_min_tso_segs(struct sock *sk)
>  {
>  	return READ_ONCE(sk->sk_pacing_rate) < (bbr_min_tso_rate >> 3) ? 1 : 2;
>  }
>  
> +__bpf_kfunc static u32 bbr_tso_segs(struct sock *sk, u32 mss_now)
> +{
> +	u32 min_tso;
> +
> +	min_tso = bbr_min_tso_segs(sk);
> +	return tcp_tso_autosize(sk, mss_now, min_tso);
> +}

[Severity: High]
Can a buggy BPF program trigger a divide-by-zero panic here?

Since bbr_tso_segs() is exposed as a BPF kfunc, the verifier does not
restrict the value ranges of scalar arguments like mss_now. If mss_now is 0,
it is passed directly to tcp_tso_autosize(), where it is used as a divisor:

tcp_tso_autosize()
    ...
    return max_t(u32, bytes / mss_now, min_tso_segs);

Does this need an explicit check for mss_now == 0 to prevent a local denial
of service?

> +
>  static u32 bbr_tso_segs_goal(struct sock *sk)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
[ ... ]
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
[ ... ]
> @@ -2269,6 +2269,7 @@ static u32 tcp_tso_autosize(const struct sock *sk, unsigned int mss_now,
>  
>  	return max_t(u32, bytes / mss_now, min_tso_segs);
>  }
> +EXPORT_SYMBOL(tcp_tso_autosize);

[Severity: Medium]
Is there a way for out-of-tree BPF congestion controls to reuse the default
autosizing logic?

With the removal of min_tso_segs, BPF congestion control algorithms that
want a custom minimum TSO size must implement tso_segs and handle the
autosizing themselves. However, while tcp_tso_autosize() is exported to C
modules here, it does not appear to be registered as a BPF kfunc (e.g., via
BTF_KFUNCS_START).

Does this force BPF developers to either lose custom minimum TSO sizing
capabilities or duplicate the tcp_tso_autosize() logic within their BPF
programs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260614071756.239686-1-chia-yu.chang@nokia-bell-labs.com?part=1