Re: [PATCH v2 bpf-next 7/8] bpf: tcp: Add kfunc to adjust sk->sk_rcvlowat.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: tcp: OOB write in __tcp_set_rcvlowat via unrelated socket
--

commit 68bce63bbe5802a4eee99c7c0f138096d7c16481
Author: Kuniyuki Iwashima <kuniyu@google.com>

bpf: tcp: Add kfunc to adjust sk->sk_rcvlowat.

This commit adds a new BPF kfunc bpf_tcp_ops_set_rcvlowat to allow
struct_ops programs to dynamically adjust sk->sk_rcvlowat. It clamps
negative values to INT_MAX and delegates the update to __tcp_set_rcvlowat().

> diff --git a/net/ipv4/bpf_tcp_ops.c b/net/ipv4/bpf_tcp_ops.c
> index b0cade34cce65..e93c8d13684ef 100644
> --- a/net/ipv4/bpf_tcp_ops.c
> +++ b/net/ipv4/bpf_tcp_ops.c
[ ... ]
> @@ -368,8 +368,62 @@ static struct bpf_struct_ops bpf_tcp_ops = {
>  	.owner = THIS_MODULE,
>  };
>  
> +__bpf_kfunc_start_defs();
> +
> +__bpf_kfunc int bpf_tcp_ops_set_rcvlowat(struct sock *sk, int rcvlowat,
> +					 const struct bpf_prog_aux *aux)
> +{
> +	u32 moff = aux->attach_st_ops_member_off;
> +	bool wakeup = false;
> +
> +	if (moff == offsetof(struct bpf_tcp_ops, dequeue_rcvq))
> +		wakeup = true;
> +
> +	if (rcvlowat < 0)
> +		rcvlowat = INT_MAX;
> +
> +	return __tcp_set_rcvlowat(sk, rcvlowat, wakeup);

[Severity: High]
Does this code allow an out-of-bounds write via an unrelated socket type?

If a privileged user loads a TCP struct_ops BPF program with an enqueue_rcvq
callback, the program can extract skb->sk. After a NULL check, the verifier
treats it as a trusted struct sock.

If the packet was sent over the loopback interface by a UDP socket, skb->sk
points to a struct udp_sock. When this socket pointer is passed to
bpf_tcp_ops_set_rcvlowat(), it is handed directly to __tcp_set_rcvlowat()
which unconditionally casts it to struct tcp_sock:

net/ipv4/tcp.c:__tcp_set_rcvlowat() {
    struct tcp_sock *tp = tcp_sk(sk);
    ...
        if (tp->window_clamp && tp->window_clamp < val)
            WRITE_ONCE(tp->window_clamp, val);
    ...
}

Because struct udp_sock is much smaller than struct tcp_sock, could this
cast and subsequent WRITE_ONCE() corrupt adjacent slab objects? Should there
be a check to ensure the socket protocol is TCP before calling
__tcp_set_rcvlowat()?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923213719.224838-1-kuniyu@google.com?part=7