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

[Paolo Abeni <pabeni@redhat.com>]

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