net/mptcp/fastopen.c | 2 +- net/mptcp/options.c | 9 +++++++++ net/mptcp/protocol.h | 4 ++-- 3 files changed, 12 insertions(+), 3 deletions(-)
With TCP Fast Open on an MPTCP listener, if the server application
writes data while the passive subflow is still in SYN_RECV (after
consuming the client's SYN data but before the MP_CAPABLE third ACK
arrives), __mptcp_subflow_active() refuses transmission and the data
is queued into the msk write queue.
When the MPC third ACK arrives, the subflow transitions to
TCP_ESTABLISHED and is marked fully established in
check_fully_established(), but because the third ACK carries no
DSS data, the queued bytes remain stranded until the peer sends
more data.
Fix this in check_fully_established() by checking if the subflow
was doing passive TFO (subflow->is_mptfo). Under mptcp_data_lock(),
flush queued bytes via __mptcp_check_push() and clear is_mptfo.
Also move is_mptfo from the shared u32 bitfield to a standalone
bool in struct mptcp_subflow_context to avoid any read-modify-write
data races with concurrent bitfield modifications (e.g.
close_event_done in mptcp_close_ssk()).
Reported-by: Petar Sakic <petar.sakic@ink.fish>
Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/
Fixes: fb7084501a61 ("mptcp: add support for TCP_FASTOPEN sockopt")
Assisted-by: Gemini 3.7
Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>
---
net/mptcp/fastopen.c | 2 +-
net/mptcp/options.c | 9 +++++++++
net/mptcp/protocol.h | 4 ++--
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index f71775090..c11fc3c5c 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -29,7 +29,7 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
if (!skb)
return;
- subflow->is_mptfo = 1;
+ subflow->is_mptfo = true;
/* dequeue the skb from sk receive queue */
__skb_unlink(skb, &ssk->sk_receive_queue);
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index ce0de02f5..7ea97b283 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1042,6 +1042,15 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
mptcp_data_lock((struct sock *)msk);
__mptcp_subflow_fully_established(msk, subflow, mp_opt);
+ /* Passive TFO: the application may have written data while the
+ * subflow was still in SYN_RECV; __mptcp_subflow_active() refused
+ * it then and nothing else spools the msk write queue when the
+ * MPC third ack (no DSS) arrives. Push it now.
+ */
+ if (subflow->is_mptfo) {
+ subflow->is_mptfo = false;
+ __mptcp_check_push((struct sock *)msk, ssk);
+ }
mptcp_data_unlock((struct sock *)msk);
check_notify:
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 0384d6a02..0e44de129 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -582,15 +582,15 @@ struct mptcp_subflow_context {
disposable : 1, /* ctx can be free at ulp release time */
closing : 1, /* must not pass rx data to msk anymore */
valid_csum_seen : 1, /* at least one csum validated */
- is_mptfo : 1, /* subflow is doing TFO */
close_event_done : 1, /* has done the post-closed part */
mpc_drop : 1, /* the MPC option has been dropped in a rtx */
resetting : 1, /* subflow is resetting */
- __unused : 8;
+ __unused : 9;
bool data_avail;
bool scheduled;
bool pm_listener; /* a listener managed by the kernel PM? */
bool fully_established; /* path validated */
+ bool is_mptfo; /* subflow is doing TFO */
u32 lent_mem_frag;
u32 remote_nonce;
u64 thmac;
--
2.34.1Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 1 · Low: 2
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The fix is incomplete: data-less passive TFO subflows are still
left with stranded data.
- [Medium] Sibling path left unfixed: fallback on the third ACK.
- [Low] The Fixes: tag 'fb7084501a61 ("mptcp: add support for
TCP_FASTOPEN sockopt")' points to a SHA that does not exist in the…
- [Low] The commit author is 'T S Rameshkumar <rameshsv06@gmail.com>',
but the only Signed-off-by is 'T S Rameshkumar…
--
mptcp: push queued data on passive TFO subflows becoming established
When a passive TFO subflow with is_mptfo set becomes fully established,
this patch pushes the msk write queue from check_fully_established().
Without that push, data written while the subflow was still in SYN_RECV
stays stranded. The patch also moves is_mptfo out of the shared u32
bitfield and makes it a standalone bool.
Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/
> Fixes: fb7084501a61 ("mptcp: add support for TCP_FASTOPEN sockopt")
[Severity: Low]
Is this Fixes: tag correct? fb7084501a61 doesn't resolve to a commit in
the tree.
By subject, the closest match is 4ffb0a02346c ("mptcp: add TCP_FASTOPEN
sock option"). The passive TFO path changed here, including is_mptfo and
mptcp_fastopen_subflow_synack_set_params(), came from:
36b122baf6a8 ("mptcp: add subflow_v(4,6)_send_synack()")
The closely related fix e00b63056fb4 ("mptcp: fastopen: only mark MPTFO
subflows with SYN data") also uses 36b122baf6a8 as its Fixes: target.
Should this be:
Fixes: 36b122baf6a8 ("mptcp: add subflow_v(4,6)_send_synack()")
> Assisted-by: Gemini 3.7
> Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>
[Severity: Low]
The author is rameshsv06@gmail.com, but the only Signed-off-by uses
rameshkumar.t@phytecembedded.in. checkpatch reports this as a
"From:/Signed-off-by: email address mismatch".
Could the author and Signed-off-by identities be aligned? The patch also
carries an Assisted-by: tag, so a matching DCO sign-off from the author
is worth having.
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index ce0de02f5a3a1..7ea97b283fda1 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -1042,6 +1042,15 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
>
> mptcp_data_lock((struct sock *)msk);
> __mptcp_subflow_fully_established(msk, subflow, mp_opt);
> + /* Passive TFO: the application may have written data while the
> + * subflow was still in SYN_RECV; __mptcp_subflow_active() refused
> + * it then and nothing else spools the msk write queue when the
> + * MPC third ack (no DSS) arrives. Push it now.
> + */
> + if (subflow->is_mptfo) {
> + subflow->is_mptfo = false;
> + __mptcp_check_push((struct sock *)msk, ssk);
> + }
[Severity: High]
Does this cover passive TFO children whose SYN carried no data?
Since e00b63056fb4 ("mptcp: fastopen: only mark MPTFO subflows with SYN
data"), mptcp_fastopen_subflow_synack_set_params() returns before setting
is_mptfo when the receive queue is empty:
net/mptcp/fastopen.c:mptcp_fastopen_subflow_synack_set_params() {
...
/* A valid TFO cookie does not guarantee SYN data. */
skb = skb_peek(&ssk->sk_receive_queue);
if (!skb)
return;
subflow->is_mptfo = true;
...
}
But tcp_try_fastopen() still creates a full Fast Open child for a
data-less SYN. This happens when the SYN has a valid cookie, or when the
listener has TFO_SERVER_COOKIE_NOT_REQD and the SYN carries a cookie
option:
net/ipv4/tcp_fastopen.c:tcp_try_fastopen() {
...
* Note: Data-less SYN with valid cookie is allowed to
* send data in SYN_RECV state.
*/
fastopen:
child = tcp_fastopen_create_child(sk, skb, req);
...
}
That child can be accept()ed while it is still in SYN_RECV.
mptcp_sk_clone_init() has already moved the msk to TCP_ESTABLISHED, so
mptcp_sendmsg() queues the data without waiting. Every push attempt is
then refused by __mptcp_subflow_active()->__tcp_can_send(), which only
allows TCPF_ESTABLISHED and TCPF_CLOSE_WAIT.
The sequence looks like this:
tcp_try_fastopen()
tcp_fastopen_create_child()
subflow_syn_recv_sock()->mptcp_sk_clone_init() /* msk ESTABLISHED */
subflow_prep_synack()
mptcp_fastopen_subflow_synack_set_params() /* is_mptfo stays false */
accept() + write() /* queued, ssk SYN_RECV */
mptcp_incoming_options() /* MPC third ACK */
check_fully_established() /* reaches set_fully_established */
When the MPC third ACK arrives, seq == ssn_offset + 1 because there was
no SYN data to offset. There is no DSS and MPC is present, so the code
reaches set_fully_established. At that point is_mptfo is false and
__mptcp_check_push() is never called.
Wouldn't the queued data then stay in the msk write queue until the peer
sends again? That is the same symptom the commit message describes.
Would it work better to gate this on something that covers every passive
Fast Open child? Options include tcp_sk(ssk)->syn_fastopen_child, or
just checking for pending data with mptcp_send_head(), rather than
is_mptfo.
> mptcp_data_unlock((struct sock *)msk);
>
> check_notify:
[Severity: Medium]
What happens when the in-sequence third ACK on a passive TFO subflow
arrives without an MP_CAPABLE option? That could happen if a middlebox
strips it or the peer leaves it out.
In that case check_fully_established() takes the earlier fallback
branch. It returns before reaching set_fully_established, so it skips
the new push:
if (!(mp_opt->suboptions & OPTIONS_MPTCP_MPC)) {
if (subflow->mp_join)
goto reset;
subflow->mp_capable = 0;
if (!mptcp_try_fallback(ssk, MPTCP_MIB_MPCAPABLEDATAFALLBACK)) {
...
}
return false;
}
mptcp_incoming_options() then returns without pushing:
if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
return sk->sk_state != TCP_CLOSE;
The fallback branch in mptcp_incoming_options() that does spool the
write queue is checked before check_fully_established():
if (__mptcp_check_fallback(msk)) {
...
mptcp_data_lock(subflow->conn);
if (sk_stream_memory_free(sk))
__mptcp_check_push(subflow->conn, sk);
So that branch only runs on the next incoming segment.
__mptcp_try_fallback() itself only updates msk->flags and does not
schedule a push.
In this fallback case, would the data written while the subflow was in
SYN_RECV also stay queued until the peer sends something else?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261008124220.213016-1-rameshkumar.t%40phytecembedded.in
Hi T S,
Thank you for the patch, but *please* stop sending multiple version the
same day without waiting for reviews. The patch traffic is already high:
if everybody sends so many versions like that, we (human) cannot deal
with that, plus it is unlikely the patch can be tested properly. Also,
please increment the patch version and include a changelog ("b4" can
help with that).
https://docs.kernel.org/process/maintainer-netdev.html
Also, if you send your patch to the MPTCP list only, please base your
patch on top of our "export" branch: this version doesn't apply on top
of it.
On 08/10/2026 14:42, T S Rameshkumar wrote:
> With TCP Fast Open on an MPTCP listener, if the server application
> writes data while the passive subflow is still in SYN_RECV (after
> consuming the client's SYN data but before the MP_CAPABLE third ACK
> arrives), __mptcp_subflow_active() refuses transmission and the data
> is queued into the msk write queue.
>
> When the MPC third ACK arrives, the subflow transitions to
> TCP_ESTABLISHED and is marked fully established in
> check_fully_established(), but because the third ACK carries no
> DSS data, the queued bytes remain stranded until the peer sends
> more data.
>
> Fix this in check_fully_established() by checking if the subflow
> was doing passive TFO (subflow->is_mptfo). Under mptcp_data_lock(),
> flush queued bytes via __mptcp_check_push() and clear is_mptfo.
Why do you need to clear is_mptfo?
If it is not needed, you probably don't need to move is_mptfo. Also, we
would go back to the version shared by Petar.
@Petar: are you working with T S? Because if you suggested the patch, we
should at least add a "Suggested-by" tag. Is it OK with the
attributions? (Next time, please send a formal patch to avoid any
conflicts here ;))
> Also move is_mptfo from the shared u32 bitfield to a standalone
> bool in struct mptcp_subflow_context to avoid any read-modify-write
> data races with concurrent bitfield modifications (e.g.
> close_event_done in mptcp_close_ssk()).
>
> Reported-by: Petar Sakic <petar.sakic@ink.fish>
> Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/
> Fixes: fb7084501a61 ("mptcp: add support for TCP_FASTOPEN sockopt")
> Assisted-by: Gemini 3.7
> Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>
> ---
> net/mptcp/fastopen.c | 2 +-
> net/mptcp/options.c | 9 +++++++++
> net/mptcp/protocol.h | 4 ++--
> 3 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
> index f71775090..c11fc3c5c 100644
> --- a/net/mptcp/fastopen.c
> +++ b/net/mptcp/fastopen.c
> @@ -29,7 +29,7 @@ void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subf
> if (!skb)
> return;
>
> - subflow->is_mptfo = 1;
> + subflow->is_mptfo = true;
>
> /* dequeue the skb from sk receive queue */
> __skb_unlink(skb, &ssk->sk_receive_queue);
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index ce0de02f5..7ea97b283 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -1042,6 +1042,15 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
>
> mptcp_data_lock((struct sock *)msk);
> __mptcp_subflow_fully_established(msk, subflow, mp_opt);
> + /* Passive TFO: the application may have written data while the
> + * subflow was still in SYN_RECV; __mptcp_subflow_active() refused
> + * it then and nothing else spools the msk write queue when the
> + * MPC third ack (no DSS) arrives. Push it now.
> + */
> + if (subflow->is_mptfo) {
> + subflow->is_mptfo = false;
> + __mptcp_check_push((struct sock *)msk, ssk);
> + }
> mptcp_data_unlock((struct sock *)msk);
>
> check_notify:
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 0384d6a02..0e44de129 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -582,15 +582,15 @@ struct mptcp_subflow_context {
> disposable : 1, /* ctx can be free at ulp release time */
> closing : 1, /* must not pass rx data to msk anymore */
> valid_csum_seen : 1, /* at least one csum validated */
> - is_mptfo : 1, /* subflow is doing TFO */
> close_event_done : 1, /* has done the post-closed part */
> mpc_drop : 1, /* the MPC option has been dropped in a rtx */
> resetting : 1, /* subflow is resetting */
> - __unused : 8;
> + __unused : 9;
> bool data_avail;
> bool scheduled;
> bool pm_listener; /* a listener managed by the kernel PM? */
> bool fully_established; /* path validated */
> + bool is_mptfo; /* subflow is doing TFO */
> u32 lent_mem_frag;
> u32 remote_nonce;
> u64 thmac;
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mptcp: data race in mptcp_subflow_context bitfield on close_event_done
--
commit 5833b858d2930ce8da576be275d9bd667c3816cd
Author: T S Rameshkumar <rameshsv06@gmail.com>
mptcp: push queued data on passive TFO subflows becoming established
With TCP Fast Open on an MPTCP listener, if the server application
writes data while the passive subflow is still in SYN_RECV,
__mptcp_subflow_active() refuses transmission and the data is queued
into the msk write queue.
Closes: https://lore.kernel.org/netdev/CAFPPu1gU2Y-D+d4i3F0MoNkYK+e1U+=X3qf6QycjfKBw+8snPg@mail.gmail.com/
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 0384d6a023f9d..0e44de1298007 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -582,15 +582,15 @@ struct mptcp_subflow_context {
> disposable : 1, /* ctx can be free at ulp release time */
> closing : 1, /* must not pass rx data to msk anymore */
> valid_csum_seen : 1, /* at least one csum validated */
> - is_mptfo : 1, /* subflow is doing TFO */
> close_event_done : 1, /* has done the post-closed part */
[Severity: High]
Does leaving close_event_done in this bitfield still result in a data race?
In net/mptcp/protocol.c:mptcp_close_ssk(), subflow->close_event_done is set
to true while holding only the msk lock.
Concurrently, the TCP RX path calls subflow_data_ready() ->
mptcp_subflow_data_available() -> subflow_check_data_avail() in
net/mptcp/subflow.c, which sets subflow->map_valid = 1 holding only the
ssk (subflow) lock.
Since close_event_done and map_valid are part of the same u32 bitfield,
could these uncoordinated read-modify-write operations cause concurrent
updates to be silently lost, potentially dropping valid data or executing
mptcp_close_ssk() multiple times?
> mpc_drop : 1, /* the MPC option has been dropped in a rtx */
> resetting : 1, /* subflow is resetting */
> - __unused : 8;
> + __unused : 9;
> bool data_avail;
> bool scheduled;
> bool pm_listener; /* a listener managed by the kernel PM? */
> bool fully_established; /* path validated */
> + bool is_mptfo; /* subflow is doing TFO */
> u32 lent_mem_frag;
> u32 remote_nonce;
> u64 thmac;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261008124220.213016-1-rameshkumar.t@phytecembedded.in?part=1
© 2016 - 2026 Red Hat, Inc.