Re: [PATCH net-next v3 2/4] udp: Remove disconnected sockets from the 4-tuple hash

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Calling `udp_unhash4()` from process context with softirqs enabled creates a deadlock vulnerability and lockdep warnings.
- [High] Concurrent executions of `udp_unhash4()` lead to double-decrements of hash bucket counters due to a TOCTOU race.
--

commit e6257404c34b723d1c990d6c54298b3c3edfb986
Author: Jordan Rife <jrife@google.com>

udp: Remove disconnected sockets from the 4-tuple hash

This commit updates udp_disconnect() and udp_abort() to always remove a
disconnected socket from the 4-tuple hash. This is done by calling the newly
introduced udp_disconnect_unhash4() function on disconnect or abort.

> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 91918ff60728f..e3f7689513d9b 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -2235,10 +2235,20 @@ int __udp_disconnect(struct sock *sk, int flags)
>  }
>  EXPORT_SYMBOL(__udp_disconnect);
>  
> +static int udp_disconnect_unhash4(struct sock *sk, int flags)
> +{
> +	struct udp_table *udptable = udp_get_table_prot(sk);
> +
> +	udp_unhash4(udptable, sk);

Is it safe to call udp_unhash4() here with softirqs enabled?

Looking at udp_unhash4(), it acquires hslot4->lock and hslot2->lock using
spin_lock(). Because lock_sock() in udp_disconnect() doesn't disable bottom
halves, an interrupt could occur while hslot4->lock is held.

If a softirq executes a BPF program that calls bpf_sock_destroy() on a
socket in the same bucket, it triggers udp_abort(). As udp_abort() skips
lock_sock(sk) when in BPF context, it will call udp_unhash4() and attempt
to acquire hslot4->lock again, resulting in a deadlock.

Also, could this lead to double-decrements of hash bucket counters?

Inside udp_unhash4(), the if (udp_hashed4(sk)) check is evaluated outside
of any spinlocks. If another path unhashes the socket concurrently (such as
udp_abort() from a BPF context bypassing lock_sock(sk)), both threads can
evaluate udp_hashed4(sk) as true.

Both threads will then serialize on spin_lock(&hslot4->lock). While
hlist_nulls_del_init_rcu() handles already-unhashed nodes, both threads
will unconditionally execute hslot4->count-- and udp_hash4_dec(hslot2),
leading to corrupted hash table counts.

> +	__udp_disconnect(sk, flags);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260330215707.2374657-1-jrife@google.com?part=2