Re: [PATCH v1 bpf-next 2/8] bpf: tcp: Introduce BPF_SOCK_OPS_RCVLOWAT_CB.

On Fri, May 8, 2026 at 12:17 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The deliberate exclusion of BPF_SOCK_OPS_RCVLOWAT_CB from is_locked_tcp_sock_ops() disables critical socket helpers, notably bpf_sock_ops_cb_flags_set().
> --
>
> commit 2ab2e31caf792b1e7312616215a88ad7ae1ef4e1
> Author: Kuniyuki Iwashima <[email protected]>
>
> bpf: tcp: Introduce BPF_SOCK_OPS_RCVLOWAT_CB.
>
> This commit introduces a new hook, BPF_SOCK_OPS_RCVLOWAT_CB, which allows
> BPF programs to dynamically adjust sk->sk_rcvlowat upon recvmsg completion
> or when packets are enqueued. The commit leaves is_locked_tcp_sock_ops
> unchanged to prevent unnecessary enablement of bpf_setsockopt.
>
> >     This will allow the BPF prog to dynamically adjust sk->sk_rcvlowat,
> >     suppressing unnecessary EPOLLIN wakeups until sufficient data
> >     (e.g., a full RPC frame) is available in the receive queue.
> >
> >     Note that is_locked_tcp_sock_ops() is left unchanged not to enable
> >     bpf_setsockopt() unnecessarily.
>
> Does leaving is_locked_tcp_sock_ops() unchanged also prevent the use of
> bpf_sock_ops_cb_flags_set() from within the BPF_SOCK_OPS_RCVLOWAT_CB hook?

Good catch.

I need to support BPF_SOCK_OPS_RCVLOWAT_CB in
bpf_sock_ops_cb_flags_set().


>
> Because BPF_SOCK_OPS_RCVLOWAT_CB is appended to the enum after
> BPF_SOCK_OPS_WRITE_HDR_OPT_CB, is_locked_tcp_sock_ops() returns false
> for this new hook.
>
> Both bpf_sock_ops_cb_flags_set() and bpf_setsockopt() require
> is_locked_tcp_sock_ops() to return true. If it returns false, they fail
> with -EOPNOTSUPP.
>
> Since direct modification of bpf_sock_ops_cb_flags is disallowed by the
> verifier, wouldn't a BPF program running in this context be entirely unable
> to clear the BPF_SOCK_OPS_RCVLOWAT_CB_FLAG once it finishes parsing?
>
> If the flag cannot be cleared, does this permanently trap the socket in the
> fast-path hook for every enqueued packet and recvmsg() call, leading to
> performance degradation?
>
> While the potential UAF concern regarding bpf_skb_load_bytes() is a false
> positive addressed later in the series by "bpf: tcp: Support
> bpf_skb_load_bytes() for BPF_SOCK_OPS_RCVLOWAT_CB", and the hook omission
> is fixed by "bpf: tcp: Add SOCK_OPS rcvlowat hook", this limitation
> regarding is_locked_tcp_sock_ops() does not appear to be addressed later
> in the series.

Looks like Sashiko now has context of the series, nice.