[PATCH net] tcp: ipv6: free ireq->ipv6_opt after an RCU grace period

[PATCH net] tcp: ipv6: free ireq->ipv6_opt after an RCU grace period

[Eric Dumazet]

ireq->ipv6_opt is only set by calipso_req_setattr(), when a CALIPSO
NetLabel mapping is configured.

Request socket migration (net.ipv4.tcp_migrate_req=1, or a
BPF_SK_REUSEPORT_SELECT_OR_MIGRATE program) clones the request
socket with inet_reqsk_clone(), so that the old request socket and
its clone share the same ipv6_opt, then reqsk_migrate_reset() clears
the pointer in the old one.

Another cpu holding a reference on the old request socket can still
be using these options, sending a SYNACK from tcp_v6_send_synack()
or creating a child in tcp_v6_syn_recv_sock(), when the clone
is freed. tcp_v6_reqsk_destructor() frees ipv6_opt with a plain
kfree(), leading to a use-after-free.

Both readers already run under RCU, like their IPv4 counterparts.
Use kfree_rcu() in tcp_v6_reqsk_destructor(), as commit e3f33b0a1d89
("ipv4: free inet_opt and ireq_opt after an RCU grace period") did
for ireq_opt.

Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/netdev/179369559062.488856.15441115725436277776@kernel.org/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
Found by Sashiko while reviewing another TCP fix :/

 net/ipv6/tcp_ipv6.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index d406cd3c8ceabfdbb66f3e1b4f9e9a74754e8d0b..b1dec5d86d633db67a754c70ee1c63290a9acb5a 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -607,7 +607,7 @@ static int tcp_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
 
 static void tcp_v6_reqsk_destructor(struct request_sock *req)
 {
-	kfree(inet_rsk(req)->ipv6_opt);
+	kfree_rcu(inet_rsk(req)->ipv6_opt, rcu);
 	consume_skb(inet_rsk(req)->pktopts);
 }
 
-- 
2.53.0

Re: [PATCH net] tcp: ipv6: free ireq->ipv6_opt after an RCU grace period

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] tcp: ipv6: free ireq->ipv6_opt after an RCU grace period

[Eric Dumazet]

Le mar. 3 nov. 2026 à 10:09, <netdev-bot+sinfo@kernel.org> a écrit :
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
>

I think Sashiko is an LLM.

I do not think it has triggered the bug, but it seems plausible to me.

> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Re: [PATCH net] tcp: ipv6: free ireq->ipv6_opt after an RCU grace period

[Hangbin Liu]

On Tue, Nov 03, 2026 at 10:00:11AM +0100, Eric Dumazet wrote:
> ireq->ipv6_opt is only set by calipso_req_setattr(), when a CALIPSO
> NetLabel mapping is configured.
> 
> Request socket migration (net.ipv4.tcp_migrate_req=1, or a
> BPF_SK_REUSEPORT_SELECT_OR_MIGRATE program) clones the request
> socket with inet_reqsk_clone(), so that the old request socket and
> its clone share the same ipv6_opt, then reqsk_migrate_reset() clears
> the pointer in the old one.
> 
> Another cpu holding a reference on the old request socket can still
> be using these options, sending a SYNACK from tcp_v6_send_synack()
> or creating a child in tcp_v6_syn_recv_sock(), when the clone
> is freed. tcp_v6_reqsk_destructor() frees ipv6_opt with a plain
> kfree(), leading to a use-after-free.
> 
> Both readers already run under RCU, like their IPv4 counterparts.
> Use kfree_rcu() in tcp_v6_reqsk_destructor(), as commit e3f33b0a1d89
> ("ipv4: free inet_opt and ireq_opt after an RCU grace period") did
> for ireq_opt.
> 
> Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/netdev/179369559062.488856.15441115725436277776@kernel.org/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---
> Found by Sashiko while reviewing another TCP fix :/
> 
>  net/ipv6/tcp_ipv6.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index d406cd3c8ceabfdbb66f3e1b4f9e9a74754e8d0b..b1dec5d86d633db67a754c70ee1c63290a9acb5a 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -607,7 +607,7 @@ static int tcp_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
>  
>  static void tcp_v6_reqsk_destructor(struct request_sock *req)
>  {
> -	kfree(inet_rsk(req)->ipv6_opt);
> +	kfree_rcu(inet_rsk(req)->ipv6_opt, rcu);
>  	consume_skb(inet_rsk(req)->pktopts);
>  }
>  
> -- 
> 2.53.0
> 

Cc Leonardo for the fixup.

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>

Re: [PATCH net] tcp: ipv6: free ireq->ipv6_opt after an RCU grace period

[Jiayuan Chen]

On 11/3/26 5:00 PM, Eric Dumazet wrote:
> ireq->ipv6_opt is only set by calipso_req_setattr(), when a CALIPSO
> NetLabel mapping is configured.
>
> Request socket migration (net.ipv4.tcp_migrate_req=1, or a
> BPF_SK_REUSEPORT_SELECT_OR_MIGRATE program) clones the request
> socket with inet_reqsk_clone(), so that the old request socket and
> its clone share the same ipv6_opt, then reqsk_migrate_reset() clears
> the pointer in the old one.
>
> Another cpu holding a reference on the old request socket can still
> be using these options, sending a SYNACK from tcp_v6_send_synack()
> or creating a child in tcp_v6_syn_recv_sock(), when the clone
> is freed. tcp_v6_reqsk_destructor() frees ipv6_opt with a plain
> kfree(), leading to a use-after-free.
>
> Both readers already run under RCU, like their IPv4 counterparts.
> Use kfree_rcu() in tcp_v6_reqsk_destructor(), as commit e3f33b0a1d89
> ("ipv4: free inet_opt and ireq_opt after an RCU grace period") did
> for ireq_opt.
>
> Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/netdev/179369559062.488856.15441115725436277776@kernel.org/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>


Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>


> ---
> Found by Sashiko while reviewing another TCP fix :/
>
>  net/ipv6/tcp_ipv6.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index d406cd3c8ceabfdbb66f3e1b4f9e9a74754e8d0b..b1dec5d86d633db67a754c70ee1c63290a9acb5a 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -607,7 +607,7 @@ static int tcp_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
>  
>  static void tcp_v6_reqsk_destructor(struct request_sock *req)
>  {
> -	kfree(inet_rsk(req)->ipv6_opt);
> +	kfree_rcu(inet_rsk(req)->ipv6_opt, rcu);
>  	consume_skb(inet_rsk(req)->pktopts);
>  }
>  

Re: [PATCH net] tcp: ipv6: free ireq->ipv6_opt after an RCU grace period

[Kuniyuki Iwashima]

On Tue, 03 Nov 2026 10:00:11 +0100, Eric Dumazet wrote:
> ireq->ipv6_opt is only set by calipso_req_setattr(), when a CALIPSO
> NetLabel mapping is configured.
> 
> Request socket migration (net.ipv4.tcp_migrate_req=1, or a
> BPF_SK_REUSEPORT_SELECT_OR_MIGRATE program) clones the request
> socket with inet_reqsk_clone(), so that the old request socket and
> its clone share the same ipv6_opt, then reqsk_migrate_reset() clears
> the pointer in the old one.
> 
> Another cpu holding a reference on the old request socket can still
> be using these options, sending a SYNACK from tcp_v6_send_synack()
> or creating a child in tcp_v6_syn_recv_sock(), when the clone
> is freed. tcp_v6_reqsk_destructor() frees ipv6_opt with a plain
> kfree(), leading to a use-after-free.
> 
> Both readers already run under RCU, like their IPv4 counterparts.
> Use kfree_rcu() in tcp_v6_reqsk_destructor(), as commit e3f33b0a1d89
> ("ipv4: free inet_opt and ireq_opt after an RCU grace period") did
> for ireq_opt.
> 
> Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/netdev/179369559062.488856.15441115725436277776@kernel.org/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net] tcp: ipv6: free ireq->ipv6_opt after an RCU grace period

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete.

Pre-existing issues:
- [High] The ipv6_opt attached to a request socket is charged to the
  listener's sk_omem_alloc and never uncharged.
- [Low] After this patch, ireq->ipv6_opt is freed after an RCU grace
  period, and the commit message says its readers 'already run under…

--

tcp: ipv6: free ireq->ipv6_opt after an RCU grace period

When a request socket is migrated, the old request socket and its clone
share ireq->ipv6_opt. If the clone is freed while another CPU is still
using the options through the old request socket, that is a
use-after-free. The patch frees ipv6_opt with kfree_rcu() in
tcp_v6_reqsk_destructor(), as was already done for the IPv4 ireq_opt.

Closes: https://lore.kernel.org/netdev/179369559062.488856.15441115725436277776@kernel.org/

> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index e006a30360eae..517f34166cda6 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -607,7 +607,7 @@ static int tcp_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
>  
>  static void tcp_v6_reqsk_destructor(struct request_sock *req)
>  {
> -	kfree(inet_rsk(req)->ipv6_opt);
> +	kfree_rcu(inet_rsk(req)->ipv6_opt, rcu);

[Severity: High]
This is a pre-existing issue and was not introduced by this patch, but it
is on the line being changed. Is the listener's sk_omem_alloc charge for
ipv6_opt ever released?

calipso_req_setattr() charges the listener, sk_to_full_sk(req), through
ipv6_renew_options()->sock_kmalloc(). It only uncharges the options that
it replaces:

net/ipv6/calipso.c:calipso_req_setattr() {
    ...
	txopts = ipv6_renew_options(sk, req_inet->ipv6_opt, IPV6_HOPOPTS, new);
    ...
	txopts = xchg(&req_inet->ipv6_opt, txopts);
	if (txopts) {
		atomic_sub(txopts->tot_len, &sk->sk_omem_alloc);
		txopt_put(txopts);
	}
    ...
}

calipso_req_delattr() uncharges in the same way. tcp_v6_reqsk_destructor()
does the final free of ipv6_opt with no atomic_sub(), both with kfree()
before this patch and with kfree_rcu() after it.

tcp_v6_syn_recv_sock() charges the child separately through
ipv6_dup_options(newsk, ...), so the listener's charge stays in place.

On a listener with a CALIPSO label, does sk_omem_alloc grow by roughly
80-110 bytes for each connection request? With the default optmem_max of
128KB, sock_kmalloc() would start failing after about 1300 requests. Then
ipv6_renew_options() returns -ENOBUFS, and every later SYN is dropped by
the LSM hook in tcp_v6_route_req():

	if (security_inet_conn_request(sk, skb, req))
		return NULL;

When the listener is destroyed, __sk_destruct() would also report optmem
leakage.

[Severity: Low]
This isn't a bug introduced by this patch, but the commit message says
that both readers "already run under RCU, like their IPv4 counterparts".
Now that ipv6_opt is freed after a grace period, should it get the
matching RCU annotations?

In struct inet_request_sock, ipv6_opt is a plain pointer, while the IPv4
member of the same union is __rcu:

	union {
		struct ip_options_rcu __rcu	*ireq_opt;
#if IS_ENABLED(CONFIG_IPV6)
		struct {
			struct ipv6_txoptions	*ipv6_opt;

Both IPv6 readers load it with a plain read. For example, in
tcp_v6_syn_recv_sock():

	opt = ireq->ipv6_opt;
	if (!opt)
		opt = rcu_dereference(np->opt);
	if (opt) {
		opt = ipv6_dup_options(newsk, opt);

The IPv4 readers use rcu_dereference(ireq->ireq_opt). Meanwhile,
reqsk_migrate_reset() writes NULL to the same field with a plain store,
on a request that other CPUs may still be reading.

KCSAN would report this as a data race, and sparse cannot check the RCU
usage. In theory the compiler could reload ireq->ipv6_opt after the NULL
test and pass NULL to ipv6_dup_options().

Would it make sense to add __rcu, rcu_dereference() and RCU_INIT_POINTER()
on top of the kfree_rcu() change?

>  	consume_skb(inet_rsk(req)->pktopts);

[Severity: High]
Does this fully close the window described in the commit message?

After inet_reqsk_clone(), the old request and its clone also share the
same inet_rsk(req)->pktopts skb. reqsk_migrate_reset() clears it together
with ipv6_opt:

net/ipv4/inet_connection_sock.c:reqsk_migrate_reset() {
    ...
	inet_rsk(req)->ipv6_opt = NULL;
	inet_rsk(req)->pktopts = NULL;
    ...
}

pktopts is still freed right away, both here and in tcp_v6_syn_recv_sock()
when that request wins own_req:

net/ipv6/tcp_ipv6.c:tcp_v6_syn_recv_sock() {
    ...
	if (*own_req) {
		...
		if (ireq->pktopts) {
			newnp->pktoptions = skb_clone_and_charge_r(ireq->pktopts, newsk);
			consume_skb(ireq->pktopts);
			ireq->pktopts = NULL;
    ...
}

At the same time, tcp_v6_send_synack() on the sibling request can read it
without any RCU protection:

net/ipv6/tcp_ipv6.c:tcp_v6_send_synack() {
    ...
		if (inet6_test_bit(REPFLOW, sk) && ireq->pktopts)
			fl6->flowlabel = ip6_flowlabel(ipv6_hdr(ireq->pktopts));
    ...
}

tcp_v6_init_req() keeps pktopts whenever REPFLOW (or the common rxopt
bits) is set on the listener, so this does not need CALIPSO.

With REPFLOW set, tcp_migrate_req=1 (or a BPF migrate program), and a
listener that is closing, one possible sequence is:

CPU A                                CPU C
reqsk_timer_handler(oreq)
  nreq = inet_reqsk_clone(oreq, nsk)
  tcp_rtx_synack(nsk, nreq)
    tcp_v6_send_synack()             tcp_check_req(oreq) (final ACK)
                                       tcp_v6_syn_recv_sock(oreq)
                                         own_req = true
                                         consume_skb(oreq->pktopts)
      ipv6_hdr(ireq->pktopts)  <- skb already freed

The timer's reference on oreq protects ipv6_opt in this case, but it does
not protect pktopts.

The reverse order also looks possible. CPU C handles a retransmitted SYN
on oreq and calls tcp_check_req()->tcp_rtx_synack()->tcp_v6_send_synack().
At the same time, CPU A has done inet_ehash_insert(nreq, oreq) but not yet
reqsk_migrate_reset(oreq). CPU B then processes the final ACK on nreq and
frees the shared skb in tcp_v6_syn_recv_sock().

In both cases ipv6_hdr() reads from a freed sk_buff. That can oops, or it
can put stale bits into the flow label of the SYNACK.

tcp_v6_syn_recv_sock() frees the skb directly, so switching to an RCU
deferred free would not help here. Does pktopts need its own handling,
for example an skb_get() for the clone, so that each request drops only
its own reference?

>  }
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103090011.4128420-1-edumazet%40kernel.org

Re: [PATCH net] tcp: ipv6: free ireq->ipv6_opt after an RCU grace period

[Eric Dumazet]

Le mer. 4 nov. 2026 à 10:02, <netdev-bot+sashiko@kernel.org> a écrit :
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0 (1)
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] The fix is incomplete.
>
> Pre-existing issues:
> - [High] The ipv6_opt attached to a request socket is charged to the
>   listener's sk_omem_alloc and never uncharged.
> - [Low] After this patch, ireq->ipv6_opt is freed after an RCU grace
>   period, and the commit message says its readers 'already run under…
>
> --
>
> tcp: ipv6: free ireq->ipv6_opt after an RCU grace period
>
> When a request socket is migrated, the old request socket and its clone
> share ireq->ipv6_opt. If the clone is freed while another CPU is still
> using the options through the old request socket, that is a
> use-after-free. The patch frees ipv6_opt with kfree_rcu() in
> tcp_v6_reqsk_destructor(), as was already done for the IPv4 ireq_opt.
>
> Closes: https://lore.kernel.org/netdev/179369559062.488856.15441115725436277776@kernel.org/
>
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index e006a30360eae..517f34166cda6 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -607,7 +607,7 @@ static int tcp_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
> >
> >  static void tcp_v6_reqsk_destructor(struct request_sock *req)
> >  {
> > -     kfree(inet_rsk(req)->ipv6_opt);
> > +     kfree_rcu(inet_rsk(req)->ipv6_opt, rcu);
>
> [Severity: High]
> This is a pre-existing issue and was not introduced by this patch, but it
> is on the line being changed. Is the listener's sk_omem_alloc charge for
> ipv6_opt ever released?


Right, this is a separate CALIPSO issue: the listener's sk_omem_alloc
is never uncharged when the request socket is freed.

Uncharging in tcp_v6_reqsk_destructor() would not be correct after a
migration, because req->rsk_listener is no longer the socket that was
charged. calipso_req_setattr() and calipso_req_delattr() should
not keep a charge on the listener at all, like cipso_v4_req_setattr()
which uses kzalloc().

I will send a separate patch.
But only after the pending patches are merged, see below.

>
> calipso_req_setattr() charges the listener, sk_to_full_sk(req), through
> ipv6_renew_options()->sock_kmalloc(). It only uncharges the options that
> it replaces:
>
> net/ipv6/calipso.c:calipso_req_setattr() {
>     ...
>         txopts = ipv6_renew_options(sk, req_inet->ipv6_opt, IPV6_HOPOPTS, new);
>     ...
>         txopts = xchg(&req_inet->ipv6_opt, txopts);
>         if (txopts) {
>                 atomic_sub(txopts->tot_len, &sk->sk_omem_alloc);
>                 txopt_put(txopts);
>         }
>     ...
> }
>
> calipso_req_delattr() uncharges in the same way. tcp_v6_reqsk_destructor()
> does the final free of ipv6_opt with no atomic_sub(), both with kfree()
> before this patch and with kfree_rcu() after it.
>
> tcp_v6_syn_recv_sock() charges the child separately through
> ipv6_dup_options(newsk, ...), so the listener's charge stays in place.
>
> On a listener with a CALIPSO label, does sk_omem_alloc grow by roughly
> 80-110 bytes for each connection request? With the default optmem_max of
> 128KB, sock_kmalloc() would start failing after about 1300 requests. Then
> ipv6_renew_options() returns -ENOBUFS, and every later SYN is dropped by
> the LSM hook in tcp_v6_route_req():
>
>         if (security_inet_conn_request(sk, skb, req))
>                 return NULL;
>
> When the listener is destroyed, __sk_destruct() would also report optmem
> leakage.
>
> [Severity: Low]
> This isn't a bug introduced by this patch, but the commit message says
> that both readers "already run under RCU, like their IPv4 counterparts".
> Now that ipv6_opt is freed after a grace period, should it get the
> matching RCU annotations?

This is net-next material.

>
> In struct inet_request_sock, ipv6_opt is a plain pointer, while the IPv4
> member of the same union is __rcu:
>
>         union {
>                 struct ip_options_rcu __rcu     *ireq_opt;
> #if IS_ENABLED(CONFIG_IPV6)
>                 struct {
>                         struct ipv6_txoptions   *ipv6_opt;
>
> Both IPv6 readers load it with a plain read. For example, in
> tcp_v6_syn_recv_sock():
>
>         opt = ireq->ipv6_opt;
>         if (!opt)
>                 opt = rcu_dereference(np->opt);
>         if (opt) {
>                 opt = ipv6_dup_options(newsk, opt);
>
> The IPv4 readers use rcu_dereference(ireq->ireq_opt). Meanwhile,
> reqsk_migrate_reset() writes NULL to the same field with a plain store,
> on a request that other CPUs may still be reading.
>
> KCSAN would report this as a data race, and sparse cannot check the RCU
> usage. In theory the compiler could reload ireq->ipv6_opt after the NULL
> test and pass NULL to ipv6_dup_options().
>
> Would it make sense to add __rcu, rcu_dereference() and RCU_INIT_POINTER()
> on top of the kfree_rcu() change?
>
> >       consume_skb(inet_rsk(req)->pktopts);
>
> [Severity: High]
> Does this fully close the window described in the commit message?
>
> After inet_reqsk_clone(), the old request and its clone also share the
> same inet_rsk(req)->pktopts skb. reqsk_migrate_reset() clears it together
> with ipv6_opt:
>
> net/ipv4/inet_connection_sock.c:reqsk_migrate_reset() {
>     ...
>         inet_rsk(req)->ipv6_opt = NULL;
>         inet_rsk(req)->pktopts = NULL;
>     ...
> }
>
> pktopts is still freed right away, both here and in tcp_v6_syn_recv_sock()
> when that request wins own_req:
>
> net/ipv6/tcp_ipv6.c:tcp_v6_syn_recv_sock() {
>     ...
>         if (*own_req) {
>                 ...
>                 if (ireq->pktopts) {
>                         newnp->pktoptions = skb_clone_and_charge_r(ireq->pktopts, newsk);
>                         consume_skb(ireq->pktopts);
>                         ireq->pktopts = NULL;
>     ...
> }
>
> At the same time, tcp_v6_send_synack() on the sibling request can read it
> without any RCU protection:
>
> net/ipv6/tcp_ipv6.c:tcp_v6_send_synack() {
>     ...
>                 if (inet6_test_bit(REPFLOW, sk) && ireq->pktopts)
>                         fl6->flowlabel = ip6_flowlabel(ipv6_hdr(ireq->pktopts));
>     ...
> }
>
> tcp_v6_init_req() keeps pktopts whenever REPFLOW (or the common rxopt
> bits) is set on the listener, so this does not need CALIPSO.
>
> With REPFLOW set, tcp_migrate_req=1 (or a BPF migrate program), and a
> listener that is closing, one possible sequence is:
>
> CPU A                                CPU C
> reqsk_timer_handler(oreq)
>   nreq = inet_reqsk_clone(oreq, nsk)
>   tcp_rtx_synack(nsk, nreq)
>     tcp_v6_send_synack()             tcp_check_req(oreq) (final ACK)
>                                        tcp_v6_syn_recv_sock(oreq)
>                                          own_req = true
>                                          consume_skb(oreq->pktopts)
>       ipv6_hdr(ireq->pktopts)  <- skb already freed
>
> The timer's reference on oreq protects ipv6_opt in this case, but it does
> not protect pktopts.
>
> The reverse order also looks possible. CPU C handles a retransmitted SYN
> on oreq and calls tcp_check_req()->tcp_rtx_synack()->tcp_v6_send_synack().
> At the same time, CPU A has done inet_ehash_insert(nreq, oreq) but not yet
> reqsk_migrate_reset(oreq). CPU B then processes the final ACK on nreq and
> frees the shared skb in tcp_v6_syn_recv_sock().
>
> In both cases ipv6_hdr() reads from a freed sk_buff. That can oops, or it
> can put stale bits into the flow label of the SYNACK.
>
> tcp_v6_syn_recv_sock() frees the skb directly, so switching to an RCU
> deferred free would not help here. Does pktopts need its own handling,
> for example an skb_get() for the clone, so that each request drops only
> its own reference?

pktopts is handled by the other pending patch, "tcp: ipv6: do not
read ireq->pktopts from tcp_v6_send_synack()" [1]

With it, tcp_v6_send_synack() no longer reads ireq->pktopts.

The only remaining user is tcp_v6_syn_recv_sock(), and only when it
wins the ehash insertion (*own_req == true). Only one of the old
request and its clone can win. When the migration fails,
reqsk_migrate_reset(nreq) clears the clone's pointer, so the skb is
never freed twice.

The two patches fix two different fields, and can be applied
independently, in any order.

[1] https://lore.kernel.org/netdev/20261102075441.3771550-1-edumazet@kernel.org/

Let's merge these fixes, so that I can proceed with more urgent bugs.

Re: [PATCH net] tcp: ipv6: free ireq->ipv6_opt after an RCU grace period

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

Hi Eric,

I wrote a reproducer for this and ran it on both sides of your patch.

Setup
-----
v7.4-rc1 (6628e7d4c6b) on an x86_64 VMware guest with 2 vCPUs,
CONFIG_KASAN=y (generic), CONFIG_SLUB_DEBUG=y, PREEMPT_LAZY. The working
tree only carries the instrumentation described below and your patch (it
reports itself as 7.4.0-rc1-dirty); nothing else differs between the two
runs.

ireq->ipv6_opt has a single in-tree producer (calipso_req_setattr()),
which needs CALIPSO + NetLabel + an MLS SELinux policy, so to keep the
test self-contained I added a debug-only patch, never meant for upstream,
that

  - allocates a fake ipv6_txoptions in tcp_v6_route_req() and stores it
    in ireq->ipv6_opt, i.e. exactly the slot calipso_req_setattr() fills;
  - does an mdelay() inside the rcu_read_lock() section of
    tcp_v6_send_synack(), right after "opt = ireq->ipv6_opt;".

The trigger is a SO_REUSEPORT listener group on ::1 with
net.ipv4.tcp_migrate_req=1: a crafted SYN from a raw socket, a second
listener joining the group, the first one closing so that the request
socket migrates and the clone takes over ipv6_opt, the second listener
closing, and a retransmitted SYN then handled on the other CPU while that
CPU sits in the (deliberately widened) RCU section.

The instrumentation also makes the two sides measurable, which is what
the timestamps below come from: the reader is parked for 3.2s with the
pointer already in a local variable, and the destructor runs in the
middle of that window.

Without your patch
------------------
  t+0.30s  SYNACK #2 on CPU1: rcu_read_lock(), opt = ireq->ipv6_opt,
           then stalled for 3.2s
  t+1.00s  reqsk_timer_handler(): oreq is migrated to the second
           listener, inet_reqsk_clone() makes the clone share ipv6_opt,
           reqsk_migrate_reset(oreq) clears oreq's pointer
  t+3.09s  the clone's timer fires with no listener left to migrate to:
           __reqsk_free() -> tcp_v6_reqsk_destructor() -> kfree()
  t+3.51s  CPU1 resumes and ip6_xmit() reads the options

  BUG: KASAN: slab-use-after-free in ip6_xmit+0x1301/0x19a0
  Read of size 2 at addr ffff88801497908a by task poc/5786
  CPU: 1 UID: 0 PID: 5786 Comm: poc
  Call Trace:
   <IRQ>
   ip6_xmit+0x1301/0x19a0
   tcp_v6_send_synack+0x631/0xbe0
   tcp_rtx_synack+0x116/0x4a0
   tcp_check_req+0xb37/0x23c0
   tcp_v6_rcv+0xfff/0x34a0
  ...
  Freed by task 0 on cpu 0:
   kfree+0x1b1/0x510
   tcp_v6_reqsk_destructor+0x37/0x70
   __reqsk_free+0xd4/0x230
   reqsk_timer_handler+0x83a/0xe10
  ...
  The buggy address belongs to the cache kmalloc-96 of size 96

The dereference lands 416 ms after the free here, which is what makes the
ordering unambiguous; in a real deployment the window is a few
instructions wide, but nothing else changes.

With your patch
---------------
Same parameters, two rounds: the timeline does not move at all (the
destructor still runs at t+3.09s, the reader still dereferences the
pointer at t+3.51s) and there is no report whatsoever. kfree_rcu()
cannot complete a grace period while CPU1 is inside rcu_read_lock(),
which is what the changelog says.

(Note that KASAN reports only once per boot unless kasan_multi_shot is
set, so on the unpatched kernel the second round is silent by
construction. The fixed kernel being silent from the very first round is
the part that matters.)

If you end up respinning for the pktopts issue Sashiko raised, I can
re-run this on the new version. The reproducer and the debug-only patch
are easy to share if anyone wants to look at them.

Tested-by: Linkui Xiao <xiaolinkui@kylinos.cn>