* [PATCH net] tcp: ipv6: keep request pktopts alive until request destruction
@ 2026-11-02 6:12 Yingjie Wang
2026-11-02 7:03 ` Kuniyuki Iwashima
0 siblings, 1 reply; 3+ messages in thread
From: Yingjie Wang @ 2026-11-02 6:12 UTC (permalink / raw)
To: netdev
Cc: edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms, daniel,
martin.lau, linux-kernel, stable
When IPv6 flow label reflection is enabled, tcp_v6_send_synack() reads
ireq->pktopts to copy the flow label from the initial SYN.
Request sockets can be processed without the listener lock. A
retransmitted SYN can therefore run tcp_v6_send_synack() while a
concurrent final ACK creates the child in tcp_v6_syn_recv_sock(). The
latter clones ireq->pktopts for the child and immediately consumes the
request's reference. This can free the skb while the retransmit path is
still reading its IPv6 header, causing a use-after-free.
Request migration has the same ownership problem. inet_reqsk_clone()
copies pktopts without taking an skb reference, and reqsk_migrate_reset()
then clears the old request's pointer. A CPU holding the old request can
still be sending a SYNACK when the cloned request releases the only skb
reference.
Do not consume pktopts while creating the child. Give each cloned request
its own skb reference and let every request release its reference from
tcp_v6_reqsk_destructor().
I found this while testing IPv6 TCP request-socket lifetime handling. The
reproducer required no source changes, artificial delay, or fault
injection beyond enabling KASAN. It triggered the use-after-free before
this change. With this change, a 2000-round run completed without a KASAN
report. A separate 10-round test recorded four successful request
migrations and no KASAN report. Networking selftests were not run because
none directly exercises this race.
Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
Cc: stable@vger.kernel.org
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
net/ipv4/inet_connection_sock.c | 6 +++++-
net/ipv6/tcp_ipv6.c | 7 ++++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 93b306d91b6d..66f8ce25aa38 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -936,6 +936,11 @@ static struct request_sock *inet_reqsk_clone(struct request_sock *req,
req->rsk_ops->obj_size - offsetof(struct sock, sk_dontcopy_end),
/* alloc is larger than struct, see above */);
+#if IS_ENABLED(CONFIG_IPV6)
+ if (inet_rsk(nreq)->pktopts)
+ skb_get(inet_rsk(nreq)->pktopts);
+#endif
+
sk_node_init(&nreq_sk->sk_node);
nreq_sk->sk_tx_queue_mapping = req_sk->sk_tx_queue_mapping;
#ifdef CONFIG_SOCK_RX_QUEUE_MAPPING
@@ -982,7 +987,6 @@ static void reqsk_migrate_reset(struct request_sock *req)
req->saved_syn = NULL;
#if IS_ENABLED(CONFIG_IPV6)
inet_rsk(req)->ipv6_opt = NULL;
- inet_rsk(req)->pktopts = NULL;
#else
inet_rsk(req)->ireq_opt = NULL;
#endif
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index e006a30360ea..0b51cb4ad960 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1522,11 +1522,12 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
if (*own_req) {
tcp_move_syn(newtp, req);
- /* Clone pktoptions received with SYN, if we own the req */
+ /* Clone pktoptions received with SYN. Keep the request's
+ * reference until the request is destroyed, as a concurrent
+ * SYN retransmit may still read it.
+ */
if (ireq->pktopts) {
newnp->pktoptions = skb_clone_and_charge_r(ireq->pktopts, newsk);
- consume_skb(ireq->pktopts);
- ireq->pktopts = NULL;
if (newnp->pktoptions)
tcp_v6_restore_cb(newnp->pktoptions);
}
base-commit: cb284914e749388eef402938bdb3115fe1f6d337
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] tcp: ipv6: keep request pktopts alive until request destruction
2026-11-02 6:12 [PATCH net] tcp: ipv6: keep request pktopts alive until request destruction Yingjie Wang
@ 2026-11-02 7:03 ` Kuniyuki Iwashima
2026-11-02 7:28 ` Eric Dumazet
0 siblings, 1 reply; 3+ messages in thread
From: Kuniyuki Iwashima @ 2026-11-02 7:03 UTC (permalink / raw)
To: Yingjie Wang
Cc: netdev, edumazet, ncardwell, davem, kuba, pabeni, horms, daniel,
martin.lau, linux-kernel, stable
On Mon, 02 Nov 2026 06:12:18 +0000, Yingjie Wang wrote:
> When IPv6 flow label reflection is enabled, tcp_v6_send_synack() reads
> ireq->pktopts to copy the flow label from the initial SYN.
>
> Request sockets can be processed without the listener lock. A
> retransmitted SYN can therefore run tcp_v6_send_synack() while a
> concurrent final ACK creates the child in tcp_v6_syn_recv_sock(). The
> latter clones ireq->pktopts for the child and immediately consumes the
> request's reference. This can free the skb while the retransmit path is
> still reading its IPv6 header, causing a use-after-free.
>
> Request migration has the same ownership problem. inet_reqsk_clone()
> copies pktopts without taking an skb reference, and reqsk_migrate_reset()
> then clears the old request's pointer. A CPU holding the old request can
> still be sending a SYNACK when the cloned request releases the only skb
> reference.
>
> Do not consume pktopts while creating the child. Give each cloned request
> its own skb reference and let every request release its reference from
> tcp_v6_reqsk_destructor().
>
> I found this while testing IPv6 TCP request-socket lifetime handling. The
> reproducer required no source changes, artificial delay, or fault
> injection beyond enabling KASAN. It triggered the use-after-free before
> this change. With this change, a 2000-round run completed without a KASAN
> report. A separate 10-round test recorded four successful request
> migrations and no KASAN report. Networking selftests were not run because
> none directly exercises this race.
>
> Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
> Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yingjie Wang <x-professor@outlook.com>
> ---
> net/ipv4/inet_connection_sock.c | 6 +++++-
> net/ipv6/tcp_ipv6.c | 7 ++++---
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 93b306d91b6d..66f8ce25aa38 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -936,6 +936,11 @@ static struct request_sock *inet_reqsk_clone(struct request_sock *req,
> req->rsk_ops->obj_size - offsetof(struct sock, sk_dontcopy_end),
> /* alloc is larger than struct, see above */);
>
> +#if IS_ENABLED(CONFIG_IPV6)
> + if (inet_rsk(nreq)->pktopts)
> + skb_get(inet_rsk(nreq)->pktopts);
> +#endif
> +
> sk_node_init(&nreq_sk->sk_node);
> nreq_sk->sk_tx_queue_mapping = req_sk->sk_tx_queue_mapping;
> #ifdef CONFIG_SOCK_RX_QUEUE_MAPPING
> @@ -982,7 +987,6 @@ static void reqsk_migrate_reset(struct request_sock *req)
> req->saved_syn = NULL;
> #if IS_ENABLED(CONFIG_IPV6)
> inet_rsk(req)->ipv6_opt = NULL;
> - inet_rsk(req)->pktopts = NULL;
> #else
> inet_rsk(req)->ireq_opt = NULL;
> #endif
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index e006a30360ea..0b51cb4ad960 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1522,11 +1522,12 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
> if (*own_req) {
> tcp_move_syn(newtp, req);
>
> - /* Clone pktoptions received with SYN, if we own the req */
> + /* Clone pktoptions received with SYN. Keep the request's
> + * reference until the request is destroyed, as a concurrent
> + * SYN retransmit may still read it.
> + */
> if (ireq->pktopts) {
> newnp->pktoptions = skb_clone_and_charge_r(ireq->pktopts, newsk);
> - consume_skb(ireq->pktopts);
> - ireq->pktopts = NULL;
This is reported from another person in security@ mailing list
yesterday and Eric has a better fix.
pw-bot: rejected
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] tcp: ipv6: keep request pktopts alive until request destruction
2026-11-02 7:03 ` Kuniyuki Iwashima
@ 2026-11-02 7:28 ` Eric Dumazet
0 siblings, 0 replies; 3+ messages in thread
From: Eric Dumazet @ 2026-11-02 7:28 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Yingjie Wang, netdev, ncardwell, davem, kuba, pabeni, horms,
daniel, martin.lau, linux-kernel, stable
Le lun. 2 nov. 2026 à 08:03, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> On Mon, 02 Nov 2026 06:12:18 +0000, Yingjie Wang wrote:
> > When IPv6 flow label reflection is enabled, tcp_v6_send_synack() reads
> > ireq->pktopts to copy the flow label from the initial SYN.
> >
> > Request sockets can be processed without the listener lock. A
> > retransmitted SYN can therefore run tcp_v6_send_synack() while a
> > concurrent final ACK creates the child in tcp_v6_syn_recv_sock(). The
> > latter clones ireq->pktopts for the child and immediately consumes the
> > request's reference. This can free the skb while the retransmit path is
> > still reading its IPv6 header, causing a use-after-free.
> >
> > Request migration has the same ownership problem. inet_reqsk_clone()
> > copies pktopts without taking an skb reference, and reqsk_migrate_reset()
> > then clears the old request's pointer. A CPU holding the old request can
> > still be sending a SYNACK when the cloned request releases the only skb
> > reference.
> >
> > Do not consume pktopts while creating the child. Give each cloned request
> > its own skb reference and let every request release its reference from
> > tcp_v6_reqsk_destructor().
> >
> > I found this while testing IPv6 TCP request-socket lifetime handling. The
> > reproducer required no source changes, artificial delay, or fault
> > injection beyond enabling KASAN. It triggered the use-after-free before
> > this change. With this change, a 2000-round run completed without a KASAN
> > report. A separate 10-round test recorded four successful request
> > migrations and no KASAN report. Networking selftests were not run because
> > none directly exercises this race.
> >
> > Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
> > Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> > Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Yingjie Wang <x-professor@outlook.com>
> > ---
> > net/ipv4/inet_connection_sock.c | 6 +++++-
> > net/ipv6/tcp_ipv6.c | 7 ++++---
> > 2 files changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> > index 93b306d91b6d..66f8ce25aa38 100644
> > --- a/net/ipv4/inet_connection_sock.c
> > +++ b/net/ipv4/inet_connection_sock.c
> > @@ -936,6 +936,11 @@ static struct request_sock *inet_reqsk_clone(struct request_sock *req,
> > req->rsk_ops->obj_size - offsetof(struct sock, sk_dontcopy_end),
> > /* alloc is larger than struct, see above */);
> >
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + if (inet_rsk(nreq)->pktopts)
> > + skb_get(inet_rsk(nreq)->pktopts);
> > +#endif
> > +
> > sk_node_init(&nreq_sk->sk_node);
> > nreq_sk->sk_tx_queue_mapping = req_sk->sk_tx_queue_mapping;
> > #ifdef CONFIG_SOCK_RX_QUEUE_MAPPING
> > @@ -982,7 +987,6 @@ static void reqsk_migrate_reset(struct request_sock *req)
> > req->saved_syn = NULL;
> > #if IS_ENABLED(CONFIG_IPV6)
> > inet_rsk(req)->ipv6_opt = NULL;
> > - inet_rsk(req)->pktopts = NULL;
> > #else
> > inet_rsk(req)->ireq_opt = NULL;
> > #endif
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index e006a30360ea..0b51cb4ad960 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -1522,11 +1522,12 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
> > if (*own_req) {
> > tcp_move_syn(newtp, req);
> >
> > - /* Clone pktoptions received with SYN, if we own the req */
> > + /* Clone pktoptions received with SYN. Keep the request's
> > + * reference until the request is destroyed, as a concurrent
> > + * SYN retransmit may still read it.
> > + */
> > if (ireq->pktopts) {
> > newnp->pktoptions = skb_clone_and_charge_r(ireq->pktopts, newsk);
> > - consume_skb(ireq->pktopts);
> > - ireq->pktopts = NULL;
>
> This is reported from another person in security@ mailing list
> yesterday and Eric has a better fix.
>
> pw-bot: rejected
Indeed, I will add another 'Reported-by: Yingjie Wang
<x-professor@outlook.com>' tag.
Thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-02 7:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02 6:12 [PATCH net] tcp: ipv6: keep request pktopts alive until request destruction Yingjie Wang
2026-11-02 7:03 ` Kuniyuki Iwashima
2026-11-02 7:28 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®