[PATCH v1 net] udp: Unhash auto-bound connected sk from 4-tuple hash table when disconnected.

[PATCH v1 net] udp: Unhash auto-bound connected sk from 4-tuple hash table when disconnected.

[Kuniyuki Iwashima]

Let's say we bind() an UDP socket to the wildcard address with a
non-zero port, connect() it to an address, and disconnect it from
the address.

bind() sets SOCK_BINDPORT_LOCK on sk->sk_userlocks (but not
SOCK_BINDADDR_LOCK), and connect() calls udp_lib_hash4() to put
the socket into the 4-tuple hash table.

Then, __udp_disconnect() clears the source address and calls
sk->sk_prot->rehash(sk).

It computes a new hash based on the wildcard address and moves
the socket to a new slot in the 4-tuple hash table, leaving a
garbage in the chain that no packet hits.

Let's remove such a socket from 4-tuple hash table when disconnected.

Fixes: 78c91ae2c6de ("ipv4/udp: Add 4-tuple hash for connected socket")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/udp.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index b96e47f1c8a2..3388e4ab9056 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2321,15 +2321,19 @@ void udp_lib_rehash(struct sock *sk, u16 newhash, u16 newhash4)
 		if (udp_hashed4(sk)) {
 			spin_lock_bh(&hslot->lock);
 
-			udp_rehash4(udptable, sk, newhash4);
-			if (hslot2 != nhslot2) {
-				spin_lock(&hslot2->lock);
-				udp_hash4_dec(hslot2);
-				spin_unlock(&hslot2->lock);
-
-				spin_lock(&nhslot2->lock);
-				udp_hash4_inc(nhslot2);
-				spin_unlock(&nhslot2->lock);
+			if (inet_rcv_saddr_any(sk)) {
+				udp_unhash4(udptable, sk);
+			} else {
+				udp_rehash4(udptable, sk, newhash4);
+				if (hslot2 != nhslot2) {
+					spin_lock(&hslot2->lock);
+					udp_hash4_dec(hslot2);
+					spin_unlock(&hslot2->lock);
+
+					spin_lock(&nhslot2->lock);
+					udp_hash4_inc(nhslot2);
+					spin_unlock(&nhslot2->lock);
+				}
 			}
 
 			spin_unlock_bh(&hslot->lock);
-- 
2.53.0.414.gf7e9f6c205-goog

Re: [v1,net] udp: Unhash auto-bound connected sk from 4-tuple hash table when disconnected.

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
udp: Unhash auto-bound connected sk from 4-tuple hash table when disconnected.

This commit fixes a bug where disconnecting an auto-bound connected UDP
socket leaves a garbage entry in the 4-tuple hash table. The socket is
now properly removed from the hash table when disconnected.

> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index b96e47f1c8a2..3388e4ab9056 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -2321,15 +2321,19 @@ void udp_lib_rehash(struct sock *sk, u16 newhash, u16 newhash4)
>  		if (udp_hashed4(sk)) {
>  			spin_lock_bh(&hslot->lock);
>
> -			udp_rehash4(udptable, sk, newhash4);
> -			if (hslot2 != nhslot2) {
> -				spin_lock(&hslot2->lock);
> -				udp_hash4_dec(hslot2);
> -				spin_unlock(&hslot2->lock);
> -
> -				spin_lock(&nhslot2->lock);
> -				udp_hash4_inc(nhslot2);
> -				spin_unlock(&nhslot2->lock);
> +			if (inet_rcv_saddr_any(sk)) {
> +				udp_unhash4(udptable, sk);
> +			} else {
> +				udp_rehash4(udptable, sk, newhash4);
> +				if (hslot2 != nhslot2) {
> +					spin_lock(&hslot2->lock);
> +					udp_hash4_dec(hslot2);
> +					spin_unlock(&hslot2->lock);
> +
> +					spin_lock(&nhslot2->lock);
> +					udp_hash4_inc(nhslot2);
> +					spin_unlock(&nhslot2->lock);
> +				}
>  			}
>
>  			spin_unlock_bh(&hslot->lock);

Is there a hash4_cnt accounting mismatch when inet_rcv_saddr_any(sk)
is true? The udp_sk(sk)->udp_portaddr_hash is updated to newhash at
the start of the function (before this block):

void udp_lib_rehash(struct sock *sk, u16 newhash, u16 newhash4)
{
	struct udp_table *udptable = udp_get_table_prot(sk);
	...
	udp_sk(sk)->udp_portaddr_hash = newhash;
	...
}

When the new code calls udp_unhash4() on the inet_rcv_saddr_any(sk)
path, udp_unhash4() computes its hslot2 from the already-updated
udp_portaddr_hash (nhslot2):

void udp_unhash4(struct udp_table *udptable, struct sock *sk)
{
	struct udp_hslot *hslot2 = udp_hashslot2(udptable, udp_sk(sk)->udp_portaddr_hash);
	...
	udp_hash4_dec(hslot2);
}

But the original udp_lib_hash4() call during connect() would have
incremented hash4_cnt on the old hslot2, not nhslot2. This seems to
decrement hash4_cnt on the wrong 2-tuple slot when hslot2 != nhslot2,
which is common for wildcard-bind + connect + disconnect.

The else branch correctly uses udp_hash4_dec(hslot2) on the old slot
and udp_hash4_inc(nhslot2) on the new slot.

While the impact is minor since hash4_cnt is a u32 optimization hint,
the counter becomes permanently incorrect and accumulates error across
connect/disconnect cycles.

Re: [v1,net] udp: Unhash auto-bound connected sk from 4-tuple hash table when disconnected.

[Kuniyuki Iwashima]

On Thu, Feb 26, 2026 at 7:40 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> udp: Unhash auto-bound connected sk from 4-tuple hash table when disconnected.
>
> This commit fixes a bug where disconnecting an auto-bound connected UDP
> socket leaves a garbage entry in the 4-tuple hash table. The socket is
> now properly removed from the hash table when disconnected.
>
> > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > index b96e47f1c8a2..3388e4ab9056 100644
> > --- a/net/ipv4/udp.c
> > +++ b/net/ipv4/udp.c
> > @@ -2321,15 +2321,19 @@ void udp_lib_rehash(struct sock *sk, u16 newhash, u16 newhash4)
> >               if (udp_hashed4(sk)) {
> >                       spin_lock_bh(&hslot->lock);
> >
> > -                     udp_rehash4(udptable, sk, newhash4);
> > -                     if (hslot2 != nhslot2) {
> > -                             spin_lock(&hslot2->lock);
> > -                             udp_hash4_dec(hslot2);
> > -                             spin_unlock(&hslot2->lock);
> > -
> > -                             spin_lock(&nhslot2->lock);
> > -                             udp_hash4_inc(nhslot2);
> > -                             spin_unlock(&nhslot2->lock);
> > +                     if (inet_rcv_saddr_any(sk)) {
> > +                             udp_unhash4(udptable, sk);
> > +                     } else {
> > +                             udp_rehash4(udptable, sk, newhash4);
> > +                             if (hslot2 != nhslot2) {
> > +                                     spin_lock(&hslot2->lock);
> > +                                     udp_hash4_dec(hslot2);
> > +                                     spin_unlock(&hslot2->lock);
> > +
> > +                                     spin_lock(&nhslot2->lock);
> > +                                     udp_hash4_inc(nhslot2);
> > +                                     spin_unlock(&nhslot2->lock);
> > +                             }
> >                       }
> >
> >                       spin_unlock_bh(&hslot->lock);
>
> Is there a hash4_cnt accounting mismatch when inet_rcv_saddr_any(sk)
> is true? The udp_sk(sk)->udp_portaddr_hash is updated to newhash at
> the start of the function (before this block):
>
> void udp_lib_rehash(struct sock *sk, u16 newhash, u16 newhash4)
> {
>         struct udp_table *udptable = udp_get_table_prot(sk);
>         ...
>         udp_sk(sk)->udp_portaddr_hash = newhash;

Exactly, this needs to be moved later.

Will fix it in v2.

Thanks!


>         ...
> }
>
> When the new code calls udp_unhash4() on the inet_rcv_saddr_any(sk)
> path, udp_unhash4() computes its hslot2 from the already-updated
> udp_portaddr_hash (nhslot2):
>
> void udp_unhash4(struct udp_table *udptable, struct sock *sk)
> {
>         struct udp_hslot *hslot2 = udp_hashslot2(udptable, udp_sk(sk)->udp_portaddr_hash);
>         ...
>         udp_hash4_dec(hslot2);
> }
>
> But the original udp_lib_hash4() call during connect() would have
> incremented hash4_cnt on the old hslot2, not nhslot2. This seems to
> decrement hash4_cnt on the wrong 2-tuple slot when hslot2 != nhslot2,
> which is common for wildcard-bind + connect + disconnect.
>
> The else branch correctly uses udp_hash4_dec(hslot2) on the old slot
> and udp_hash4_inc(nhslot2) on the new slot.
>
> While the impact is minor since hash4_cnt is a u32 optimization hint,
> the counter becomes permanently incorrect and accumulates error across
> connect/disconnect cycles.