[PATCH net] ipv4: free inet_opt after an RCU grace period

[PATCH net] ipv4: free inet_opt after an RCU grace period

[Eric Dumazet]

tcp_v4_syn_recv_sock() transfers ownership of ireq->ireq_opt to the
child socket (newinet->inet_opt) without copying it.

Another cpu can concurrently retransmit a SYNACK for the same request
socket (either from a retransmitted SYN, or from the SYNACK timer).
tcp_v4_send_synack() and inet_csk_route_req() read ireq->ireq_opt
under rcu_read_lock() only, and ip_build_and_send_pkt() and
ip_options_build() then read opt->optlen twice.

Since commit 079096f103fa ("tcp/dccp: install syn_recv requests
into ehash table"), request sockets are processed without holding
the listener lock, so nothing prevents the child socket from being
freed while the SYNACK is still being built. TCP child sockets do
not have SOCK_RCU_FREE, and inet_sock_destruct() frees inet_opt
with a plain kfree(), leading to a use-after-free in
ip_options_build().

Readers of inet_opt already use RCU, and other paths replacing
inet_opt (do_ip_setsockopt(), cipso_v4_sock_setattr()...) already use
kfree_rcu(). Use kfree_rcu() in inet_sock_destruct() as well.

IPv6 is not affected, tcp_v6_syn_recv_sock() duplicates the options.

Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
Reported-by: Xinyang Ge <xinyang@anthropic.com>
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/ipv4/af_inet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 4ce38c99fef9ef2a24edff34cd5b110dddfec193..14ce01092fda65dbcf835662c03d78f4de0301f2 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -161,7 +161,7 @@ void inet_sock_destruct(struct sock *sk)
 	WARN_ON_ONCE(sk->sk_wmem_queued);
 	WARN_ON_ONCE(sk->sk_forward_alloc);
 
-	kfree(rcu_dereference_protected(inet->inet_opt, 1));
+	kfree_rcu(rcu_dereference_protected(inet->inet_opt, 1), rcu);
 	dst_release(rcu_dereference_protected(sk->sk_dst_cache, 1));
 	dst_release(rcu_dereference_protected(sk->sk_rx_dst, 1));
 	psp_sk_assoc_free(sk);
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH net] ipv4: free inet_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:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - 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] ipv4: free inet_opt after an RCU grace period

[Jiayuan Chen]

On 9/30/26 5:43 AM, Eric Dumazet wrote:
> tcp_v4_syn_recv_sock() transfers ownership of ireq->ireq_opt to the
> child socket (newinet->inet_opt) without copying it.
>
> Another cpu can concurrently retransmit a SYNACK for the same request
> socket (either from a retransmitted SYN, or from the SYNACK timer).
> tcp_v4_send_synack() and inet_csk_route_req() read ireq->ireq_opt
> under rcu_read_lock() only, and ip_build_and_send_pkt() and
> ip_options_build() then read opt->optlen twice.
>
> Since commit 079096f103fa ("tcp/dccp: install syn_recv requests
> into ehash table"), request sockets are processed without holding
> the listener lock, so nothing prevents the child socket from being
> freed while the SYNACK is still being built. TCP child sockets do
> not have SOCK_RCU_FREE, and inet_sock_destruct() frees inet_opt
> with a plain kfree(), leading to a use-after-free in
> ip_options_build().
>
> Readers of inet_opt already use RCU, and other paths replacing
> inet_opt (do_ip_setsockopt(), cipso_v4_sock_setattr()...) already use
> kfree_rcu(). Use kfree_rcu() in inet_sock_destruct() as well.
>
> IPv6 is not affected, tcp_v6_syn_recv_sock() duplicates the options.
>
> Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> Reported-by: Xinyang Ge <xinyang@anthropic.com>
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>



For the SYNACK timer case, inet_csk_complete_hashdance() calls
timer_delete_sync() before the child can be freed, so I don't see how
that one can race. The retransmitted SYN path is the real one I think.

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

> ---
>   net/ipv4/af_inet.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 4ce38c99fef9ef2a24edff34cd5b110dddfec193..14ce01092fda65dbcf835662c03d78f4de0301f2 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -161,7 +161,7 @@ void inet_sock_destruct(struct sock *sk)
>   	WARN_ON_ONCE(sk->sk_wmem_queued);
>   	WARN_ON_ONCE(sk->sk_forward_alloc);
>   
> -	kfree(rcu_dereference_protected(inet->inet_opt, 1));
> +	kfree_rcu(rcu_dereference_protected(inet->inet_opt, 1), rcu);
>   	dst_release(rcu_dereference_protected(sk->sk_dst_cache, 1));
>   	dst_release(rcu_dereference_protected(sk->sk_rx_dst, 1));
>   	psp_sk_assoc_free(sk);

Re: [PATCH net] ipv4: free inet_opt after an RCU grace period

[Eric Dumazet]

On Wed, Sep 30, 2026 at 3:50 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
>
> On 9/30/26 5:43 AM, Eric Dumazet wrote:
> > tcp_v4_syn_recv_sock() transfers ownership of ireq->ireq_opt to the
> > child socket (newinet->inet_opt) without copying it.
> >
> > Another cpu can concurrently retransmit a SYNACK for the same request
> > socket (either from a retransmitted SYN, or from the SYNACK timer).
> > tcp_v4_send_synack() and inet_csk_route_req() read ireq->ireq_opt
> > under rcu_read_lock() only, and ip_build_and_send_pkt() and
> > ip_options_build() then read opt->optlen twice.
> >
> > Since commit 079096f103fa ("tcp/dccp: install syn_recv requests
> > into ehash table"), request sockets are processed without holding
> > the listener lock, so nothing prevents the child socket from being
> > freed while the SYNACK is still being built. TCP child sockets do
> > not have SOCK_RCU_FREE, and inet_sock_destruct() frees inet_opt
> > with a plain kfree(), leading to a use-after-free in
> > ip_options_build().
> >
> > Readers of inet_opt already use RCU, and other paths replacing
> > inet_opt (do_ip_setsockopt(), cipso_v4_sock_setattr()...) already use
> > kfree_rcu(). Use kfree_rcu() in inet_sock_destruct() as well.
> >
> > IPv6 is not affected, tcp_v6_syn_recv_sock() duplicates the options.
> >
> > Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> > Reported-by: Xinyang Ge <xinyang@anthropic.com>
> > Signed-off-by: Eric Dumazet <edumazet@kernel.org>
>
>
>
> For the SYNACK timer case, inet_csk_complete_hashdance() calls
> timer_delete_sync() before the child can be freed, so I don't see how
> that one can race. The retransmitted SYN path is the real one I think.
>
> Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>

Agreed, if a V2 is needed, I can amend the changelog.

    Another cpu can concurrently process a retransmitted SYN for the same
    request socket, and send a SYNACK from tcp_check_req().
    tcp_v4_send_synack() and inet_csk_route_req() read ireq->ireq_opt
    under rcu_read_lock() only, and ip_build_and_send_pkt() and
    ip_options_build() then read opt->optlen twice.

    Note that the SYNACK timer is not an issue, because
    inet_csk_reqsk_queue_drop() calls timer_delete_sync() before the
    child can be freed.

Re: [PATCH net] ipv4: free inet_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 · Medium: 0 · Low: 0

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.

--

ipv4: free inet_opt after an RCU grace period

This commit changes inet_sock_destruct() to free inet->inet_opt with
kfree_rcu() instead of kfree(). tcp_v4_syn_recv_sock() hands
ireq->ireq_opt to the child socket without copying it. A SYNACK
retransmit running at the same time can still be reading it under
rcu_read_lock() only.

> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 4ce38c99fef9e..14ce01092fda6 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -161,7 +161,7 @@ void inet_sock_destruct(struct sock *sk)
>  	WARN_ON_ONCE(sk->sk_wmem_queued);
>  	WARN_ON_ONCE(sk->sk_forward_alloc);
>  
> -	kfree(rcu_dereference_protected(inet->inet_opt, 1));
> +	kfree_rcu(rcu_dereference_protected(inet->inet_opt, 1), rcu);

[Severity: High]
Is there a second handoff of ireq_opt that still ends in a plain kfree()?

With net.ipv4.tcp_migrate_req=1 and SO_REUSEPORT listeners,
reqsk_timer_handler() migrates a request socket whose listener has been
closed:

net/ipv4/inet_connection_sock.c:reqsk_timer_handler() {
    ...
		nreq = inet_reqsk_clone(req, nsk);
    ...
		if (!inet_ehash_insert(req_to_sk(nreq), req_to_sk(oreq), NULL)) {
    ...
		reqsk_migrate_reset(oreq);
    ...
}

inet_reqsk_clone() memcpy()s the request socket, so nreq->ireq_opt points
to the same ip_options_rcu as oreq. reqsk_migrate_reset() then clears
oreq's slot with a plain NULL store, and nreq becomes the owner.

At the same time, another CPU can hold a reference on oreq in
tcp_v4_rcv(), pick a new listener through reuseport_migrate_sock(), and
handle a retransmitted SYN:

tcp_v4_rcv()
  tcp_check_req()
    tcp_rtx_synack()
      tcp_v4_send_synack()
        rcu_dereference(ireq->ireq_opt)
        ip_build_and_send_pkt()
          ip_options_build()

nreq can then be dropped, for example by an RST through
tcp_check_req()->inet_csk_reqsk_queue_drop()->reqsk_put(), or when its
own timer expires. When that happens, the options are freed without a
grace period:

net/ipv4/tcp_ipv4.c:tcp_v4_reqsk_destructor() {
	kfree(rcu_dereference_protected(inet_rsk(req)->ireq_opt, 1));
}

The reference on oreq no longer protects the options, because oreq does
not own them anymore. The reader's rcu_read_lock() gives no protection
against a plain kfree() either.

Can this lead to the same use-after-free in ip_build_and_send_pkt() and
ip_options_build()? Both read opt->optlen, so if the memory is reallocated
in between, the two reads could disagree. That could turn into an
out-of-bounds write while the SYNACK is being built.

The commit message says the other paths that replace inet_opt already
use kfree_rcu(). Should tcp_v4_reqsk_destructor() also be converted to
kfree_rcu(..., rcu) as part of this fix?

>  	dst_release(rcu_dereference_protected(sk->sk_dst_cache, 1));
>  	dst_release(rcu_dereference_protected(sk->sk_rx_dst, 1));
>  	psp_sk_assoc_free(sk);

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

Re: [PATCH net] ipv4: free inet_opt after an RCU grace period

[Eric Dumazet]

On Thu, Oct 1, 2026 at 11:45 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
>
> 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.

Good catch, tcp_v4_reqsk_destructor() needs the same treatment
after a request migration. I will send a v2.

Not sure about the 'High' classification, as the race would involve 3 cpus.

pw-bot: cr