Re: [PATCH 5.10.y 3/3] mptcp: close race between scheduler and state change

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mptcp: skipped timer reset when mss_now is 0 leads to connection stall
- [High] mptcp: allow_infinite_fallback state corruption when mss_now is 0
--

commit 4c31a1df5ba815467dd012d7f0c9b7bc2457aca9
Author: Paolo Abeni <pabeni@redhat.com>

mptcp: close race between scheduler and state change

This commit adds a check for mss_now being 0 to prevent a divide by zero
operation when selecting a subflow socket. It modifies mptcp_sendmsg and
mptcp_worker to only trigger tcp_push if both copied and mss_now are non-zero.

Link: https://patch.msgid.link/20260917-net-mptcp-misc-fixes-7-3-rc4-v2-2-0cf5c72667c8@kernel.org

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 292c21713eb7f..fdebc451e23aa 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1381,7 +1381,7 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
>  	}
>  
>  	mptcp_set_timeout(sk, ssk);
> -	if (copied) {
> +	if (copied && mss_now) {
>  		tcp_push(ssk, msg->msg_flags, mss_now, tcp_sk(ssk)->nonagle,
>  			 size_goal);
>  
>  		/* start the timer, if it's not pending */
>  		if (!mptcp_timer_pending(sk))
>  			mptcp_reset_timer(sk);
>  	}

[Severity: High]
Does this unintentionally skip resetting the retransmit timer when mss_now
is 0?

If multiple fragments are sent and a subsequent fragment fails, it can leave
copied > 0 but mss_now == 0. By bundling the timer initialization with the
mss_now check, this might prevent data queued to the MPTCP socket from being
retransmitted if the current subflow fails, potentially stalling the
connection.

[ ... ]

> @@ -1891,7 +1891,7 @@ static void mptcp_worker(struct work_struct *work)
>  		if (!mptcp_ext_cache_refill(msk))
>  			break;
>  	}
> -	if (copied) {
> +	if (copied && mss_now) {
>  		tcp_push(ssk, msg.msg_flags, mss_now, tcp_sk(ssk)->nonagle,
>  			 size_goal);
>  		WRITE_ONCE(msk->allow_infinite_fallback, false);
>  	}

[Severity: High]
Could skipping this block when mss_now is 0 cause allow_infinite_fallback
state corruption?

In mptcp_worker(), if a subflow failure occurs during retransmission causing
mss_now == 0, copied might still be > 0. Skipping the update to
msk->allow_infinite_fallback could leave it as true even after MPTCP data
was successfully queued. A subsequent error might then trigger an invalid
fallback to plain TCP, corrupting the connection state and stream data.

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