[PATCH bpf] bpf, sockmap: fix lock inversion between stab->lock and sk_callback_lock

[PATCH bpf] bpf, sockmap: fix lock inversion between stab->lock and sk_callback_lock

[Sechang Lim]

sock_map_update_common() and __sock_map_delete() hold stab->lock and call
sock_map_unref() -> sock_map_del_link() under it. sock_map_del_link() takes
sk_callback_lock for write to stop the strparser and verdict, giving the
lock order stab->lock -> sk_callback_lock.

The opposite order comes from an SK_SKB stream parser. On RX,
sk_psock_strp_data_ready() holds sk_callback_lock for read while running
the parser. The verdict redirects the skb to egress, where a sched_cls
program calls bpf_map_delete_elem() on a sockmap, which takes stab->lock:

  WARNING: possible circular locking dependency detected
  7.1.0-rc6 Not tainted
  ------------------------------------------------------
  syz.9.8824 is trying to acquire lock:
  (&stab->lock){+.-.}-{3:3}, at: __sock_map_delete net/core/sock_map.c:421
  but task is already holding lock:
  (clock-AF_INET){++.-}-{3:3}, at: sk_psock_strp_data_ready net/core/skmsg.c:1173

  -> #1 (clock-AF_INET){++.-}-{3:3}:
         _raw_write_lock_bh
         sock_map_del_link net/core/sock_map.c:167
         sock_map_unref net/core/sock_map.c:184
         sock_map_update_common net/core/sock_map.c:509
         sock_map_update_elem_sys net/core/sock_map.c:588
         map_update_elem kernel/bpf/syscall.c:1805

  -> #0 (&stab->lock){+.-.}-{3:3}:
         _raw_spin_lock_bh
         __sock_map_delete net/core/sock_map.c:421
         sock_map_delete_elem net/core/sock_map.c:452
         bpf_prog_06044d24140080b6
         tcx_run net/core/dev.c:4451
         sch_handle_egress net/core/dev.c:4541
         __dev_queue_xmit net/core/dev.c:4808
         ...
         tcp_bpf_strp_read_sock net/ipv4/tcp_bpf.c:701
         strp_data_ready net/strparser/strparser.c:402
         sk_psock_strp_data_ready net/core/skmsg.c:1174
         tcp_data_queue net/ipv4/tcp_input.c:5661

  Possible unsafe locking scenario:

         CPU0                    CPU1
         ----                    ----
    rlock(clock-AF_INET);
                                 lock(&stab->lock);
                                 lock(clock-AF_INET);
    lock(&stab->lock);

   *** DEADLOCK ***

sk_callback_lock is an rwlock and the established side takes it for write,
so the read side cannot re-enter once a writer is queued.

sock_map_del_link() uses psock->link_lock and sk_callback_lock, not
stab->lock. The socket is removed from the slot with xchg() under
stab->lock, which leaves a single deleter owning it, and its reference is
dropped only by sk_psock_put() in sock_map_unref(). Release stab->lock
right after the xchg() and run sock_map_unref() outside it. Do the same
for the replaced socket in sock_map_update_common(). sock_map_free()
already unrefs without stab->lock.

Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
---
 net/core/sock_map.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 99e3789492a0..390bd5ee46d4 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -421,13 +421,13 @@ static int __sock_map_delete(struct bpf_stab *stab, struct sock *sk_test,
 	spin_lock_bh(&stab->lock);
 	if (!sk_test || sk_test == *psk)
 		sk = xchg(psk, NULL);
+	spin_unlock_bh(&stab->lock);
 
 	if (likely(sk))
 		sock_map_unref(sk, psk);
 	else
 		err = -EINVAL;
 
-	spin_unlock_bh(&stab->lock);
 	return err;
 }
 
@@ -505,9 +505,10 @@ static int sock_map_update_common(struct bpf_map *map, u32 idx,
 
 	sock_map_add_link(psock, link, map, &stab->sks[idx]);
 	stab->sks[idx] = sk;
+	spin_unlock_bh(&stab->lock);
+
 	if (osk)
 		sock_map_unref(osk, &stab->sks[idx]);
-	spin_unlock_bh(&stab->lock);
 	return 0;
 out_unlock:
 	spin_unlock_bh(&stab->lock);
-- 
2.43.0

Re: [PATCH bpf] bpf, sockmap: fix lock inversion between stab->lock and sk_callback_lock

[Jiayuan Chen]

On 6/16/26 5:11 PM, Sechang Lim wrote:
> sock_map_update_common() and __sock_map_delete() hold stab->lock and call
> sock_map_unref() -> sock_map_del_link() under it. sock_map_del_link() takes
> sk_callback_lock for write to stop the strparser and verdict, giving the
> lock order stab->lock -> sk_callback_lock.
>
> The opposite order comes from an SK_SKB stream parser. On RX,
> sk_psock_strp_data_ready() holds sk_callback_lock for read while running
> the parser. The verdict redirects the skb to egress, where a sched_cls


The commit message is wrong. A verdict does not redirect to egress
synchronously — sk_psock_skb_redirect() only queues the skb and
schedule_delayed_work()s sk_psock_backlog, so egress runs in workqueue
context, not under sk_callback_lock.


> program calls bpf_map_delete_elem() on a sockmap, which takes stab->lock:
>
>    WARNING: possible circular locking dependency detected
>    7.1.0-rc6 Not tainted
>    ------------------------------------------------------
>    syz.9.8824 is trying to acquire lock:
>    (&stab->lock){+.-.}-{3:3}, at: __sock_map_delete net/core/sock_map.c:421
>    but task is already holding lock:
>    (clock-AF_INET){++.-}-{3:3}, at: sk_psock_strp_data_ready net/core/skmsg.c:1173
>
>    -> #1 (clock-AF_INET){++.-}-{3:3}:
>           _raw_write_lock_bh
>           sock_map_del_link net/core/sock_map.c:167
>           sock_map_unref net/core/sock_map.c:184
>           sock_map_update_common net/core/sock_map.c:509
>           sock_map_update_elem_sys net/core/sock_map.c:588
>           map_update_elem kernel/bpf/syscall.c:1805
>
>    -> #0 (&stab->lock){+.-.}-{3:3}:
>           _raw_spin_lock_bh
>           __sock_map_delete net/core/sock_map.c:421
>           sock_map_delete_elem net/core/sock_map.c:452
>           bpf_prog_06044d24140080b6
>           tcx_run net/core/dev.c:4451
>           sch_handle_egress net/core/dev.c:4541
>           __dev_queue_xmit net/core/dev.c:4808
>           ...
>           tcp_bpf_strp_read_sock net/ipv4/tcp_bpf.c:701


I guess it is an ACK. What is the actual purpose of a sched_cls program 
calling

sockmap delete on the TX path of an ACK? If there is no real use case 
for it, this is

just broken BPF usage, not a kernel bug worth this change.

Re: [PATCH bpf] bpf, sockmap: fix lock inversion between stab->lock and sk_callback_lock

[Sechang Lim]

On Tue, Jun 16, 2026 at 06:17:48PM +0800, Jiayuan Chen wrote:
>
>On 6/16/26 5:11 PM, Sechang Lim wrote:
>>sock_map_update_common() and __sock_map_delete() hold stab->lock and call
>>sock_map_unref() -> sock_map_del_link() under it. sock_map_del_link() takes
>>sk_callback_lock for write to stop the strparser and verdict, giving the
>>lock order stab->lock -> sk_callback_lock.
>>
>>The opposite order comes from an SK_SKB stream parser. On RX,
>>sk_psock_strp_data_ready() holds sk_callback_lock for read while running
>>the parser. The verdict redirects the skb to egress, where a sched_cls
>
>
>The commit message is wrong. A verdict does not redirect to egress
>synchronously — sk_psock_skb_redirect() only queues the skb and
>schedule_delayed_work()s sk_psock_backlog, so egress runs in workqueue
>context, not under sk_callback_lock.
>

Thanks, you're right. it's the inline ACK, not the redirect. Sorry for
the misleading changelog, I'll fix it in v2.

>
>>program calls bpf_map_delete_elem() on a sockmap, which takes stab->lock:
>>
>>   WARNING: possible circular locking dependency detected
>>   7.1.0-rc6 Not tainted
>>   ------------------------------------------------------
>>   syz.9.8824 is trying to acquire lock:
>>   (&stab->lock){+.-.}-{3:3}, at: __sock_map_delete net/core/sock_map.c:421
>>   but task is already holding lock:
>>   (clock-AF_INET){++.-}-{3:3}, at: sk_psock_strp_data_ready net/core/skmsg.c:1173
>>
>>   -> #1 (clock-AF_INET){++.-}-{3:3}:
>>          _raw_write_lock_bh
>>          sock_map_del_link net/core/sock_map.c:167
>>          sock_map_unref net/core/sock_map.c:184
>>          sock_map_update_common net/core/sock_map.c:509
>>          sock_map_update_elem_sys net/core/sock_map.c:588
>>          map_update_elem kernel/bpf/syscall.c:1805
>>
>>   -> #0 (&stab->lock){+.-.}-{3:3}:
>>          _raw_spin_lock_bh
>>          __sock_map_delete net/core/sock_map.c:421
>>          sock_map_delete_elem net/core/sock_map.c:452
>>          bpf_prog_06044d24140080b6
>>          tcx_run net/core/dev.c:4451
>>          sch_handle_egress net/core/dev.c:4541
>>          __dev_queue_xmit net/core/dev.c:4808
>>          ...
>>          tcp_bpf_strp_read_sock net/ipv4/tcp_bpf.c:701
>
>
>I guess it is an ACK. What is the actual purpose of a sched_cls 
>program calling
>
>sockmap delete on the TX path of an ACK? If there is no real use case 
>for it, this is
>
>just broken BPF usage, not a kernel bug worth this change.
>
>

I don't have a real use case for that exact program. But the verifier
allows sockmap delete from tc, and it deadlocks when the strparser's
socket is concurrently removed from the same map. The fix only moves
sock_map_unref() out from under stab->lock.

Best,
Sechang

Re: [PATCH bpf] bpf, sockmap: fix lock inversion between stab->lock and sk_callback_lock

[John Fastabend]

On Tue, Jun 16, 2026 at 06:40:09PM +0000, Sechang Lim wrote:
>On Tue, Jun 16, 2026 at 06:17:48PM +0800, Jiayuan Chen wrote:
>>
>>On 6/16/26 5:11 PM, Sechang Lim wrote:
>>>sock_map_update_common() and __sock_map_delete() hold stab->lock and call
>>>sock_map_unref() -> sock_map_del_link() under it. sock_map_del_link() takes
>>>sk_callback_lock for write to stop the strparser and verdict, giving the
>>>lock order stab->lock -> sk_callback_lock.
>>>
>>>The opposite order comes from an SK_SKB stream parser. On RX,
>>>sk_psock_strp_data_ready() holds sk_callback_lock for read while running
>>>the parser. The verdict redirects the skb to egress, where a sched_cls
>>
>>
>>The commit message is wrong. A verdict does not redirect to egress
>>synchronously — sk_psock_skb_redirect() only queues the skb and
>>schedule_delayed_work()s sk_psock_backlog, so egress runs in workqueue
>>context, not under sk_callback_lock.
>>
>
>Thanks, you're right. it's the inline ACK, not the redirect. Sorry for
>the misleading changelog, I'll fix it in v2.
>
>>
>>>program calls bpf_map_delete_elem() on a sockmap, which takes stab->lock:
>>>
>>>  WARNING: possible circular locking dependency detected
>>>  7.1.0-rc6 Not tainted
>>>  ------------------------------------------------------
>>>  syz.9.8824 is trying to acquire lock:
>>>  (&stab->lock){+.-.}-{3:3}, at: __sock_map_delete net/core/sock_map.c:421
>>>  but task is already holding lock:
>>>  (clock-AF_INET){++.-}-{3:3}, at: sk_psock_strp_data_ready net/core/skmsg.c:1173
>>>
>>>  -> #1 (clock-AF_INET){++.-}-{3:3}:
>>>         _raw_write_lock_bh
>>>         sock_map_del_link net/core/sock_map.c:167
>>>         sock_map_unref net/core/sock_map.c:184
>>>         sock_map_update_common net/core/sock_map.c:509
>>>         sock_map_update_elem_sys net/core/sock_map.c:588
>>>         map_update_elem kernel/bpf/syscall.c:1805
>>>
>>>  -> #0 (&stab->lock){+.-.}-{3:3}:
>>>         _raw_spin_lock_bh
>>>         __sock_map_delete net/core/sock_map.c:421
>>>         sock_map_delete_elem net/core/sock_map.c:452
>>>         bpf_prog_06044d24140080b6
>>>         tcx_run net/core/dev.c:4451
>>>         sch_handle_egress net/core/dev.c:4541
>>>         __dev_queue_xmit net/core/dev.c:4808
>>>         ...
>>>         tcp_bpf_strp_read_sock net/ipv4/tcp_bpf.c:701
>>
>>
>>I guess it is an ACK. What is the actual purpose of a sched_cls 
>>program calling
>>
>>sockmap delete on the TX path of an ACK? If there is no real use 
>>case for it, this is
>>
>>just broken BPF usage, not a kernel bug worth this change.
>>
>>
>
>I don't have a real use case for that exact program. But the verifier
>allows sockmap delete from tc, and it deadlocks when the strparser's
>socket is concurrently removed from the same map. The fix only moves
>sock_map_unref() out from under stab->lock.
>
>Best,
>Sechang

The bot also thinks it found another locking issue. I'm not sure
supporting 'tc' is really needed here. sockmap is much more easy
to reason about from socket layer. What about just blocking sockmap
manipulations from these prog types.

My current thinking on sockmap at the moment is its has sprawled
across so many layers the locking is overly tricky to reason about.

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d9bdc3b32c05..5e08d3e03453 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8567,11 +8567,7 @@ static bool may_update_sockmap(struct bpf_verifier_env *env, int func_id)
                         return true;
                 break;
         case BPF_PROG_TYPE_SOCKET_FILTER:
-       case BPF_PROG_TYPE_SCHED_CLS:
-       case BPF_PROG_TYPE_SCHED_ACT:
-       case BPF_PROG_TYPE_XDP:
         case BPF_PROG_TYPE_SK_REUSEPORT:
-       case BPF_PROG_TYPE_FLOW_DISSECTOR:
         case BPF_PROG_TYPE_SK_LOOKUP:
                 return true;
	default: