* Re: [PATCH net v2 06/15] mptcp: prevent race between disconnect() and rtx
[not found] <20260908-net-mptcp-misc-fixes-7-3-rc1-v2-6-df1de70348b6@kernel.org>
@ 2026-09-09 14:49 ` netdev-bot+sashiko
2026-09-09 15:54 ` Paolo Abeni
0 siblings, 1 reply; 2+ messages in thread
From: netdev-bot+sashiko @ 2026-09-09 14:49 UTC (permalink / raw)
To: matttbe
Cc: martineau, geliang, davem, edumazet, kuba, pabeni, horms, netdev,
mptcp, linux-kernel, stable
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 0 · Low: 2
- [Low] The commit message of "mptcp: prevent race between disconnect()
and rtx" describes the change as only "Prevent the race using the…
- [Low] The commit carries "Fixes: b29fcfb54cd7 (\"mptcp: full disconnect
implementation\")" (v5.18) for a real race that can leave a reused msk…
--
mptcp: prevent race between disconnect() and rtx
This replaces the mptcp_stop_rtx_timer() and mptcp_stop_tout_timer() calls in
mptcp_disconnect() with open-coded sk_stop_timer_sync() calls, so a
concurrently running retransmit timer cannot resurrect state after the
disconnect path has reset it. No Link: or Closes: tags are present in the
commit header.
A couple of questions below, one about the changelog and one about the tags.
> mptcp: prevent race between disconnect() and rtx
>
> Sashiko noted that the two event can race, leading to inconsistent
> status. Prevent the race using the synchronous timer stop operation.
>
> Fixes: b29fcfb54cd7 ("mptcp: full disconnect implementation")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
[Severity: Low]
The Fixes: tag points at b29fcfb54cd7, which went in during v5.18, and the
race described can leave a reused msk with MPTCP_RETRANSMIT / MPTCP_WORK_RTX
set after mptcp_disconnect() has already cleared msk->flags and msk->cb_flags.
Should this also carry Cc: stable@vger.kernel.org? Most of the other fixes in
this series do, including "mptcp: do not reschedule the RTX timer for fallback
sockets", whose MPTCP_RTX_ENABLED bit this change relies on so the timer cannot
be re-armed once the socket is in TCP_CLOSE.
Also, the report is credited to "Sashiko" in prose only. Would it be possible
to add the matching Closes: or Link: tag, the way the sibling commits in the
series do?
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index be59651e708e..d611af2eb74f 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -3588,6 +3588,7 @@ static void mptcp_destroy_common(struct mptcp_sock *msk)
>
> static int mptcp_disconnect(struct sock *sk, int flags)
> {
> + struct inet_connection_sock *icsk = inet_csk(sk);
> struct mptcp_sock *msk = mptcp_sk(sk);
>
> /* We are on the fastopen error path. We can't call straight into the
> @@ -3600,8 +3601,13 @@ static int mptcp_disconnect(struct sock *sk, int flags)
> mptcp_check_listen_stop(sk);
> mptcp_set_state(sk, TCP_CLOSE);
>
> - mptcp_stop_rtx_timer(sk);
> - mptcp_stop_tout_timer(sk);
> + /* The later subflow close can not kick again the tout timer,
> + * as the msk is already in closed status.
> + */
> + msk->timer_ival = icsk->icsk_rto_min;
> + sk_stop_timer_sync(sk, &sk->mptcp_retransmit_timer);
> + icsk->icsk_mtup.probe_timestamp = 0;
> + sk_stop_timer_sync(sk, &icsk->mptcp_tout_timer);
[Severity: Low]
The changelog says the race is prevented "using the synchronous timer stop
operation", which reads like a straight sk_stop_timer() to sk_stop_timer_sync()
swap, but the open-coded version in mptcp_disconnect() also changes two other
pieces of state. Could the message mention them?
First, msk->timer_ival now ends up as icsk->icsk_rto_min, while the helper it
replaces sets it to zero:
net/mptcp/protocol.c:mptcp_stop_rtx_timer() {
sk_stop_timer(sk, &sk->mptcp_retransmit_timer);
mptcp_sk(sk)->timer_ival = 0;
}
The new value is what __mptcp_init_sock() seeds, so it looks intentional and
harmless, and the only reader, mptcp_reset_rtx_timer(), is gated on
MPTCP_RTX_ENABLED which mptcp_set_state(TCP_CLOSE) has just cleared. Is that
the reasoning?
Second, the tout timer is now stopped and probe_timestamp cleared
unconditionally, whereas the replaced helper bailed out early when
probe_timestamp was already zero:
net/mptcp/protocol.h:mptcp_stop_tout_timer() {
if (!inet_csk(sk)->icsk_mtup.probe_timestamp)
return;
sk_stop_timer(sk, &inet_csk(sk)->mptcp_tout_timer);
inet_csk(sk)->icsk_mtup.probe_timestamp = 0;
}
That early return means a tout timer armed only for an MP_FAIL timeout, where
mptcp_reset_tout_timer() left probe_timestamp at zero, stayed pending across
disconnect(). Is that a separate fix folded in here, and would it be worth
calling out in the changelog so stable backporters can see it?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-net-mptcp-misc-fixes-7-3-rc1-v2-0-df1de70348b6%40kernel.org
^ permalink raw reply [flat|nested] 2+ messages in thread