Re: [PATCH v8 net-next 2/2] bpf: make tcp_tso_autosize() available to BPF congestion controls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Type confusion in `tcp_tso_autosize` leading to out-of-bounds read when called with a non-TCP socket from BPF.
--

--- Patch [2]: [PATCH v8 net-next 2/2] bpf: make tcp_tso_autosize() available to BPF congestion controls ---
commit 6f535d3aed5d0c1f1117963d858318f7f63354f5
Author: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>

bpf: make tcp_tso_autosize() available to BPF congestion controls

Expose tcp_tso_autosize() as a BPF kfunc and register it in the TCP
congestion-control kfunc set. This allows BPF congestion controls to
reuse the kernel TSO autosizing logic while applying their own
minimum TSO segment policy.

> diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
> index ed4fea98dfde1..9deed2244c2de 100644
> --- a/net/ipv4/bpf_tcp_ca.c
> +++ b/net/ipv4/bpf_tcp_ca.c

[ ... ]

> @@ -194,6 +194,7 @@ BTF_ID_FLAGS(func, tcp_reno_cong_avoid)
>  BTF_ID_FLAGS(func, tcp_reno_undo_cwnd)
>  BTF_ID_FLAGS(func, tcp_slow_start)
>  BTF_ID_FLAGS(func, tcp_cong_avoid_ai)
> +BTF_ID_FLAGS(func, tcp_tso_autosize)
>  BTF_KFUNCS_END(bpf_tcp_ca_check_kfunc_ids)
>  
>  static const struct btf_kfunc_id_set bpf_tcp_ca_kfunc_set = {

[Severity: High]
Does this allow non-TCP struct_ops programs to call tcp_tso_autosize()?

Because bpf_tcp_ca_kfunc_set lacks a .filter callback, it appears this kfunc
becomes callable by any BPF program that can access a generic struct sock,
such as a bpf_qdisc program.

> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 7d3e0e715c4bf..a98036bcd987e 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c

[ ... ]

> @@ -2252,13 +2252,21 @@ static bool tcp_nagle_check(bool partial, const struct tcp_sock *tp,
>   * in bigger TSO bursts. We we cut the RTT-based allowance in half
>   * 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.
> + *
> + * The min_tso_segs is floored to 1 to avoid surprising conversion. Also,
> + * BPF callers may pass mss_now == 0. In that case the function returns the
> + * sanitized min_tso_segs value and skips autosizing.
>   */
> -u32 tcp_tso_autosize(const struct sock *sk, unsigned int mss_now,
> -		     int min_tso_segs)
> +__bpf_kfunc u32 tcp_tso_autosize(const struct sock *sk, unsigned int mss_now,
> +				 int min_tso_segs)
>  {
> +	u32 min_tso = max(min_tso_segs, 1);
>  	unsigned long bytes;
>  	u32 r;
>  
> +	if (unlikely(!mss_now))
> +		return min_tso;
> +
>  	bytes = READ_ONCE(sk->sk_pacing_rate) >> READ_ONCE(sk->sk_pacing_shift);
>  
>  	r = tcp_min_rtt(tcp_sk(sk)) >> READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_tso_rtt_log);

[Severity: High]
If a non-TCP BPF program passes a generic struct sock to this kfunc, can this
lead to an out-of-bounds read?

The tcp_sk() macro casts the generic struct sock to a struct tcp_sock without
validating the socket type. If a smaller socket structure (like struct udp_sock)
is passed, accessing tp->rtt_min in tcp_min_rtt() could read past the end of
the allocated socket structure.

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