[PATCH v2] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()

[PATCH v2] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()

[Yilin Zhang]

subflow_syn_recv_sock() sets drop_req when an MP_JOIN SYN takes the fatal
fallback and destroys the cloned child. tcp_fastopen_create_child()
ignored the flag and could queue the destroyed child.

With an MPTCP listener using server-side Fast Open, a valid-cookie
MP_JOIN SYN could then expose the freed child through accept().

Release the locked child and drop the request before tcp_conn_request()
sends a SYN-ACK. Initialize drop_req when allocating the request so the
check cannot observe stale state after request-socket reuse.

Changes in v2:
- unlock the child before dropping its reference
- drop the request instead of sending a SYN-ACK after the MPTCP reset,
  as suggested by Jiayuan Chen

Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join subflows")
Reported-by: Kimi Security Team <bug-report@moonshot.ai>
Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
---
 net/ipv4/tcp_fastopen.c | 6 ++++++
 net/ipv4/tcp_input.c    | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 471c78be5513..22494ff746c7 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -337,6 +337,12 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
 	if (!child)
 		return NULL;
 
+	if (own_req && rsk_drop_req(req)) {
+		bh_unlock_sock(child);
+		sock_put(child);
+		return NULL;
+	}
+
 	spin_lock(&queue->fastopenq.lock);
 	queue->fastopenq.qlen++;
 	spin_unlock(&queue->fastopenq.lock);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0f60a1dbf927..eb0e4259c280 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -7669,8 +7669,9 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
 	tcp_rsk(req)->txhash = net_tx_rndhash();
 #if IS_ENABLED(CONFIG_MPTCP)
 	tcp_rsk(req)->is_mptcp = 0;
+	tcp_rsk(req)->drop_req = false;
 #endif
 
 	tcp_clear_options(&tmp_opt);
 	tmp_opt.mss_clamp = af_ops->mss_clamp;
 	tmp_opt.user_mss  = READ_ONCE(tp->rx_opt.user_mss);
@@ -7775,6 +7776,10 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
 		READ_ONCE(sk->sk_data_ready)(sk);
 		bh_unlock_sock(fastopen_sk);
 		sock_put(fastopen_sk);
+	} else if (rsk_drop_req(req)) {
+		reqsk_free(req);
+		dst_release(dst);
+		return 0;
 	} else {
 		tcp_rsk(req)->tfo_listener = false;
 		if (!want_cookie &&
-- 
2.43.0

Re: [PATCH v2] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()

[MPTCP CI]

Hi Yilin,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: 
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/33742176049

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/a32ce1a16691
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1156939


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH v2] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()

[Paolo Abeni]

On 9/3/26 11:40 AM, Yilin Zhang wrote:
> subflow_syn_recv_sock() sets drop_req when an MP_JOIN SYN takes the fatal
> fallback and destroys the cloned child. tcp_fastopen_create_child()
> ignored the flag and could queue the destroyed child.
> 
> With an MPTCP listener using server-side Fast Open, a valid-cookie
> MP_JOIN SYN could then expose the freed child through accept().
> 
> Release the locked child and drop the request before tcp_conn_request()
> sends a SYN-ACK. Initialize drop_req when allocating the request so the
> check cannot observe stale state after request-socket reuse.
> 
> Changes in v2:
> - unlock the child before dropping its reference
> - drop the request instead of sending a SYN-ACK after the MPTCP reset,
>   as suggested by Jiayuan Chen
> 
> Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join subflows")
> Reported-by: Kimi Security Team <bug-report@moonshot.ai>
> Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>

This looks like the wrong fix. IIRC fastopen is not compatible with MPJ
- as the latter must accept data only after the 4way handshake completion.

@Mat(s): could you please double check the above ^^^ statement???

If so mptcp should reject entirely MPJ + fastopen and no addtional code
required on the TCP side.

A minor process note: the changelog should come after the tag area and a
'---' separator.

/P

Re: [PATCH v2] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()

[Matthieu Baerts]

Hi Paolo,

Thank you for having checked!

On 10/09/2026 10:12, Paolo Abeni wrote:
> On 9/3/26 11:40 AM, Yilin Zhang wrote:
>> subflow_syn_recv_sock() sets drop_req when an MP_JOIN SYN takes the fatal
>> fallback and destroys the cloned child. tcp_fastopen_create_child()
>> ignored the flag and could queue the destroyed child.
>>
>> With an MPTCP listener using server-side Fast Open, a valid-cookie
>> MP_JOIN SYN could then expose the freed child through accept().
>>
>> Release the locked child and drop the request before tcp_conn_request()
>> sends a SYN-ACK. Initialize drop_req when allocating the request so the
>> check cannot observe stale state after request-socket reuse.
>>
>> Changes in v2:
>> - unlock the child before dropping its reference
>> - drop the request instead of sending a SYN-ACK after the MPTCP reset,
>>   as suggested by Jiayuan Chen
>>
>> Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join subflows")
>> Reported-by: Kimi Security Team <bug-report@moonshot.ai>
>> Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>> Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
> 
> This looks like the wrong fix. IIRC fastopen is not compatible with MPJ
> - as the latter must accept data only after the 4way handshake completion.
> 
> @Mat(s): could you please double check the above ^^^ statement???
> 
> If so mptcp should reject entirely MPJ + fastopen and no addtional code
> required on the TCP side.

I didn't check in details, but when I try with this packetdrill repro...

 https://lore.kernel.org/20260903070649.3965366-1-yilinzhang@moonshot.ai

... it looks like the kernel replies with an MP_RST, but also a SYN+ACK,
and a WARN:

  refcount_t: underflow; use-after-free.
  (...)
  inet_csk_listen_stop (net/ipv4/inet_connection_sock.c:1533)
  ? mptcp_subflow_queue_clean (net/mptcp/subflow.c:1925)
  mptcp_check_listen_stop.part.0 (include/net/sock.h:1486
                                  net/mptcp/protocol.c:3482)
  __mptcp_close (net/mptcp/protocol.c:3534)
  mptcp_close (net/mptcp/protocol.c:3572)
  inet_release (net/ipv4/af_inet.c:425)

With this v2, I don't see the SYN+ACK, only the RST.


If I'm not mistaken, with TFO, syn_recv_sock will be called first
(tcp_conn_request -> tcp_fastopen_create_child -> subflow_syn_recv_sock)
then MPTCP will only check TFO when the SYN+ACK is being sent
(tcp_conn_request -> subflow_v(46)_send_synack).

In subflow_syn_recv_sock(), I don't think we have a strong indicator
that the caller is TFO. It looks like we would also need to change the
TCP stack to pass this info, no?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH v2] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()

[Jiayuan Chen]

On 9/10/26 6:13 PM, Matthieu Baerts wrote:
> Hi Paolo,
>
> Thank you for having checked!
>
> On 10/09/2026 10:12, Paolo Abeni wrote:
>> On 9/3/26 11:40 AM, Yilin Zhang wrote:
>>> subflow_syn_recv_sock() sets drop_req when an MP_JOIN SYN takes the fatal
>>> fallback and destroys the cloned child. tcp_fastopen_create_child()
>>> ignored the flag and could queue the destroyed child.
>>>
>>> With an MPTCP listener using server-side Fast Open, a valid-cookie
>>> MP_JOIN SYN could then expose the freed child through accept().
>>>
>>> Release the locked child and drop the request before tcp_conn_request()
>>> sends a SYN-ACK. Initialize drop_req when allocating the request so the
>>> check cannot observe stale state after request-socket reuse.
>>>
>>> Changes in v2:
>>> - unlock the child before dropping its reference
>>> - drop the request instead of sending a SYN-ACK after the MPTCP reset,
>>>    as suggested by Jiayuan Chen
>>>
>>> Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join subflows")
>>> Reported-by: Kimi Security Team <bug-report@moonshot.ai>
>>> Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>>> Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
>> This looks like the wrong fix. IIRC fastopen is not compatible with MPJ
>> - as the latter must accept data only after the 4way handshake completion.
>>
>> @Mat(s): could you please double check the above ^^^ statement???
>>
>> If so mptcp should reject entirely MPJ + fastopen and no addtional code
>> required on the TCP side.
> I didn't check in details, but when I try with this packetdrill repro...
>
>   https://lore.kernel.org/20260903070649.3965366-1-yilinzhang@moonshot.ai
>
> ... it looks like the kernel replies with an MP_RST, but also a SYN+ACK,
> and a WARN:
>
>    refcount_t: underflow; use-after-free.
>    (...)
>    inet_csk_listen_stop (net/ipv4/inet_connection_sock.c:1533)
>    ? mptcp_subflow_queue_clean (net/mptcp/subflow.c:1925)
>    mptcp_check_listen_stop.part.0 (include/net/sock.h:1486
>                                    net/mptcp/protocol.c:3482)
>    __mptcp_close (net/mptcp/protocol.c:3534)
>    mptcp_close (net/mptcp/protocol.c:3572)
>    inet_release (net/ipv4/af_inet.c:425)
>
> With this v2, I don't see the SYN+ACK, only the RST.
>
>
> If I'm not mistaken, with TFO, syn_recv_sock will be called first
> (tcp_conn_request -> tcp_fastopen_create_child -> subflow_syn_recv_sock)
> then MPTCP will only check TFO when the SYN+ACK is being sent
> (tcp_conn_request -> subflow_v(46)_send_synack).
>
> In subflow_syn_recv_sock(), I don't think we have a strong indicator


You inspired me. Maybe we can use such code:

     if (subflow_req->mp_join &&
         (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_SYN))
             return NULL;


We now skip RST and SYN+ACK also can be sent by tcp_conn_request().




> that the caller is TFO. It looks like we would also need to change the
> TCP stack to pass this info, no?

Re: [PATCH v2] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()

[Matthieu Baerts]

Hi Jiayuan,

On 10/09/2026 14:15, Jiayuan Chen wrote:
> 
> On 9/10/26 6:13 PM, Matthieu Baerts wrote:
>> Hi Paolo,
>>
>> Thank you for having checked!
>>
>> On 10/09/2026 10:12, Paolo Abeni wrote:
>>> On 9/3/26 11:40 AM, Yilin Zhang wrote:
>>>> subflow_syn_recv_sock() sets drop_req when an MP_JOIN SYN takes the
>>>> fatal
>>>> fallback and destroys the cloned child. tcp_fastopen_create_child()
>>>> ignored the flag and could queue the destroyed child.
>>>>
>>>> With an MPTCP listener using server-side Fast Open, a valid-cookie
>>>> MP_JOIN SYN could then expose the freed child through accept().
>>>>
>>>> Release the locked child and drop the request before tcp_conn_request()
>>>> sends a SYN-ACK. Initialize drop_req when allocating the request so the
>>>> check cannot observe stale state after request-socket reuse.
>>>>
>>>> Changes in v2:
>>>> - unlock the child before dropping its reference
>>>> - drop the request instead of sending a SYN-ACK after the MPTCP reset,
>>>>    as suggested by Jiayuan Chen
>>>>
>>>> Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join
>>>> subflows")
>>>> Reported-by: Kimi Security Team <bug-report@moonshot.ai>
>>>> Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>>>> Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
>>> This looks like the wrong fix. IIRC fastopen is not compatible with MPJ
>>> - as the latter must accept data only after the 4way handshake
>>> completion.
>>>
>>> @Mat(s): could you please double check the above ^^^ statement???
>>>
>>> If so mptcp should reject entirely MPJ + fastopen and no addtional code
>>> required on the TCP side.
>> I didn't check in details, but when I try with this packetdrill repro...
>>
>>   https://lore.kernel.org/20260903070649.3965366-1-yilinzhang@moonshot.ai
>>
>> ... it looks like the kernel replies with an MP_RST, but also a SYN+ACK,
>> and a WARN:
>>
>>    refcount_t: underflow; use-after-free.
>>    (...)
>>    inet_csk_listen_stop (net/ipv4/inet_connection_sock.c:1533)
>>    ? mptcp_subflow_queue_clean (net/mptcp/subflow.c:1925)
>>    mptcp_check_listen_stop.part.0 (include/net/sock.h:1486
>>                                    net/mptcp/protocol.c:3482)
>>    __mptcp_close (net/mptcp/protocol.c:3534)
>>    mptcp_close (net/mptcp/protocol.c:3572)
>>    inet_release (net/ipv4/af_inet.c:425)
>>
>> With this v2, I don't see the SYN+ACK, only the RST.
>>
>>
>> If I'm not mistaken, with TFO, syn_recv_sock will be called first
>> (tcp_conn_request -> tcp_fastopen_create_child -> subflow_syn_recv_sock)
>> then MPTCP will only check TFO when the SYN+ACK is being sent
>> (tcp_conn_request -> subflow_v(46)_send_synack).
>>
>> In subflow_syn_recv_sock(), I don't think we have a strong indicator
> 
> 
> You inspired me. Maybe we can use such code:
> 
>     if (subflow_req->mp_join &&
>         (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_SYN))
>             return NULL;

Good idea! Indeed, having a SYN here only happens with TFO.

It would be good to add a comment there then.

> We now skip RST and SYN+ACK also can be sent by tcp_conn_request().

Yes, I guess returning NULL is not enough, a reset should probably be
sent as well (prohibit), a MIB counter incremented (MPJ rejected?), and
the request dropped.

Just to avoid a deadlock: @Yilin: will you send a v3 with this suggestion?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH v2] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()

[Jiayuan Chen]

On 9/11/26 12:12 AM, Matthieu Baerts wrote:
> Hi Jiayuan,
>
> On 10/09/2026 14:15, Jiayuan Chen wrote:
>> On 9/10/26 6:13 PM, Matthieu Baerts wrote:
>>> Hi Paolo,
>>>
>>> Thank you for having checked!
>>>
>>> On 10/09/2026 10:12, Paolo Abeni wrote:
>>>> On 9/3/26 11:40 AM, Yilin Zhang wrote:
>>>>> subflow_syn_recv_sock() sets drop_req when an MP_JOIN SYN takes the
>>>>> fatal
>>>>> fallback and destroys the cloned child. tcp_fastopen_create_child()
>>>>> ignored the flag and could queue the destroyed child.
>>>>>
>>>>> With an MPTCP listener using server-side Fast Open, a valid-cookie
>>>>> MP_JOIN SYN could then expose the freed child through accept().
>>>>>
>>>>> Release the locked child and drop the request before tcp_conn_request()
>>>>> sends a SYN-ACK. Initialize drop_req when allocating the request so the
>>>>> check cannot observe stale state after request-socket reuse.
>>>>>
>>>>> Changes in v2:
>>>>> - unlock the child before dropping its reference
>>>>> - drop the request instead of sending a SYN-ACK after the MPTCP reset,
>>>>>     as suggested by Jiayuan Chen
>>>>>
>>>>> Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join
>>>>> subflows")
>>>>> Reported-by: Kimi Security Team <bug-report@moonshot.ai>
>>>>> Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>>>>> Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
>>>> This looks like the wrong fix. IIRC fastopen is not compatible with MPJ
>>>> - as the latter must accept data only after the 4way handshake
>>>> completion.
>>>>
>>>> @Mat(s): could you please double check the above ^^^ statement???
>>>>
>>>> If so mptcp should reject entirely MPJ + fastopen and no addtional code
>>>> required on the TCP side.
>>> I didn't check in details, but when I try with this packetdrill repro...
>>>
>>>    https://lore.kernel.org/20260903070649.3965366-1-yilinzhang@moonshot.ai
>>>
>>> ... it looks like the kernel replies with an MP_RST, but also a SYN+ACK,
>>> and a WARN:
>>>
>>>     refcount_t: underflow; use-after-free.
>>>     (...)
>>>     inet_csk_listen_stop (net/ipv4/inet_connection_sock.c:1533)
>>>     ? mptcp_subflow_queue_clean (net/mptcp/subflow.c:1925)
>>>     mptcp_check_listen_stop.part.0 (include/net/sock.h:1486
>>>                                     net/mptcp/protocol.c:3482)
>>>     __mptcp_close (net/mptcp/protocol.c:3534)
>>>     mptcp_close (net/mptcp/protocol.c:3572)
>>>     inet_release (net/ipv4/af_inet.c:425)
>>>
>>> With this v2, I don't see the SYN+ACK, only the RST.
>>>
>>>
>>> If I'm not mistaken, with TFO, syn_recv_sock will be called first
>>> (tcp_conn_request -> tcp_fastopen_create_child -> subflow_syn_recv_sock)
>>> then MPTCP will only check TFO when the SYN+ACK is being sent
>>> (tcp_conn_request -> subflow_v(46)_send_synack).
>>>
>>> In subflow_syn_recv_sock(), I don't think we have a strong indicator
>>
>> You inspired me. Maybe we can use such code:
>>
>>      if (subflow_req->mp_join &&
>>          (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_SYN))
>>              return NULL;
> Good idea! Indeed, having a SYN here only happens with TFO.
>
> It would be good to add a comment there then.
>
>> We now skip RST and SYN+ACK also can be sent by tcp_conn_request().
> Yes, I guess returning NULL is not enough, a reset should probably be
> sent as well (prohibit), a MIB counter incremented (MPJ rejected?), and
> the request dropped.


Should we accept this subflow ?

It's just a SYN with MPJ + valid fastopen cookie, replying SYNACK 
and fallback to 3-way handshake may be easier.

(I'm not sure whether RFC define it or not.)


>
> Just to avoid a deadlock: @Yilin: will you send a v3 with this suggestion?


If we want to reject such SYN, we need modify tcp_conn_request() to 
avoid the SYNACK being sent

and explicit‌ly send RST before returning NULL.

>
> Cheers,
> Matt

Re: [PATCH v2] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()

[Yilin Zhang]

Hi Jiayuan, Paolo, Matt,

On 11/09/2026, Jiayuan Chen wrote:
> Should we accept this subflow ?
>
> It's just a SYN with MPJ + valid fastopen cookie, replying SYNACK
> and fallback to 3-way handshake may be easier.
> (I'm not sure whether RFC define it or not.)

This looks good to me. Falling back like this is also standard TFO
behavior (RFC 7413, sec. 4.2.2), so IIUC the RFCs don't object.

On 10/09/2026, Paolo Abeni wrote:
> If so mptcp should reject entirely MPJ + fastopen and no addtional
> code required on the TCP side.

With Jiayuan's SYN-flag check in subflow_syn_recv_sock(), we can do
exactly that: return NULL there, and tcp_conn_request() falls back to
the regular MP_JOIN handshake. No TCP-side changes needed.

> A minor process note: the changelog should come after the tag area
> and a '---' separator.

Noted, thanks for the reminder. I'll draft a v3 shortly along these
lines.

Thanks,
Yilin

Re: [PATCH v2] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()

[Matthieu Baerts]

Hi Jiayuan,

On 11/09/2026 04:25, Jiayuan Chen wrote:
> 
> On 9/11/26 12:12 AM, Matthieu Baerts wrote:
>> Hi Jiayuan,
>>
>> On 10/09/2026 14:15, Jiayuan Chen wrote:
>>> On 9/10/26 6:13 PM, Matthieu Baerts wrote:
>>>> Hi Paolo,
>>>>
>>>> Thank you for having checked!
>>>>
>>>> On 10/09/2026 10:12, Paolo Abeni wrote:
>>>>> On 9/3/26 11:40 AM, Yilin Zhang wrote:
>>>>>> subflow_syn_recv_sock() sets drop_req when an MP_JOIN SYN takes the
>>>>>> fatal
>>>>>> fallback and destroys the cloned child. tcp_fastopen_create_child()
>>>>>> ignored the flag and could queue the destroyed child.
>>>>>>
>>>>>> With an MPTCP listener using server-side Fast Open, a valid-cookie
>>>>>> MP_JOIN SYN could then expose the freed child through accept().
>>>>>>
>>>>>> Release the locked child and drop the request before
>>>>>> tcp_conn_request()
>>>>>> sends a SYN-ACK. Initialize drop_req when allocating the request
>>>>>> so the
>>>>>> check cannot observe stale state after request-socket reuse.
>>>>>>
>>>>>> Changes in v2:
>>>>>> - unlock the child before dropping its reference
>>>>>> - drop the request instead of sending a SYN-ACK after the MPTCP
>>>>>> reset,
>>>>>>     as suggested by Jiayuan Chen
>>>>>>
>>>>>> Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join
>>>>>> subflows")
>>>>>> Reported-by: Kimi Security Team <bug-report@moonshot.ai>
>>>>>> Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>>>>>> Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
>>>>> This looks like the wrong fix. IIRC fastopen is not compatible with
>>>>> MPJ
>>>>> - as the latter must accept data only after the 4way handshake
>>>>> completion.
>>>>>
>>>>> @Mat(s): could you please double check the above ^^^ statement???
>>>>>
>>>>> If so mptcp should reject entirely MPJ + fastopen and no addtional
>>>>> code
>>>>> required on the TCP side.
>>>> I didn't check in details, but when I try with this packetdrill
>>>> repro...
>>>>
>>>>    https://lore.kernel.org/20260903070649.3965366-1-
>>>> yilinzhang@moonshot.ai
>>>>
>>>> ... it looks like the kernel replies with an MP_RST, but also a
>>>> SYN+ACK,
>>>> and a WARN:
>>>>
>>>>     refcount_t: underflow; use-after-free.
>>>>     (...)
>>>>     inet_csk_listen_stop (net/ipv4/inet_connection_sock.c:1533)
>>>>     ? mptcp_subflow_queue_clean (net/mptcp/subflow.c:1925)
>>>>     mptcp_check_listen_stop.part.0 (include/net/sock.h:1486
>>>>                                     net/mptcp/protocol.c:3482)
>>>>     __mptcp_close (net/mptcp/protocol.c:3534)
>>>>     mptcp_close (net/mptcp/protocol.c:3572)
>>>>     inet_release (net/ipv4/af_inet.c:425)
>>>>
>>>> With this v2, I don't see the SYN+ACK, only the RST.
>>>>
>>>>
>>>> If I'm not mistaken, with TFO, syn_recv_sock will be called first
>>>> (tcp_conn_request -> tcp_fastopen_create_child ->
>>>> subflow_syn_recv_sock)
>>>> then MPTCP will only check TFO when the SYN+ACK is being sent
>>>> (tcp_conn_request -> subflow_v(46)_send_synack).
>>>>
>>>> In subflow_syn_recv_sock(), I don't think we have a strong indicator
>>>
>>> You inspired me. Maybe we can use such code:
>>>
>>>      if (subflow_req->mp_join &&
>>>          (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_SYN))
>>>              return NULL;
>> Good idea! Indeed, having a SYN here only happens with TFO.
>>
>> It would be good to add a comment there then.
>>
>>> We now skip RST and SYN+ACK also can be sent by tcp_conn_request().
>> Yes, I guess returning NULL is not enough, a reset should probably be
>> sent as well (prohibit), a MIB counter incremented (MPJ rejected?), and
>> the request dropped.
> 
> 
> Should we accept this subflow ?
> 
> It's just a SYN with MPJ + valid fastopen cookie, replying SYNACK
> and fallback to 3-way handshake may be easier.

Yes, better indeed.

But then I guess we should not send a TFO cookie in the SYNACK for this
case, right?

Setting foc->len = -1 for MPJ in subflow_prep_synack()?

> (I'm not sure whether RFC define it or not.)

I don't think the MPTCP one mentions this case.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

[PATCH v3] mptcp: do not use Fast Open on MP_JOIN subflows

[Yilin Zhang]

tcp_fastopen_create_child() hands the SYN packet itself to
subflow_syn_recv_sock(). For an MP_JOIN request this takes the
fatal fallback: the cloned child is destroyed and handed back with
drop_req flagged, but tcp_fastopen_create_child() does not check
the flag and queues it, so accept() can expose the freed child.

MP_JOIN cannot use Fast Open: data on a subflow requires the
completed HMAC exchange (RFC 8684, sec. 3.2). Refuse it (only the
TFO path passes a SYN skb here) and let tcp_conn_request() fall
back to the regular MP_JOIN handshake; also strip the Fast Open
cookie from MP_JOIN SYN/ACKs.

Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join subflows")
Reported-by: Kimi Security Team <bug-report@moonshot.ai>
Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Suggested-by: Matthieu Baerts <matttbe@kernel.org>
Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
---
v3:
- refuse TFO for MP_JOIN in subflow_syn_recv_sock() and fall back to
  the regular handshake; the TCP-side hunks from v2 are dropped
- strip the TFO cookie from MP_JOIN SYN/ACKs
v2:
- https://lore.kernel.org/netdev/20260903094010.4066892-1-yilinzhang@moonshot.ai/
v1:
- https://lore.kernel.org/netdev/20260902121247.3248539-1-yilinzhang@moonshot.ai/

 net/mptcp/subflow.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index af81ad5..2b454d4 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -348,6 +348,10 @@ static void subflow_prep_synack(const struct sock *sk, struct request_sock *req,
 	if (foc && foc->len > -1)
 		ireq->tstamp_ok = 0;
 
+	/* MP_JOIN cannot use TFO, do not send a cookie in the SYN/ACK */
+	if (foc && mptcp_subflow_rsk(req)->mp_join)
+		foc->len = -1;
+
 	if (synack_type == TCP_SYNACK_FASTOPEN)
 		mptcp_fastopen_subflow_synack_set_params(subflow, req);
 }
@@ -832,6 +836,12 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
 	if (fallback)
 		goto create_child;
 
+	/* a SYN skb here comes from TFO, which MP_JOIN cannot use: just
+	 * fall back to the regular path.
+	 */
+	if (subflow_req->mp_join && (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_SYN))
+		return NULL;
+
 	/* if the sk is MP_CAPABLE, we try to fetch the client key */
 	if (subflow_req->mp_capable) {
 		/* we can receive and accept an in-window, out-of-order pkt,
-- 
2.34.1

Re: [PATCH v3] mptcp: do not use Fast Open on MP_JOIN subflows

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net/mptcp: unintentional disabling of TCP timestamps for MP_JOIN
--

commit 2eba4e34ce837e7639b005fa4e73483919c7fa14
Author: Yilin Zhang <yilinzhang@moonshot.ai>

mptcp: do not use Fast Open on MP_JOIN subflows

This commit rejects Fast Open on MP_JOIN subflows since data on a subflow
requires a completed HMAC exchange. It falls back to the regular handshake
for TFO SYN packets and strips the Fast Open cookie from MP_JOIN SYN/ACKs.

> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index f0a6725d2c376..7bf94aea5f681 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -348,6 +348,10 @@ static void subflow_prep_synack(const struct sock *sk, struct request_sock *req,
>  	if (foc && foc->len > -1)
>  		ireq->tstamp_ok = 0;
>  
> +	/* MP_JOIN cannot use TFO, do not send a cookie in the SYN/ACK */
> +	if (foc && mptcp_subflow_rsk(req)->mp_join)
> +		foc->len = -1;
> +

[Severity: Medium]
Does this sequence unintentionally disable TCP timestamps for MP_JOIN requests
with TFO cookies?

In subflow_prep_synack(), if a remote client sends an MP_JOIN SYN with a TFO
cookie request, foc->len will initially be > -1, causing ireq->tstamp_ok to
be cleared to 0. 

Immediately following that, this new check sets foc->len to -1 to reject TFO.
While the TFO cookie is correctly dropped and not sent, tstamp_ok has already
been cleared and remains 0, leading to an unintended loss of TCP timestamps
on the subflow.

>  	if (synack_type == TCP_SYNACK_FASTOPEN)
>  		mptcp_fastopen_subflow_synack_set_params(subflow, req);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920061755.3575063-1-yilinzhang@moonshot.ai?part=1

Re: [PATCH v3] mptcp: do not use Fast Open on MP_JOIN subflows

[MPTCP CI]

Hi Yilin,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/35494752388

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/7a943777d986
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1169617


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

[PATCH v3] mptcp: do not use Fast Open on MP_JOIN subflows

[Yilin Zhang]

tcp_fastopen_create_child() hands the SYN packet itself to
subflow_syn_recv_sock(). For an MP_JOIN request this takes the
fatal fallback: the cloned child is destroyed and handed back with
drop_req flagged, but tcp_fastopen_create_child() does not check
the flag and queues it, so accept() can expose the freed child.

MP_JOIN cannot use Fast Open: data on a subflow requires the
completed HMAC exchange (RFC 8684, sec. 3.2). Refuse it (only the
TFO path passes a SYN skb here) and let tcp_conn_request() fall
back to the regular MP_JOIN handshake; also strip the Fast Open
cookie from MP_JOIN SYN/ACKs.

Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join subflows")
Reported-by: Kimi Security Team <bug-report@moonshot.ai>
Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Suggested-by: Matthieu Baerts <matttbe@kernel.org>
Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
---
v3:
- refuse TFO for MP_JOIN in subflow_syn_recv_sock() and fall back to
  the regular handshake; the TCP-side hunks from v2 are dropped
- strip the TFO cookie from MP_JOIN SYN/ACKs
v2:
- https://lore.kernel.org/netdev/20260903094010.4066892-1-yilinzhang@moonshot.ai/
v1:
- https://lore.kernel.org/netdev/20260902121247.3248539-1-yilinzhang@moonshot.ai/

 net/mptcp/subflow.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index af81ad5..2b454d4 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -348,6 +348,10 @@ static void subflow_prep_synack(const struct sock *sk, struct request_sock *req,
 	if (foc && foc->len > -1)
 		ireq->tstamp_ok = 0;
 
+	/* MP_JOIN cannot use TFO, do not send a cookie in the SYN/ACK */
+	if (foc && mptcp_subflow_rsk(req)->mp_join)
+		foc->len = -1;
+
 	if (synack_type == TCP_SYNACK_FASTOPEN)
 		mptcp_fastopen_subflow_synack_set_params(subflow, req);
 }
@@ -832,6 +836,12 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
 	if (fallback)
 		goto create_child;
 
+	/* a SYN skb here comes from TFO, which MP_JOIN cannot use: just
+	 * fall back to the regular path.
+	 */
+	if (subflow_req->mp_join && (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_SYN))
+		return NULL;
+
 	/* if the sk is MP_CAPABLE, we try to fetch the client key */
 	if (subflow_req->mp_capable) {
 		/* we can receive and accept an in-window, out-of-order pkt,
-- 
2.34.1

Re: [PATCH v3] mptcp: do not use Fast Open on MP_JOIN subflows

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mptcp: disabled TCP timestamps on MP_JOIN with TFO due to wrong logic order
--

commit 7c8131d455e6d16e5fa896b1f57ebf371c13798d
Author: Yilin Zhang <yilinzhang@moonshot.ai>

mptcp: do not use Fast Open on MP_JOIN subflows

This commit disables TCP Fast Open on MP_JOIN subflows, as data on a subflow
requires a completed HMAC exchange. It updates the SYN processing path to
fallback to a regular handshake and strips the TFO cookie from MP_JOIN
SYN/ACKs.

> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index f0a6725d2c376..7bf94aea5f681 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -348,6 +348,10 @@ static void subflow_prep_synack(const struct sock *sk, struct request_sock *req,
>  	if (foc && foc->len > -1)
>  		ireq->tstamp_ok = 0;
>  
> +	/* MP_JOIN cannot use TFO, do not send a cookie in the SYN/ACK */
> +	if (foc && mptcp_subflow_rsk(req)->mp_join)
> +		foc->len = -1;
> +
[Severity: Medium]
Does this logic order introduce a regression that unnecessarily disables
TCP timestamps on MP_JOIN subflows?

When a client initiates an MP_JOIN connection with a TCP Fast Open cookie
request, the original code in subflow_prep_synack() evaluates
foc && foc->len > -1 and sets ireq->tstamp_ok = 0.

Right after that, the newly added patch logic checks mp_join and sets
foc->len = -1 to reject TFO.

Because the timestamp check happens before the cookie is disabled, the SYN/ACK
is sent without both the TFO cookie and the TCP timestamp. Any remote client
can trigger this by simply adding a TFO option to their MP_JOIN SYN packet.

Could reversing the order of these checks preserve TCP timestamps by causing
the timestamp condition to evaluate to false when foc->len is updated to -1?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920061904.3575780-1-yilinzhang@moonshot.ai?part=1

Re: [PATCH v3] mptcp: do not use Fast Open on MP_JOIN subflows

[MPTCP CI]

Hi Yilin,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/35494716290

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/2f1608ef83a5
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1169618


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH v3] mptcp: do not use Fast Open on MP_JOIN subflows

[Matthieu Baerts]

Hi Yilin,

20 Sept 2026 08:19:21 Yilin Zhang <yilinzhang@moonshot.ai>:

> tcp_fastopen_create_child() hands the SYN packet itself to
> subflow_syn_recv_sock(). For an MP_JOIN request this takes the
> fatal fallback: the cloned child is destroyed and handed back with
> drop_req flagged, but tcp_fastopen_create_child() does not check
> the flag and queues it, so accept() can expose the freed child.
>
> MP_JOIN cannot use Fast Open: data on a subflow requires the
> completed HMAC exchange (RFC 8684, sec. 3.2). Refuse it (only the
> TFO path passes a SYN skb here) and let tcp_conn_request() fall
> back to the regular MP_JOIN handshake; also strip the Fast Open
> cookie from MP_JOIN SYN/ACKs.
>
> Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join subflows")
> Reported-by: Kimi Security Team <bug-report@moonshot.ai>
> Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Suggested-by: Matthieu Baerts <matttbe@kernel.org>
> Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
> ---
> v3:
> - refuse TFO for MP_JOIN in subflow_syn_recv_sock() and fall back to
>   the regular handshake; the TCP-side hunks from v2 are dropped
> - strip the TFO cookie from MP_JOIN SYN/ACKs

Thank you for the V3.

Please start a new thread when sending a new version.

> v2:
> - https://lore.kernel.org/netdev/20260903094010.4066892-1-yilinzhang@moonshot.ai/
> v1:
> - https://lore.kernel.org/netdev/20260902121247.3248539-1-yilinzhang@moonshot.ai/
>
> net/mptcp/subflow.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index af81ad5..2b454d4 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -348,6 +348,10 @@ static void subflow_prep_synack(const struct sock *sk, struct request_sock *req,
>     if (foc && foc->len > -1)
>         ireq->tstamp_ok = 0;
>
> +   /* MP_JOIN cannot use TFO, do not send a cookie in the SYN/ACK */
> +   if (foc && mptcp_subflow_rsk(req)->mp_join)
> +       foc->len = -1;

This should go above the previous block, not to disable TCP timestamps
in this case.

> +
>     if (synack_type == TCP_SYNACK_FASTOPEN)
>         mptcp_fastopen_subflow_synack_set_params(subflow, req);
> }
> @@ -832,6 +836,12 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
>     if (fallback)
>         goto create_child;
>
> +   /* a SYN skb here comes from TFO, which MP_JOIN cannot use: just
> +    * fall back to the regular path.
> +    */

Can be shorter and on one line maybe?

  MP_JOIN + TFO: unsupported now, drop TFO data, back later on

(Or something similar)

> +   if (subflow_req->mp_join && (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_SYN))
> +       return NULL;
> +
>     /* if the sk is MP_CAPABLE, we try to fetch the client key */
>     if (subflow_req->mp_capable) {
>         /* we can receive and accept an in-window, out-of-order pkt,
> --
> 2.34.1

Cheers,
Matt

Re: [PATCH v3] mptcp: do not use Fast Open on MP_JOIN subflows

[Paolo Abeni]

On 9/21/26 11:43, Matthieu Baerts wrote:
> 20 Sept 2026 08:19:21 Yilin Zhang <yilinzhang@moonshot.ai>:
>> tcp_fastopen_create_child() hands the SYN packet itself to
>> subflow_syn_recv_sock(). For an MP_JOIN request this takes the
>> fatal fallback: the cloned child is destroyed and handed back with
>> drop_req flagged, but tcp_fastopen_create_child() does not check
>> the flag and queues it, so accept() can expose the freed child.
>>
>> MP_JOIN cannot use Fast Open: data on a subflow requires the
>> completed HMAC exchange (RFC 8684, sec. 3.2). Refuse it (only the
>> TFO path passes a SYN skb here) and let tcp_conn_request() fall
>> back to the regular MP_JOIN handshake; also strip the Fast Open
>> cookie from MP_JOIN SYN/ACKs.
>>
>> Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join subflows")
>> Reported-by: Kimi Security Team <bug-report@moonshot.ai>
>> Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>> Suggested-by: Paolo Abeni <pabeni@redhat.com>
>> Suggested-by: Matthieu Baerts <matttbe@kernel.org>
>> Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
>> ---
>> v3:
>> - refuse TFO for MP_JOIN in subflow_syn_recv_sock() and fall back to
>>    the regular handshake; the TCP-side hunks from v2 are dropped
>> - strip the TFO cookie from MP_JOIN SYN/ACKs
> 
> Thank you for the V3.
> 
> Please start a new thread when sending a new version.
> 
>> v2:
>> - https://lore.kernel.org/netdev/20260903094010.4066892-1-yilinzhang@moonshot.ai/
>> v1:
>> - https://lore.kernel.org/netdev/20260902121247.3248539-1-yilinzhang@moonshot.ai/
>>
>> net/mptcp/subflow.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
>> index af81ad5..2b454d4 100644
>> --- a/net/mptcp/subflow.c
>> +++ b/net/mptcp/subflow.c
>> @@ -348,6 +348,10 @@ static void subflow_prep_synack(const struct sock *sk, struct request_sock *req,
>>      if (foc && foc->len > -1)
>>          ireq->tstamp_ok = 0;
>>
>> +   /* MP_JOIN cannot use TFO, do not send a cookie in the SYN/ACK */
>> +   if (foc && mptcp_subflow_rsk(req)->mp_join)
>> +       foc->len = -1;
> 
> This should go above the previous block, not to disable TCP timestamps
> in this case.
I'm sorry for lagging behind on this thread.

I think this is not the correct approach.

AFAICS only pktdrill or similar can send MPJ TFO; there is no gain in
keeping such subflows open, and some potential risks. IMHO this too
similar to the disconnect()/ADDR_FORM scenarios to embark into the 'keep
the feature alive' path.

I suggest just resetting the TFO MPJ subflow and avoid other possible
follow-ups.

@Yilin, please wait for agreement on this point before sending a new
revision.

Thanks,

Paolo