Re: [PATCH 6.1.y 1/3] mptcp: consolidate subflow cleanup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mptcp: socket hang in fallback teardown due to restrictive state checks
--

commit 56f82de860d8f3b2e8b76eca945e8a61ec848e3a
Author: Paolo Abeni <pabeni@redhat.com>

mptcp: consolidate subflow cleanup

This commit consolidates subflow cleanup actions requiring the worker into a
single helper. It ensures the dummy data FIN creation for fallback sockets is
performed only when the TCP receive queue is empty.

Link: https://patch.msgid.link/20250218-net-next-mptcp-rx-path-refactor-v1-1-4a47d90d7998@kernel.org

> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index d9734bfb7283b..9bbeed6290d7a 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c

[ ... ]

> @@ -1169,8 +1174,18 @@ static void subflow_sched_work_if_closed(struct mptcp_sock *msk, struct sock *ss
>  		    inet_sk_state_load(sk) != TCP_ESTABLISHED)))
>  		return;
>  
> -	if (skb_queue_empty(&ssk->sk_receive_queue) &&
> -	    !test_and_set_bit(MPTCP_WORK_CLOSE_SUBFLOW, &msk->flags))
> +	if (!skb_queue_empty(&ssk->sk_receive_queue))
> +		return;
> +
> +	if (!test_and_set_bit(MPTCP_WORK_CLOSE_SUBFLOW, &msk->flags))
> +		mptcp_schedule_work(sk);
> +
> +	/* when the fallback subflow closes the rx side, trigger a 'dummy'
> +	 * ingress data fin, so that the msk state will follow along
> +	 */
> +	if (__mptcp_check_fallback(msk) && subflow_is_done(ssk) &&
> +	    msk->first == ssk &&
> +	    mptcp_update_rcv_data_fin(msk, READ_ONCE(msk->ack_seq), true))
>  		mptcp_schedule_work(sk);
>  }

[Severity: High]
Does this new placement of the fallback dummy FIN logic cause a socket hang
during connection teardown?

In the previous implementation, the dummy FIN injection was executed in
subflow_state_change() regardless of the early returns in
subflow_sched_work_if_closed().

By moving it inside subflow_sched_work_if_closed(), it is now gated by the
restrictive state checks at the top of the function:

	if (likely(ssk->sk_state != TCP_CLOSE &&
		   (ssk->sk_state != TCP_CLOSE_WAIT ||
		    inet_sk_state_load(sk) != TCP_ESTABLISHED)))
		return;

When a local application closes a fallback connection, the msk transitions
to TCP_FIN_WAIT1 or TCP_FIN_WAIT2. If the remote peer then sends a FIN, the
underlying TCP subflow (ssk) transitions to TCP_CLOSING or TCP_TIME_WAIT.
Since the ssk is neither in TCP_CLOSE nor TCP_CLOSE_WAIT, the function
returns early.

Will this skip the fallback dummy FIN injection entirely, leaving the msk
state machine stuck without mptcp_update_rcv_data_fin() and causing
fallback MPTCP sockets to hang indefinitely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919201252.2025112-5-matttbe@kernel.org?part=1