Netdev List
 help / color / mirror / Atom feed
From: Jiayuan Chen <jiayuan.chen@linux.dev>
To: Hyunwoo Kim <imv4bel@gmail.com>,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, dsahern@kernel.org, ncardwell@google.com,
	kuniyu@google.com, horms@kernel.org, willemb@google.com,
	andrew+netdev@lunn.ch
Cc: netdev@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH net 1/3] ipv6: fix request socket use-after-free after IPV6_ADDRFORM
Date: Mon, 17 Aug 2026 20:14:03 +0800	[thread overview]
Message-ID: <0e872f9e-ee7a-41db-afbf-4bae49257bc0@linux.dev> (raw)
In-Reply-To: <20260817090319.3897799-2-imv4bel@gmail.com>


On 8/17/26 5:03 PM, Hyunwoo Kim wrote:
> IPV6_ADDRFORM turns an AF_INET6 TCP socket into an AF_INET one. It requires
> the socket to be established, and a listener can get there with
> connect(AF_UNSPEC) followed by connect(). Request sockets queued while it
> was listening are still there: inet_csk_listen_stop() leaves them in the
> ehash, and their timers only drop them while the socket is not listening,
> so making it listen again keeps them alive.
>
> A request that arrived over IPv6 was hashed with inet6_ehashfn(). Its child
> is cloned from the converted socket and hashed with inet_ehashfn(), so it
> belongs in a different bucket.
>
> inet_ehash_insert() locks the child's bucket, warns about the mismatching
> hashes, and replaces the request with the child in the request's own bucket
> anyway. reqsk_queue_unlink() locks the bucket the request is really in, so
> there is no synchronization between the two. Both can see the request still
> hashed and both can drop the reference the ehash holds.
>
> The extra put takes the request's refcount to zero too early, so it is
> freed while it is still on the listener's accept queue. The listener is
> then closed, and inet_csk_listen_stop() reads the freed request and
> writes to it in reqsk_put().
>
> Refuse the conversion if inet_csk_reqsk_queue_len() is not zero. Nothing
> clears that counter when a socket stops listening or listens again, so it
> still accounts for the requests left in the ehash. A socket that never
> listened is not affected.
>
> Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>   net/ipv6/ipv6_sockglue.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index b4c977434c2e0a..64fc6127e75332 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -572,6 +572,10 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
>   					retv = -EBUSY;
>   					break;
>   				}
> +				if (inet_csk_reqsk_queue_len(sk)) {
> +					retv = -EBUSY;
> +					break;
> +				}
>   			} else {
>   				break;
>   			}

Just thinking out loud.

Gating on inet_csk_reqsk_queue_len() reads a bit oddly, since we already 
require TCP_ESTABLISHED right above.

it's really just a proxy for "this socket used to listen and still has 
leftover requests.

The real issue is we should drain req when disconnect the listen socket, 
at least we should avoid replaces the request with the child even when 
sk->sk_hash != osk->sk_hash.

But that's the hot path, so hardening it for such a rare corner case 
isn't worth the cost.


  reply	other threads:[~2026-08-17 12:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17  9:03 [PATCH net 0/3] net: fixes for requests completing on a socket that no longer listens Hyunwoo Kim
2026-08-17  9:03 ` [PATCH net 1/3] ipv6: fix request socket use-after-free after IPV6_ADDRFORM Hyunwoo Kim
2026-08-17 12:14   ` Jiayuan Chen [this message]
2026-08-17  9:03 ` [PATCH net 2/3] net: fix out-of-bounds write in sk_clone() racing with IPV6_ADDRFORM Hyunwoo Kim
2026-08-17  9:03 ` [PATCH net 3/3] tcp: do not inherit out_of_order_queue from parent Hyunwoo Kim
2026-08-17 12:27   ` Jiayuan Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0e872f9e-ee7a-41db-afbf-4bae49257bc0@linux.dev \
    --to=jiayuan.chen@linux.dev \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=imv4bel@gmail.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox