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

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

Posted by Yilin Zhang 6 days, 4 hours ago

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

Posted by Matthieu Baerts 5 days, 1 hour ago

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

Posted by Paolo Abeni 4 days, 19 hours ago

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

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

Posted by MPTCP CI 6 days, 3 hours ago

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

Posted by sashiko-bot@kernel.org 6 days, 4 hours ago

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