[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.