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

[John Fastabend <john.fastabend@gmail.com>]

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: