Re: [PATCH mptcp-next v5 09/16] mptcp: implement read_skb for sockmap stream verdict

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mptcp_read_skb ignores MPTCP_SKB_CB(skb)->offset, passing already-consumed or overlapping data to the BPF verdict program.
- [High] mptcp_read_skb delegates SKB consumption to the BPF framework, but the framework's cleanup routine (tcp_eat_skb) ignores MPTCP sockets, causing a permanent receive window stall.
--

commit 6e8107704d63a143d654e959c9d367d27969a1b4
Author: Geliang Tang <geliang@kernel.org>

mptcp: implement read_skb for sockmap stream verdict

This commit implements the read_skb callback for MPTCP stream operations,
allowing MPTCP sockets to function with BPF sockmap stream verdict programs.
It adds mptcp_read_skb() which peeks, unlinks, and hands skbs to the
receive actor.

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c

[ ... ]

> @@ -4801,6 +4801,31 @@ static ssize_t mptcp_splice_read(struct socket *sock, loff_t *ppos,
> +static int mptcp_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
> +{
> +	struct sk_buff *skb;
> +	int copied = 0;
> +
> +	if (sk->sk_state == TCP_LISTEN)
> +		return -ENOTCONN;
> +
> +	while ((skb = skb_peek(&sk->sk_receive_queue)) != NULL) {
> +		int used;
> +
> +		__skb_unlink(skb, &sk->sk_receive_queue);
> +		WARN_ON_ONCE(!skb_set_owner_sk_safe(skb, sk));
> +		used = recv_actor(sk, skb);

[Severity: High]
Does this expose stale or overlapping data to the BPF stream verdict program?

It looks like the SKB is handed off to the BPF framework without first
pulling or accounting for MPTCP_SKB_CB(skb)->offset. If the SKB has a
non-zero offset (e.g., due to overlapping sequences handled by MPTCP),
passing it directly to recv_actor() without calling skb_pull() or adjusting
skb->data could allow the verdict program to read and redirect bytes that
precede the valid payload.

[Severity: High]
Will this cause a permanent receive window stall for MPTCP sockets?

When the BPF verdict is SK_REDIRECT or SK_DROP, the framework calls
tcp_eat_skb() to advance sequences and update the receive window. However,
tcp_eat_skb() checks sk_is_tcp():

net/ipv4/tcp_bpf.c:tcp_eat_skb() {
    if (!skb || !skb->len || !sk_is_tcp(sk))
        return;
    ...
}

Since sk_is_tcp() returns false for MPTCP sockets (IPPROTO_MPTCP),
tcp_eat_skb() will return early. This means the consumed bytes are never
advanced and mptcp_cleanup_rbuf() is never invoked, which appears to cause
the MPTCP receive window to permanently close.

> +		if (used < 0) {
> +			if (!copied)
> +				copied = used;
> +			break;
> +		}
> +		copied += used;
> +	}
> +
> +	return copied;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789294365.git.tanggeliang@kylinos.cn?part=9