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

[Jiayuan Chen <jiayuan.chen@linux.dev>]

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?