[PATCH bpf v2] bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector

[PATCH bpf v2] bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector

[Sechang Lim]

sock_map_update_common() and __sock_map_delete() hold stab->lock and call
sock_map_unref() -> sock_map_del_link(), which takes sk_callback_lock for
write. That gives the order stab->lock -> sk_callback_lock.

The reverse order comes from the SK_SKB stream parser.
sk_psock_strp_data_ready() holds sk_callback_lock for read, and after the
verdict tcp_bpf_strp_read_sock() acks the consumed data inline via
__tcp_cleanup_rbuf(). The ACK goes out egress, where a sched_cls program
deletes from the sockmap and 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 ***

A tc, xdp or flow_dissector program has no reason to update or delete a
sockmap, and redirect does not go through here. Drop them from
may_update_sockmap() so the verifier rejects it. It also closes the
matching sockhash inversion.

Fixes: 0126240f448d ("bpf: sockmap: Allow update from BPF")
Suggested-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
---
v2:
 - reject sockmap update/delete from tc, xdp and flow_dissector (John
   Fastabend)
 - fix the changelog (Jiayuan Chen)

v1:
 - https://lore.kernel.org/all/20260616091153.2966617-1-rhkrqnwk98@gmail.com/

 kernel/bpf/verifier.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7fb88e1cd7c4..94d225521b5a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8766,11 +8766,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:
-- 
2.43.0

Re: [PATCH bpf v2] bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The commit silently breaks multiple existing BPF selftests by rejecting sockmap updates from TC, XDP, and flow_dissector programs without updating the tests, indicating an incomplete patch and a breaking UAPI change.

Pre-existing issues:
- [High] The patch incompletely fixes a circular locking dependency between `sk_callback_lock` and `stab->lock`. While it removes `BPF_PROG_TYPE_SCHED_CLS`, `BPF_PROG_TYPE_SCHED_ACT`, `BPF_PROG_TYPE_XDP`, and `BPF_PROG_TYPE_FLOW_DISSECTOR` from the allowed program types in `may_update_sockmap()`, it fails to remove `BPF_PROG_TYPE_SOCK_OPS` and `BPF_PROG_TYPE_SOCKET_FILTER`. Both of these can still be executed synchronously on the exact same egress path, triggering the identical deadlock.
--

commit 44025f27fc1d4e7065cdd08d5667a66505bcb5cf
Author: Sechang Lim <rhkrqnwk98@gmail.com>

bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector

This patch removes the ability to update or delete sockmaps from TC, XDP, and
flow_dissector BPF programs. It aims to fix a circular locking dependency
between the sk_callback_lock and stab->lock that occurs during the egress
path.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2abc79dbf281c..612b264439c6c 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -8452,11 +8452,7 @@ static bool may_update_sockmap(struct bpf_verifier_env *env, int func_id)
>  			return true;
>  		break;
>  	case BPF_PROG_TYPE_SOCKET_FILTER:

[Severity: High]
This is a pre-existing issue, but does leaving BPF_PROG_TYPE_SOCKET_FILTER
(and BPF_PROG_TYPE_SOCK_OPS just above this) allowed here still leave the door
open for the same deadlock?

Both of these can execute synchronously on the exact same egress path you
mentioned in the commit message:

sk_psock_strp_data_ready()
  tcp_bpf_strp_read_sock()
    __tcp_cleanup_rbuf()
      tcp_send_ack()
        __tcp_send_ack()
          __tcp_transmit_skb()

Inside __tcp_transmit_skb(), bpf_skops_write_hdr_opt() can execute a
BPF_PROG_TYPE_SOCK_OPS program. Alternatively, dev_queue_xmit() can call
dev_queue_xmit_nit() if an AF_PACKET socket is attached, executing a
BPF_PROG_TYPE_SOCKET_FILTER program.

If either program updates or deletes a sockmap element, wouldn't it acquire
stab->lock and recreate the sk_callback_lock -> stab->lock dependency?

> -	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;

[Severity: High]
Since this drops support for XDP, TC, and flow_dissector, are there updates
needed for the selftests in tools/testing/selftests/bpf/progs/verifier_sockmap_mutate.c?

That test file explicitly expects success for xdp, flow_dissector,
classifier, and action programs performing sockmap mutations:

SEC("xdp")
__success
int test_xdp(struct xdp_md *ctx __always_unused)

Will this change cause the verifier to reject those programs and fail the
test suite?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620034632.2308-1-rhkrqnwk98@gmail.com?part=1

Re: [PATCH bpf v2] bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector

[Jiayuan Chen]

On 6/20/26 11:46 AM, Sechang Lim wrote:
> sock_map_update_common() and __sock_map_delete() hold stab->lock and call
> sock_map_unref() -> sock_map_del_link(), which takes sk_callback_lock for
> write. That gives the order stab->lock -> sk_callback_lock.
>
> The reverse order comes from the SK_SKB stream parser.
> sk_psock_strp_data_ready() holds sk_callback_lock for read, and after the
> verdict tcp_bpf_strp_read_sock() acks the consumed data inline via
> __tcp_cleanup_rbuf(). The ACK goes out egress, where a sched_cls program
> deletes from the sockmap and 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 ***
>
> A tc, xdp or flow_dissector program has no reason to update or delete a
> sockmap, and redirect does not go through here. Drop them from
> may_update_sockmap() so the verifier rejects it. It also closes the
> matching sockhash inversion.
>
> Fixes: 0126240f448d ("bpf: sockmap: Allow update from BPF")
> Suggested-by: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
> ---
> v2:
>   - reject sockmap update/delete from tc, xdp and flow_dissector (John
>     Fastabend)
>   - fix the changelog (Jiayuan Chen)
>
> v1:
>   - https://lore.kernel.org/all/20260616091153.2966617-1-rhkrqnwk98@gmail.com/
>
>   kernel/bpf/verifier.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 7fb88e1cd7c4..94d225521b5a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -8766,11 +8766,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:

CI failed.

https://github.com/kernel-patches/bpf/actions/runs/27859622337/job/82454035306

Please drop or change such trigger.


Also, please drop Fixes tag and target to bpf-next for the same reason 
in you another thread.


Nit:

You can also manually fork and create pull request against

https://github.com/kernel-patches/bpf/ to run full test to run the full

test and make sure all tests pass before you send patch.