Re: [PATCH v2 bpf] sockmap: Fix sk_psock_drop() race vs sock_map_{unhash,close,destroy}().

On Thu, Apr 23, 2026 at 2:17 PM Martin KaFai Lau <[email protected]> wrote:
>
> On Tue, Apr 21, 2026 at 11:21:27AM +0800, Jiayuan Chen wrote:
> > > > @@ -1652,20 +1657,25 @@ void sock_map_destroy(struct sock *sk)
> > > >           void (*saved_destroy)(struct sock *sk);
> > > >           struct sk_psock *psock;
> > > > +retry:
> > > >           rcu_read_lock();
> > > >           psock = sk_psock_get(sk);
> > > >           if (unlikely(!psock)) {
> > > >                   rcu_read_unlock();
> > > >                   saved_destroy = READ_ONCE(sk->sk_prot)->destroy;
> > > > +         if (unlikely(saved_destroy == sock_map_destroy))
> > > > +                 goto retry;
> > > Can this unbounded retry loop cause a hard lockup due to softirq preemption
> > > or priority inversion?
> > >
> > > If sk_psock_put() is called from process context with bottom-halves enabled
> > > (such as from sock_map_close() after release_sock()), an interrupt could
> > > preempt the thread immediately after refcount_dec_and_test(&psock->refcnt)
> > > drops to 0 but before sk_psock_drop() disables BH.
> > >
> > > If a network interrupt fires on the same CPU, NET_RX_SOFTIRQ may process a
> > > packet (such as an RST) that triggers socket destruction via:
> > > tcp_done() -> inet_csk_destroy_sock() -> sk->sk_prot->destroy()
> > >
> > > Since sk_psock_drop() has not yet restored the protocol, sk->sk_prot->destroy
> > > is still sock_map_destroy().
> > >
> > > When sock_map_destroy() calls sk_psock_get(), it returns NULL because the
> > > refcount is exactly 0. The code then falls into the !psock branch, sees that
> > > sk->sk_prot->destroy is still sock_map_destroy(), and jumps to retry.
> > >
> > > Because the softirq spins infinitely in this tight loop and never yields the
> > > CPU, the preempted process context can never execute sk_psock_drop(),
> > > resulting in a permanent hard lockup.
> >
> >
> > sock_map_close(sk)
> > |___ sk_psock_put(sk, psock)   <- refcnt-hits-0 window lives here
> > |___ saved_close == tcp_close
> >       |__tcp_close
> >             |____ sock_orphan   <- SOCK_DEAD set here
> >             |____(later) inet_csk_destroy_sock
> >
> > At the exact instant the refcnt can be observed at 0 with
> > sk_prot not yet restored, SOCK_DEAD is guaranteed not to be set.
>
>
> > > A similar priority inversion deadlock could also occur on PREEMPT_RT if the
> > > thread calling sk_psock_drop() is preempted by a higher-priority task.
>
> The same SOCK_DEAD reasoning applies to PREEMPT_RT?
>
> It is useful to have some explanation in the commit message for this case.
>
> Kuniyuki, does the above make sense? I can fold it in before landing.

Yes, that applies to PREEMPT_RT too and the AI's scenario
does not occur.

Thanks !