Re: [PATCH net-next v1] mptcp: fix incorrect IPv4/IPv6 check

[Matthieu Baerts <matttbe@kernel.org>]

Hi Jiayuan,

Thank you for sharing this patch!

On 14/10/2025 14:26, Jiayuan Chen wrote:
> When MPTCP falls back to normal TCP, it needs to reset proto_ops. However,
> for sockmap and TLS, they have their own custom proto_ops, so simply
> checking sk->sk_prot is insufficient.
> 
> For example, an IPv6 request might incorrectly follow the IPv4 code path,
> leading to kernel panic.

Did you experiment issues, or is it a supposition? If yes, do you have
traces containing such panics (or just a WARN()?), and ideally the
userspace code that was leading to this?

What is unclear to me is how you got an MPTCP + TLS + sockmap socket.
And if yes, can we set sk_socket->ops to inet(6)_stream_ops and nothing
else without having any other issues?

And do we maybe have to update some code in subflow.c also looking at
sk->sk_prot? I guess no because there, the socket is created by MPTCP,
and it should be set to tcp(v6)_prot. Except if there is some BPF code
that can change that?

> Note that Golang has enabled MPTCP by default [1]
> 
> [1] https://go-review.googlesource.com/c/go/+/607715
> 
> Fixes: 8e2b8a9fa512 ("mptcp: don't overwrite sock_ops in mptcp_is_tcpsk()")
If I understand the issue correctly, was it not present from the
beginning, before the mentioned commit?

> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
>  net/mptcp/protocol.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 0292162a14ee..efcdaeff91f8 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -62,10 +62,10 @@ static u64 mptcp_wnd_end(const struct mptcp_sock *msk)
>  static const struct proto_ops *mptcp_fallback_tcp_ops(const struct sock *sk)
>  {
>  #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> -	if (sk->sk_prot == &tcpv6_prot)
> +	if (sk->sk_family == AF_INET6)

sk_prot was proving it was a TCP + IPv4/6 socket, and then that's OK to
set inet(6)_stream_ops. I guess we could only check the family, but, can
we always return inet(6)_stream_ops no matter what sk->sk_prot is?

If the protocol has been modified, the stream one has maybe been
modified too, no?

>  		return &inet6_stream_ops;
>  #endif
> -	WARN_ON_ONCE(sk->sk_prot != &tcp_prot);
> +	WARN_ON(sk->sk_family != AF_INET);

Please keep the WARN_ON_ONCE().

Maybe we should not return inet_stream_ops in case the previous
condition was wrong, and not change sk_socket->ops.

>  	return &inet_stream_ops;
>  }

Note about the subject: if it is a fix for an older commit, it should
target 'net', not 'net-next' (+ cc stable). Can you also have a clearer
subject mentioning 'proto' and 'fallback' please?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.