From: Matthieu Baerts <matttbe@kernel.org>
To: Tao Cui <cui.tao@linux.dev>, sashiko-reviews@lists.linux.dev
Cc: mptcp@lists.linux.dev
Subject: Re: [PATCH mptcp-next 2/2] mptcp: pm: skip extra_subflows accounting on disconnected msk
Date: Wed, 2 Sep 2026 18:32:28 +0200 [thread overview]
Message-ID: <f6c30ebb-5c79-4998-bcf5-3b4d95aa0ab8@kernel.org> (raw)
In-Reply-To: <739848e0-1d35-4ed7-afd7-4b0609ddb8d0@linux.dev>
Hi Tao,
On 31/08/2026 15:00, Tao Cui wrote:
> Hi,
>
> Thanks for the review.
>
> 在 2026/8/31 18:11, sashiko-bot@kernel.org 写道:
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>
>> Pre-existing issues:
>> - [High] Subflows in `join_list` can be leaked due to unsynchronized zeroing of `cb_flags` in `mptcp_disconnect`.
>> - [High] Data race on `pm->reset` and `pm->rm_list_*` fields due to missing `pm->lock` in `mptcp_pm_data_reset()`.
>
> Both findings are pre-existing issues rather
> than regressions from this series, but I had a look at them:
>
> For the join_list leak, I agree it is real. When an MP_JOIN is
> processed while the parent socket is being disconnected, the subflow
> is queued in msk->join_list and MPTCP_FLUSH_JOIN_LIST is set under
> the data lock, but mptcp_disconnect() then clears msk->cb_flags with
> a plain write. The flag is lost, mptcp_release_cb() never flushes
> the list, and since mptcp_destroy_common() only iterates conn_list,
> those subflow sockets are leaked. The plain write also drops flags
> that were already pending before the disconnect, so taking the data
> lock around it is not enough. What I have in mind is to keep the
> flush-related bits across the disconnect and let release_sock()
> process the join list as originally intended, or to drain it
> explicitly in the disconnect path, checking the lock nesting
> commented in mptcp_destroy_common().
>
> For the pm->reset data race, the lasting effect is the counter
> imbalance this series now clamps and traces, and the other fields
> are discarded on a closed socket anyway. The direct fix is to take
> pm->lock around the reset in mptcp_pm_data_reset(), after checking
> the lock ordering on all callers.
>
> These are two separate bugs with separate Fixes: targets, so I plan
> to submit them as their own series rather than folding them into
> this one.
Thank you! Yes a different series is better.
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
next prev parent reply other threads:[~2026-09-02 16:32 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-31 9:32 [PATCH mptcp-next 0/2] mptcp: pm: fix reachable extra_subflows guards Tao Cui
2026-08-31 9:32 ` [PATCH mptcp-next 1/2] mptcp: pm: bound extra_subflows admission on userspace PM Tao Cui
2026-09-02 16:39 ` Matthieu Baerts
2026-09-03 3:42 ` quanyeyang
2026-09-03 8:57 ` Tao Cui
2026-09-03 8:55 ` Tao Cui
2026-09-03 9:13 ` Matthieu Baerts
2026-08-31 9:32 ` [PATCH mptcp-next 2/2] mptcp: pm: skip extra_subflows accounting on disconnected msk Tao Cui
2026-08-31 10:11 ` sashiko-bot
2026-08-31 13:00 ` Tao Cui
2026-09-02 16:32 ` Matthieu Baerts [this message]
2026-09-02 16:56 ` Matthieu Baerts
2026-08-31 10:25 ` [PATCH mptcp-next 0/2] mptcp: pm: fix reachable extra_subflows guards MPTCP CI
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f6c30ebb-5c79-4998-bcf5-3b4d95aa0ab8@kernel.org \
--to=matttbe@kernel.org \
--cc=cui.tao@linux.dev \
--cc=mptcp@lists.linux.dev \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox