Re: [PATCH bpf v2 09/15] bpf, sockmap: Check the TCP state in recvmsg()

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 11/2/26 3:21 PM, Alexei Starovoitov wrote:
> From: Chris Mason <mason@kernel.org>
>
> With an sk_skb verdict prog, tcp_bpf_recvmsg_parser() copies data from the
> psock. Then it calls __tcp_cleanup_rbuf() to calculate the receive window
> and to send an ACK. The window and the ACK need a connection on the socket.
> But tcp_disconnect() keeps the socket in the sockmap and the data on the
> psock. For a TCP socket sock_map_redirect_allowed() rejects only
> TCP_LISTEN. Thus the psock can also get data after tcp_disconnect().
>
> __tcp_select_window() divides by icsk_ack.rcv_mss, which can be 0 in
> TCP_CLOSE. In TCP_SYN_SENT the new peer can get an ACK with the sequence
> numbers of the old connection.
>
> Call __tcp_cleanup_rbuf() only when tcp_can_send_ack() returns true.
> recvmsg() returns the data as before. Move tcp_can_send_ack() from MPTCP to
> tcp.h. It also returns false for a fastopen child in TCP_SYN_RECV. From its
> peer the child has only the SYN data, and the SYN-ACK acknowledges it.
>
> Fixes: e5c6de5fa025 ("bpf, sockmap: Incorrectly handling copied_seq")
> Signed-off-by: Chris Mason <mason@kernel.org>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>


Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>


> ---
>  include/net/tcp.h    | 6 ++++++
>  net/ipv4/tcp_bpf.c   | 3 ++-
>  net/mptcp/protocol.c | 6 ------
>  3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 3560adddf821..23a22d70718e 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1845,6 +1845,12 @@ static inline void tcp_adjust_rcv_ssthresh(struct sock *sk)
>  void tcp_cleanup_rbuf(struct sock *sk, int copied);
>  void __tcp_cleanup_rbuf(struct sock *sk, int copied);
>  
> +/* Is the socket in a state in which TCP may send an ACK? */
> +static inline bool tcp_can_send_ack(const struct sock *sk)
> +{
> +	return !((1 << inet_sk_state_load(sk)) &
> +	       (TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_TIME_WAIT | TCPF_CLOSE | TCPF_LISTEN));
> +}
>  
>  /* We provision sk_rcvbuf around 200% of sk_rcvlowat.
>   * If 87.5 % (7/8) of the space has been consumed, we want to override
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index ef2e15de0fe3..3d703f1517d8 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -343,7 +343,8 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
>  	if (!peek)
>  		WRITE_ONCE(tcp->copied_seq, seq);
>  	tcp_rcv_space_adjust(sk);
> -	if (copied > 0)
> +	/* Data on the psock outlives the connection that it came in on */
> +	if (copied > 0 && tcp_can_send_ack(sk))
>  		__tcp_cleanup_rbuf(sk, copied);
>  
>  unlock:
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 7a22e9388e0e..d62cfbf2d490 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -619,12 +619,6 @@ static void mptcp_set_timeout(struct sock *sk)
>  	__mptcp_set_timeout(sk, tout);
>  }
>  
> -static inline bool tcp_can_send_ack(const struct sock *ssk)
> -{
> -	return !((1 << inet_sk_state_load(ssk)) &
> -	       (TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_TIME_WAIT | TCPF_CLOSE | TCPF_LISTEN));
> -}
> -
>  void __mptcp_subflow_send_ack(struct sock *ssk)
>  {
>  	if (tcp_can_send_ack(ssk))