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

[Matthieu Baerts <matttbe@kernel.org>]

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.