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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f046fdda-3bad-4f7f-8587-dca30d183f82@kernel.org>
Date: Tue, 14 Oct 2025 17:27:41 +0200
From: Matthieu Baerts <matttbe@...nel.org>
To: Jiayuan Chen <jiayuan.chen@...ux.dev>
Cc: Mat Martineau <martineau@...nel.org>, Geliang Tang <geliang@...nel.org>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, Davide Caratti <dcaratti@...hat.com>,
 netdev@...r.kernel.org, mptcp@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v1] mptcp: fix incorrect IPv4/IPv6 check

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@...ux.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.