Re: [PATCH mptcp-net 02/34] mptcp: drain the queues of a listen() that fails

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] mptcp: missing inuse add and state reset in
  mptcp_pm_nl_create_listen_socket() [1]
--

commit 2094b28eb838bd9f43ac4fd932bbfb028ce12096
Author: Chris Mason <mason@kernel.org>

mptcp: drain the queues of a listen() that fails

This commit cleans up the subflow accept queue and Fast Open RST queue in
mptcp_listen() when __inet_listen_sk() fails and leaves the subflow in
TCP_CLOSE.

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 50de5cedfd0fb..dd9fb803cad5f 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -4381,6 +4381,11 @@ static int mptcp_listen(struct socket *sock, int backlog)
>  
>  	lock_sock(ssk);
>  	err = __inet_listen_sk(ssk, backlog);
> +	if (err && ssk->sk_state == TCP_CLOSE) {
> +		/* inet_csk_listen_start() leaves this drain to MPTCP */
> +		mptcp_subflow_queue_clean(sk, ssk);
> +		inet_csk_listen_stop(ssk);
> +	}
>  	release_sock(ssk);
>  	mptcp_set_state(sk, inet_sk_state_load(ssk));
>  

[Severity: High]
This problem wasn't introduced by this patch, but does the other
__inet_listen_sk() caller in mptcp_pm_nl_create_listen_socket() [1]
miss updating newsk's state and sock_prot_inuse_add()?

net/mptcp/pm_kernel.c:mptcp_pm_nl_create_listen_socket() {
	...
	inet_sk_state_store(newsk, TCP_LISTEN);
	sock_set_flag(newsk, SOCK_RCU_FREE);
	lock_sock(ssk);
	WRITE_ONCE(mptcp_subflow_ctx(ssk)->pm_listener, true);
	err = __inet_listen_sk(ssk, backlog);
	if (!err)
		mptcp_event_pm_listener(ssk, MPTCP_EVENT_LISTENER_CREATED);
	release_sock(ssk);
	return err;
}

Unlike mptcp_listen(), mptcp_pm_nl_create_listen_socket() does not call
sock_prot_inuse_add() on success or reset newsk from ssk's state when
__inet_listen_sk() fails.  When the PM listener socket is later closed via
__mptcp_pm_release_addr_entry() -> __mptcp_close() ->
mptcp_check_listen_stop(), seeing newsk in TCP_LISTEN decrements the
per-netns protocol in-use counter without a matching increment and triggers
WARN_ON_ONCE(inet_sk_state_load(ssk) != TCP_LISTEN) if __inet_listen_sk()
failed and left ssk in TCP_CLOSE.

[1] https://sashiko.dev/#/bug/linux-2c383603-dae6-4f8b-a0ea-d210d6d43b44

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-mptcp-cmason-fixes-v1-0-9205b77e9a2f@kernel.org?part=2