Re: [PATCH v1 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] Missing validation for `mss_now` in the `bbr_tso_segs` BPF kfunc allows a divide-by-zero kernel panic.
- [Low] Missing EXPORT_SYMBOL for tcp_tso_autosize causes a build failure when tcp_bbr is compiled as a module.
--

commit 44d0693bc14ccae351456d523fdf83240793c554
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 the min_tso_segs() callback with a tso_segs() callback
to provide an explicit TSO segment number for each data burst, overriding
tcp_tso_autosize().

> diff --git a/net/ipv4/tcp_bbr.c b/net/ipv4/tcp_bbr.c
> index 82378a2bfd1e1..15536564246c0 100644
> --- 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]
Does this code allow a division by zero panic?

The BPF verifier does not enforce positive non-zero bounds on plain scalar
arguments. If a BPF program passes 0 for mss_now to bbr_tso_segs(), it is
passed unvalidated to tcp_tso_autosize(), where it is used as a divisor:

net/ipv4/tcp_output.c:tcp_tso_autosize() {
    ...
    return max_t(u32, bytes / mss_now, min_tso_segs);
}

This executes a division by zero, causing an immediate kernel panic. While
BPF programs require privileges, crashing the kernel via a kfunc violates
BPF safety guarantees.

> +}
> +
>  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
> index 26dd751ec72ab..14fde0aa14beb 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2253,8 +2253,8 @@ static bool tcp_nagle_check(bool partial, const struct tcp_sock *tp,
>   * for every 2^9 usec (aka 512 us) of RTT, so that the RTT-based allowance
>   * is below 1500 bytes after 6 * ~500 usec = 3ms.
>   */
> -static u32 tcp_tso_autosize(const struct sock *sk, unsigned int mss_now,
> -			    int min_tso_segs)
> +u32 tcp_tso_autosize(const struct sock *sk, unsigned int mss_now,
> +		     int min_tso_segs)

[Severity: Low]
Does this cause a build failure when tcp_bbr is compiled as a loadable module?

Because tcp_tso_autosize() is called from bbr_tso_segs() in tcp_bbr.c but
does not have an EXPORT_SYMBOL or EXPORT_SYMBOL_GPL declaration, building
CONFIG_TCP_CONG_BBR=m will fail with an undefined symbol error during modpost.

>  {
>  	unsigned long bytes;
>  	u32 r;

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