netdev - Re: [PATCH mptcp] mptcp: sockopt: make sync_socket_options propagate SOCK

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <802a2629b135d7f69f42f7bac5a5350374ed2570.camel@kernel.org>
Date: Sat, 06 Sep 2025 09:29:01 +0800
From: Geliang Tang <geliang@...nel.org>
To: Krister Johansen <kjlx@...pleofstupid.com>, Matthieu Baerts
	 <matttbe@...nel.org>, Mat Martineau <martineau@...nel.org>
Cc: "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>, Florian Westphal
 <fw@...len.de>, netdev@...r.kernel.org, 	mptcp@...ts.linux.dev,
 linux-kernel@...r.kernel.org, David Reaver	 <me@...idreaver.com>
Subject: Re: [PATCH mptcp] mptcp: sockopt: make sync_socket_options
 propagate SOCK_KEEPOPEN

Hi Krister,

On Fri, 2025-09-05 at 17:43 -0700, Krister Johansen wrote:
> Users reported a scenario where MPTCP connections that were
> configured
> with SO_KEEPALIVE prior to connect would fail to enable their
> keepalives
> if MTPCP fell back to TCP mode.
> 
> After investigating, this affects keepalives for any connection where
> sync_socket_options is called on a socket that is in the closed or
> listening state.  Joins are handled properly. For connects,
> sync_socket_options is called when the socket is still in the closed
> state.  The tcp_set_keepalive() function does not act on sockets that
> are closed or listening, hence keepalive is not immediately enabled.
> Since the SO_KEEPOPEN flag is absent, it is not enabled later in the
> connect sequence via tcp_finish_connect.  Setting the keepalive via
> sockopt after connect does work, but would not address any
> subsequently
> created flows.
> 
> Fortunately, the fix here is straight-forward: set SOCK_KEEPOPEN on
> the
> subflow when calling sync_socket_options.
> 
> The fix was valdidated both by using tcpdump to observe keeplaive
> packets not being sent before the fix, and being sent after the fix. 
> It
> was also possible to observe via ss that the keepalive timer was not
> enabled on these sockets before the fix, but was enabled afterwards.
> 
> Fixes: 1b3e7ede1365 ("mptcp: setsockopt: handle SO_KEEPALIVE and
> SO_PRIORITY")
> Cc: stable@...r.kernel.org
> Signed-off-by: Krister Johansen <kjlx@...pleofstupid.com>

Thanks for this fix. Good catch!

Reviewed-by: Geliang Tang <geliang@...nel.org>

-Geliang

> ---
>  net/mptcp/sockopt.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index 2c267aff95be..13108e9f982b 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -1532,13 +1532,11 @@ static void sync_socket_options(struct
> mptcp_sock *msk, struct sock *ssk)
>  {
>  	static const unsigned int tx_rx_locks = SOCK_RCVBUF_LOCK |
> SOCK_SNDBUF_LOCK;
>  	struct sock *sk = (struct sock *)msk;
> +	int kaval = !!sock_flag(sk, SOCK_KEEPOPEN);
>  
> -	if (ssk->sk_prot->keepalive) {
> -		if (sock_flag(sk, SOCK_KEEPOPEN))
> -			ssk->sk_prot->keepalive(ssk, 1);
> -		else
> -			ssk->sk_prot->keepalive(ssk, 0);
> -	}
> +	if (ssk->sk_prot->keepalive)
> +		ssk->sk_prot->keepalive(ssk, kaval);
> +	sock_valbool_flag(ssk, SOCK_KEEPOPEN, kaval);
>  
>  	ssk->sk_priority = sk->sk_priority;
>  	ssk->sk_bound_dev_if = sk->sk_bound_dev_if;
> 
> base-commit: 319f7385f22c85618235ab0169b80092fa3c7696