Re: [PATCH bpf v3 1/2] bpf: Reject TCP_NODELAY in TCP header option callbacks

On Mon, Apr 20, 2026 at 09:41:06PM +0800, KaFai Wan wrote:
> > Does this same recursion vulnerability exist for BPF TCP congestion control
> > algorithms using BPF_PROG_TYPE_STRUCT_OPS?
> > 
> > If a BPF congestion control algorithm invokes bpf_setsockopt(TCP_NODELAY)
> > from its cwnd_event callback when handling CA_EVENT_TX_START, could it
> > trigger the same unbounded recursion?
> > 
> > When the kernel transmits the first packet of a data train via
> > tcp_transmit_skb(), it invokes tcp_event_data_sent(). Because
> > tp->packets_out is not incremented until later, tcp_packets_in_flight(tp)
> > evaluates to 0, triggering tcp_ca_event(sk, CA_EVENT_TX_START).
> > 
> > If the BPF program then calls bpf_setsockopt(TCP_NODELAY), it would result
> > in this call chain:
> > 
> > tcp_transmit_skb()
> >   tcp_event_data_sent() -> invokes CA_EVENT_TX_START
> >     cwnd_event()
> >       bpf_setsockopt(TCP_NODELAY)
> >         tcp_push_pending_frames()
> >           tcp_write_xmit()
> > 
> > Since the outer tcp_transmit_skb() hasn't finished, the send head hasn't
> > advanced. Wouldn't tcp_write_xmit() see the same SKB, attempt to transmit
> > it again, and re-enter tcp_transmit_skb() causing an infinite recursion?
> > 
> You are right. I can reproduce this. 
>  
> > Should the restriction on TCP_NODELAY be enforced at a broader level, such
> > as inside _bpf_setsockopt(), to protect contexts holding the socket lock
> > during TX paths?
> > 
> We can check in sol_tcp_sockopt().

I don't know how it can use the socket lock to single out this case.
All bpf programs that are allowed to call bpf_setsockopt should
have the sock lock held. Maybe I am missing something obvious.

In bpf_tcp_ca_get_func_proto, it checks what ops can do bpf_sk_setsockopt_proto.
Right now, it rejects the "release" ops. One option is to create a new
func_proto, bpf_sk_setsockopt_nodelay_proto, to reject TCP_NODELAY.
Instead of checking cwnd_event[_tx_start] in bpf_tcp_ca_get_func_proto,
I would return bpf_sk_setsockopt_nodelay_proto for all ops. We can revisit
and be more selective in the future if the hammer turns out to be too big.
"release" ops will remain disallowed from calling bpf_setsockopt.