Netdev List
 help / color / mirror / Atom feed
* [PATCH] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()
@ 2026-09-02 12:12 Yilin Zhang
  2026-09-02 14:12 ` Jiayuan Chen
  0 siblings, 1 reply; 4+ messages in thread
From: Yilin Zhang @ 2026-09-02 12:12 UTC (permalink / raw)
  To: Mat Martineau, Matthieu Baerts
  Cc: Yilin Zhang, netdev, mptcp, Kimi Security Team

subflow_syn_recv_sock() destroys the freshly cloned child for an MP_JOIN
SYN under the fatal fallback and hands it back with drop_req=true to
tell the caller to drop both the request and the child. Of the three
syn_recv_sock() callers, tcp_check_req() and the cookie path honor that
contract; tcp_fastopen_create_child() only checks child != NULL.

With an MPTCP listener and server-side Fast Open enabled, this can
become a use-after-free:

  1. fetch a TFO cookie,
  2. complete a normal MP_CAPABLE handshake to learn the server's key
     and compute the token offline,
  3. send an MP_JOIN SYN carrying that token and the valid TFO cookie,
  4. a subsequent accept() hands the freed socket to userspace, and
     any fd operation triggers slab-use-after-free, leading to a
     denial of service.

Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join subflows")
Reported-by: Kimi Security Team <bug-report@moonshot.ai>
Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
---
 net/ipv4/tcp_fastopen.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 6a031a1a6c9f..7a0b58b39a2c 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -337,6 +337,11 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
 	if (!child)
 		return NULL;
 
+	if (own_req && rsk_drop_req(req)) {
+		sock_put(child);
+		return NULL;
+	}
+
 	spin_lock(&queue->fastopenq.lock);
 	queue->fastopenq.qlen++;
 	spin_unlock(&queue->fastopenq.lock);
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()
  2026-09-02 12:12 [PATCH] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child() Yilin Zhang
@ 2026-09-02 14:12 ` Jiayuan Chen
  2026-09-02 15:31   ` Matthieu Baerts
  0 siblings, 1 reply; 4+ messages in thread
From: Jiayuan Chen @ 2026-09-02 14:12 UTC (permalink / raw)
  To: Yilin Zhang, Mat Martineau, Matthieu Baerts
  Cc: netdev, mptcp, Kimi Security Team


on 9/2/26 8:12 PM, Yilin Zhang wrote:
> subflow_syn_recv_sock() destroys the freshly cloned child for an MP_JOIN
> SYN under the fatal fallback and hands it back with drop_req=true to
> tell the caller to drop both the request and the child. Of the three
> syn_recv_sock() callers, tcp_check_req() and the cookie path honor that
> contract; tcp_fastopen_create_child() only checks child != NULL.
>
> With an MPTCP listener and server-side Fast Open enabled, this can
> become a use-after-free:
>
>    1. fetch a TFO cookie,
>    2. complete a normal MP_CAPABLE handshake to learn the server's key
>       and compute the token offline,
>    3. send an MP_JOIN SYN carrying that token and the valid TFO cookie,
>    4. a subsequent accept() hands the freed socket to userspace, and
>       any fd operation triggers slab-use-after-free, leading to a
>       denial of service.
>
> Fixes: 90bf45134d55 ("mptcp: add new sock flag to deal with join subflows")
> Reported-by: Kimi Security Team <bug-report@moonshot.ai>
> Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
> ---
>   net/ipv4/tcp_fastopen.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> index 6a031a1a6c9f..7a0b58b39a2c 100644
> --- a/net/ipv4/tcp_fastopen.c
> +++ b/net/ipv4/tcp_fastopen.c
> @@ -337,6 +337,11 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
>   	if (!child)
>   		return NULL;
>   
> +	if (own_req && rsk_drop_req(req)) {
> +		sock_put(child);


The child is still locked by inet_csk_clone_lock.


We should do this instead:

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);


> +		return NULL;


tcp_conn_request will still send a SYN-ACK, but the RST was already sent


It's not clean. Maybe we should do something like this instead (untested)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0f60a1dbf927..d371845d601a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -7775,6 +7775,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 &&



BTW, since this is reported by LLM I think it is not difficult to have 
the LLM write a     reproducer and test this patch.


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()
  2026-09-02 14:12 ` Jiayuan Chen
@ 2026-09-02 15:31   ` Matthieu Baerts
  0 siblings, 0 replies; 4+ messages in thread
From: Matthieu Baerts @ 2026-09-02 15:31 UTC (permalink / raw)
  To: Jiayuan Chen, Yilin Zhang, Mat Martineau
  Cc: netdev, mptcp, Kimi Security Team

Hi Jiayuan, Yilin,

Thank you for the patch and the review!

On 02/09/2026 16:12, Jiayuan Chen wrote:
> 
> on 9/2/26 8:12 PM, Yilin Zhang wrote:
>> subflow_syn_recv_sock() destroys the freshly cloned child for an MP_JOIN
>> SYN under the fatal fallback and hands it back with drop_req=true to
>> tell the caller to drop both the request and the child. Of the three
>> syn_recv_sock() callers, tcp_check_req() and the cookie path honor that
>> contract; tcp_fastopen_create_child() only checks child != NULL.
>>
>> With an MPTCP listener and server-side Fast Open enabled, this can
>> become a use-after-free:
>>
>>    1. fetch a TFO cookie,
>>    2. complete a normal MP_CAPABLE handshake to learn the server's key
>>       and compute the token offline,
>>    3. send an MP_JOIN SYN carrying that token and the valid TFO cookie,
>>    4. a subsequent accept() hands the freed socket to userspace, and
>>       any fd operation triggers slab-use-after-free, leading to a
>>       denial of service.

(...)

> BTW, since this is reported by LLM I think it is not difficult to have
> the LLM write a     reproducer and test this patch.

That would be great to use the MPTCP packetdrill fork for that:

  https://github.com/multipath-tcp/packetdrill/

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child()
       [not found] <c4d4d8c-34af-4ea8-ace8-e10e547d6744@kernel.org>
@ 2026-09-03  7:06 ` Yilin Zhang
  0 siblings, 0 replies; 4+ messages in thread
From: Yilin Zhang @ 2026-09-03  7:06 UTC (permalink / raw)
  To: Matthieu Baerts, Jiayuan Chen; +Cc: Mat Martineau, netdev, mptcp

Here is a packetdrill reproducer for the MPTCP subflow/TFO issue.

It first obtains a TFO cookie and the server key during an MP_CAPABLE
handshake, then sends an MP_JOIN SYN from a new source port with the
corresponding token and cookie. The final ACK is intentionally omitted.

With the fix, the request remains in the SYN queue: the MPTCP reset is
followed by a normal SYN/ACK that echoes the cookie, and the second
accept() returns EAGAIN. Without the fix, tcp_fastopen_create_child()
ignores drop_req=true after subflow_syn_recv_sock() destroys the cloned
child. The TFO path queues that child instead; the SYN/ACK omits the
cookie, and closing the listener reports a refcount/use-after-free
warning.

-- >8 -- gtests/net/mptcp/fastopen/server-tfo-mpjoin-syn_v4.pkt -- >8 --

// Regression test: an MP_JOIN SYN with a valid TFO cookie must not queue
// a destroyed child.
--tolerance_usecs=100000
`../common/defaults.sh`

// MPTCP listener with server-side Fast Open enabled.
+0.0  socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3
+0.0  setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0.0  setsockopt(3, SOL_TCP, TCP_FASTOPEN, [2], 4) = 0
+0.0  fcntl(3, F_GETFL) = 0x2 (flags O_RDWR)
+0.0  fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
+0.0  bind(3, ..., ...) = 0
+0.0  listen(3, 1) = 0

// Obtain a TFO cookie and learn the server key.
+0.1  <  S   0:0(0)         win 65535  <mss 1460, sackOK, TS val 100 ecr 0,   nop, wscale 8, FO,            nop, nop, mpcapable v1 flags[flag_h] nokey>
+0.0  >  S.  0:0(0)  ack 1             <mss 1460, nop, nop, sackOK,           nop, wscale 8, FO TFO_COOKIE, nop, nop, mpcapable v1 flags[flag_h] key[skey]>
+0.2  <  .   1:1(0)  ack 1     win 450  <                                                                              mpcapable v1 flags[flag_h] key[ckey=2, skey]>
+0.0  accept(3, ..., ...) = 4

// Use a new source port with the same address; the cookie remains valid.
+0.1  <  192.0.2.1:15000 > 192.168.0.1:8080  S  0:0(0)  win 65535  <mss 1460, sackOK, nop, wscale 8, FO TFO_COOKIE, nop, nop, mp_join_syn address_id=1 token=sha256_32(skey), nop, nop>
// The fatal MPTCP fallback destroys the cloned child and resets the flow.
+0.0  >  192.168.0.1:8080 > 192.0.2.1:15000  R.  0:0(0)  ack 1  <mp_reset 1>
// Normal SYN processing sends a SYN/ACK and echoes the TFO cookie.
+0.0  >  192.168.0.1:8080 > 192.0.2.1:15000  S.  0:0(0)  ack 1  <mss 1460, nop, nop, sackOK, nop, wscale 8, FO TFO_COOKIE, nop, nop, mp_join_syn_ack address_id=0 sender_hmac=auto>

// Without the final ACK, the request must remain in the SYN queue.
+0.2  accept(3, ..., ...) = -1 EAGAIN (Resource temporarily unavailable)

-- >8 --

For the MPTCP packetdrill fork, save this as
`gtests/net/mptcp/fastopen/server-tfo-mpjoin-syn_v4.pkt` and run it with
that fork's packetdrill binary.

Tested on Linux 7.3.0-rc1 (defconfig + MPTCP + KASAN, QEMU guest): the
unpatched kernel sends a SYN/ACK without the TFO cookie and reports
`refcount_t: underflow; use-after-free` when the listener closes; the
fixed kernel passes, with the second `accept()` returning EAGAIN.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-03  7:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 12:12 [PATCH] tcp: fastopen: check rsk_drop_req() in tcp_fastopen_create_child() Yilin Zhang
2026-09-02 14:12 ` Jiayuan Chen
2026-09-02 15:31   ` Matthieu Baerts
     [not found] <c4d4d8c-34af-4ea8-ace8-e10e547d6744@kernel.org>
2026-09-03  7:06 ` Yilin Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox