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

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] The patch prevents an infinite recursion causing a kernel stack overflow by rejecting TCP_NODELAY in specific BPF_SOCK_OPS callbacks. However, it fails to address the exact same recursion vulnerability achievable via bpf_sk_setsockopt in a BPF TCP congestion control algorithm (BPF_PROG_TYPE_STRUCT_OPS).
--

commit 4ff4d98f6b4e3c3d90f9268761de84573f49fd86
Author: KaFai Wan <kafai.wan@linux.dev>

bpf: Reject TCP_NODELAY in TCP header option callbacks

This patch rejects bpf_setsockopt(TCP_NODELAY) when called from
BPF_SOCK_OPS_HDR_OPT_LEN_CB or BPF_SOCK_OPS_WRITE_HDR_OPT_CB to prevent
unbounded recursion and kernel stack overflow.

Closes: https://lore.kernel.org/bpf/d1d523c9-6901-4454-a183-94462b8f3e4e@std.uestc.edu.cn/

> diff --git a/net/core/filter.c b/net/core/filter.c
> index fcfcb72663ca3..eae2f2c542716 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5833,6 +5833,12 @@ BPF_CALL_5(bpf_sock_ops_setsockopt, struct bpf_sock_ops_kern *, bpf_sock,
>  	if (!is_locked_tcp_sock_ops(bpf_sock))
>  		return -EOPNOTSUPP;
>  
> +	/* TCP_NODELAY triggers tcp_push_pending_frames() and re-enters these callbacks. */
> +	if ((bpf_sock->op == BPF_SOCK_OPS_HDR_OPT_LEN_CB ||
> +	     bpf_sock->op == BPF_SOCK_OPS_WRITE_HDR_OPT_CB) &&
> +	    level == SOL_TCP && optname == TCP_NODELAY)
> +		return -EOPNOTSUPP;

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?

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?

> +
>  	return _bpf_setsockopt(bpf_sock->sk, level, optname, optval, optlen);
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260417092035.2299913-1-kafai.wan@linux.dev?part=1