* [PATCH bpf-next v3 0/2] bpf, sockmap: disallow sockmap mutation from tc, xdp and flow_dissector
@ 2026-06-29 17:26 Sechang Lim
2026-06-29 17:27 ` [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete " Sechang Lim
2026-06-29 17:27 ` [PATCH bpf-next v3 2/2] selftests/bpf: drop tc/xdp/flow_dissector sockmap mutation tests Sechang Lim
0 siblings, 2 replies; 9+ messages in thread
From: Sechang Lim @ 2026-06-29 17:26 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
John Fastabend, Eduard Zingerman, Kumar Kartikeya Dwivedi,
David S . Miller, Jakub Kicinski, Jesper Dangaard Brouer
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
Stanislav Fomichev, Emil Tsalapatis, Lorenz Bauer, Jakub Sitnicki,
Jiayuan Chen, Shuah Khan, bpf, netdev, linux-kselftest,
linux-kernel
A tc, xdp or flow_dissector program updating or deleting a sockmap
deadlocks on stab->lock vs sk_callback_lock and has no reason to. Patch 1
disallows it in may_update_sockmap(); patch 2 drops the selftests that
exercised it.
v3:
- drop the broken selftests (Jiayuan Chen)
- drop the Fixes tag and target bpf-next (Jiayuan Chen)
v2:
- https://lore.kernel.org/all/20260620034632.2308-1-rhkrqnwk98@gmail.com/
v1:
- https://lore.kernel.org/all/20260616091153.2966617-1-rhkrqnwk98@gmail.com/
Sechang Lim (2):
bpf, sockmap: disallow update and delete from tc, xdp and
flow_dissector
selftests/bpf: drop tc/xdp/flow_dissector sockmap mutation tests
kernel/bpf/verifier.c | 4 --
.../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 13 -----
.../selftests/bpf/prog_tests/sockmap_basic.c | 52 -------------------
.../bpf/progs/freplace_cls_redirect.c | 34 ------------
.../selftests/bpf/progs/test_sockmap_update.c | 48 -----------------
.../bpf/progs/verifier_sockmap_mutate.c | 10 ++--
6 files changed, 5 insertions(+), 156 deletions(-)
delete mode 100644 tools/testing/selftests/bpf/progs/freplace_cls_redirect.c
delete mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_update.c
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector
2026-06-29 17:26 [PATCH bpf-next v3 0/2] bpf, sockmap: disallow sockmap mutation from tc, xdp and flow_dissector Sechang Lim
@ 2026-06-29 17:27 ` Sechang Lim
2026-06-29 17:42 ` sashiko-bot
` (2 more replies)
2026-06-29 17:27 ` [PATCH bpf-next v3 2/2] selftests/bpf: drop tc/xdp/flow_dissector sockmap mutation tests Sechang Lim
1 sibling, 3 replies; 9+ messages in thread
From: Sechang Lim @ 2026-06-29 17:27 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
John Fastabend, Eduard Zingerman, Kumar Kartikeya Dwivedi,
David S . Miller, Jakub Kicinski, Jesper Dangaard Brouer
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
Stanislav Fomichev, Emil Tsalapatis, Lorenz Bauer, Jakub Sitnicki,
Jiayuan Chen, Shuah Khan, bpf, netdev, linux-kselftest,
linux-kernel
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
------------------------------------------------------
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.
Suggested-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Sechang Lim <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 25aea4271cd0..58d766c34626 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8489,11 +8489,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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next v3 2/2] selftests/bpf: drop tc/xdp/flow_dissector sockmap mutation tests
2026-06-29 17:26 [PATCH bpf-next v3 0/2] bpf, sockmap: disallow sockmap mutation from tc, xdp and flow_dissector Sechang Lim
2026-06-29 17:27 ` [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete " Sechang Lim
@ 2026-06-29 17:27 ` Sechang Lim
2026-06-29 17:48 ` sashiko-bot
1 sibling, 1 reply; 9+ messages in thread
From: Sechang Lim @ 2026-06-29 17:27 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
John Fastabend, Eduard Zingerman, Kumar Kartikeya Dwivedi,
David S . Miller, Jakub Kicinski, Jesper Dangaard Brouer
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
Stanislav Fomichev, Emil Tsalapatis, Lorenz Bauer, Jakub Sitnicki,
Jiayuan Chen, Shuah Khan, bpf, netdev, linux-kselftest,
linux-kernel
tc, xdp and flow_dissector programs can no longer update or delete a
sockmap. Adjust the tests:
- verifier_sockmap_mutate: the tc, xdp and flow_dissector cases now
expect __failure with "cannot update sockmap in this context".
- sockmap_basic: drop "sockmap update" / "sockhash update", which load
a SEC("tc") program that copies a sock between maps.
- fexit_bpf2bpf: drop "func_sockmap_update", whose freplace program
updates a sockmap in the tc cls_redirect context.
Remove the now-unused test_sockmap_update.c and freplace_cls_redirect.c.
Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
---
.../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 13 -----
.../selftests/bpf/prog_tests/sockmap_basic.c | 52 -------------------
.../bpf/progs/freplace_cls_redirect.c | 34 ------------
.../selftests/bpf/progs/test_sockmap_update.c | 48 -----------------
.../bpf/progs/verifier_sockmap_mutate.c | 10 ++--
5 files changed, 5 insertions(+), 152 deletions(-)
delete mode 100644 tools/testing/selftests/bpf/progs/freplace_cls_redirect.c
delete mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_update.c
diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
index 92c20803ea76..d3a954158c33 100644
--- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
@@ -336,17 +336,6 @@ static void test_fmod_ret_freplace(void)
}
-static void test_func_sockmap_update(void)
-{
- const char *prog_name[] = {
- "freplace/cls_redirect",
- };
- test_fexit_bpf2bpf_common("./freplace_cls_redirect.bpf.o",
- "./test_cls_redirect.bpf.o",
- ARRAY_SIZE(prog_name),
- prog_name, false, NULL);
-}
-
static void test_func_replace_void(void)
{
const char *prog_name[] = {
@@ -599,8 +588,6 @@ void serial_test_fexit_bpf2bpf(void)
test_func_replace();
if (test__start_subtest("func_replace_verify"))
test_func_replace_verify();
- if (test__start_subtest("func_sockmap_update"))
- test_func_sockmap_update();
if (test__start_subtest("func_replace_return_code"))
test_func_replace_return_code();
if (test__start_subtest("func_map_prog_compatibility"))
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index cb3229711f93..33f788e2786d 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -7,7 +7,6 @@
#include "test_progs.h"
#include "test_skmsg_load_helpers.skel.h"
-#include "test_sockmap_update.skel.h"
#include "test_sockmap_invalid_update.skel.h"
#include "test_sockmap_skb_verdict_attach.skel.h"
#include "test_sockmap_progs_query.skel.h"
@@ -235,53 +234,6 @@ static void test_skmsg_helpers_with_link(enum bpf_map_type map_type)
test_skmsg_load_helpers__destroy(skel);
}
-static void test_sockmap_update(enum bpf_map_type map_type)
-{
- int err, prog, src;
- struct test_sockmap_update *skel;
- struct bpf_map *dst_map;
- const __u32 zero = 0;
- char dummy[14] = {0};
- LIBBPF_OPTS(bpf_test_run_opts, topts,
- .data_in = dummy,
- .data_size_in = sizeof(dummy),
- .repeat = 1,
- );
- __s64 sk;
-
- sk = connected_socket_v4();
- if (!ASSERT_NEQ(sk, -1, "connected_socket_v4"))
- return;
-
- skel = test_sockmap_update__open_and_load();
- if (!ASSERT_OK_PTR(skel, "open_and_load"))
- goto close_sk;
-
- prog = bpf_program__fd(skel->progs.copy_sock_map);
- src = bpf_map__fd(skel->maps.src);
- if (map_type == BPF_MAP_TYPE_SOCKMAP)
- dst_map = skel->maps.dst_sock_map;
- else
- dst_map = skel->maps.dst_sock_hash;
-
- err = bpf_map_update_elem(src, &zero, &sk, BPF_NOEXIST);
- if (!ASSERT_OK(err, "update_elem(src)"))
- goto out;
-
- err = bpf_prog_test_run_opts(prog, &topts);
- if (!ASSERT_OK(err, "test_run"))
- goto out;
- if (!ASSERT_NEQ(topts.retval, 0, "test_run retval"))
- goto out;
-
- compare_cookies(skel->maps.src, dst_map);
-
-out:
- test_sockmap_update__destroy(skel);
-close_sk:
- close(sk);
-}
-
static void test_sockmap_invalid_update(void)
{
struct test_sockmap_invalid_update *skel;
@@ -1385,10 +1337,6 @@ void test_sockmap_basic(void)
test_skmsg_helpers(BPF_MAP_TYPE_SOCKMAP);
if (test__start_subtest("sockhash sk_msg load helpers"))
test_skmsg_helpers(BPF_MAP_TYPE_SOCKHASH);
- if (test__start_subtest("sockmap update"))
- test_sockmap_update(BPF_MAP_TYPE_SOCKMAP);
- if (test__start_subtest("sockhash update"))
- test_sockmap_update(BPF_MAP_TYPE_SOCKHASH);
if (test__start_subtest("sockmap update in unsafe context"))
test_sockmap_invalid_update();
if (test__start_subtest("sockmap copy"))
diff --git a/tools/testing/selftests/bpf/progs/freplace_cls_redirect.c b/tools/testing/selftests/bpf/progs/freplace_cls_redirect.c
deleted file mode 100644
index 7e94412d47a5..000000000000
--- a/tools/testing/selftests/bpf/progs/freplace_cls_redirect.c
+++ /dev/null
@@ -1,34 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-// Copyright (c) 2020 Facebook
-
-#include <linux/stddef.h>
-#include <linux/bpf.h>
-#include <linux/pkt_cls.h>
-#include <bpf/bpf_endian.h>
-#include <bpf/bpf_helpers.h>
-
-struct {
- __uint(type, BPF_MAP_TYPE_SOCKMAP);
- __type(key, int);
- __type(value, int);
- __uint(max_entries, 2);
-} sock_map SEC(".maps");
-
-SEC("freplace/cls_redirect")
-int freplace_cls_redirect_test(struct __sk_buff *skb)
-{
- int ret = 0;
- const int zero = 0;
- struct bpf_sock *sk;
-
- sk = bpf_map_lookup_elem(&sock_map, &zero);
- if (!sk)
- return TC_ACT_SHOT;
-
- ret = bpf_map_update_elem(&sock_map, &zero, sk, 0);
- bpf_sk_release(sk);
-
- return ret == 0 ? TC_ACT_OK : TC_ACT_SHOT;
-}
-
-char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_update.c b/tools/testing/selftests/bpf/progs/test_sockmap_update.c
deleted file mode 100644
index 6d64ea536e3d..000000000000
--- a/tools/testing/selftests/bpf/progs/test_sockmap_update.c
+++ /dev/null
@@ -1,48 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-// Copyright (c) 2020 Cloudflare
-#include "vmlinux.h"
-#include <bpf/bpf_helpers.h>
-
-struct {
- __uint(type, BPF_MAP_TYPE_SOCKMAP);
- __uint(max_entries, 1);
- __type(key, __u32);
- __type(value, __u64);
-} src SEC(".maps");
-
-struct {
- __uint(type, BPF_MAP_TYPE_SOCKMAP);
- __uint(max_entries, 1);
- __type(key, __u32);
- __type(value, __u64);
-} dst_sock_map SEC(".maps");
-
-struct {
- __uint(type, BPF_MAP_TYPE_SOCKHASH);
- __uint(max_entries, 1);
- __type(key, __u32);
- __type(value, __u64);
-} dst_sock_hash SEC(".maps");
-
-SEC("tc")
-int copy_sock_map(void *ctx)
-{
- struct bpf_sock *sk;
- bool failed = false;
- __u32 key = 0;
-
- sk = bpf_map_lookup_elem(&src, &key);
- if (!sk)
- return SK_DROP;
-
- if (bpf_map_update_elem(&dst_sock_map, &key, sk, 0))
- failed = true;
-
- if (bpf_map_update_elem(&dst_sock_hash, &key, sk, 0))
- failed = true;
-
- bpf_sk_release(sk);
- return failed ? SK_DROP : SK_PASS;
-}
-
-char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/verifier_sockmap_mutate.c b/tools/testing/selftests/bpf/progs/verifier_sockmap_mutate.c
index fe4b123187b8..b11026123690 100644
--- a/tools/testing/selftests/bpf/progs/verifier_sockmap_mutate.c
+++ b/tools/testing/selftests/bpf/progs/verifier_sockmap_mutate.c
@@ -74,7 +74,7 @@ static __always_inline void test_sockmap_lookup_and_mutate(void)
}
SEC("action")
-__success
+__failure __msg("cannot update sockmap in this context")
int test_sched_act(struct __sk_buff *skb)
{
test_sockmap_mutate(skb->sk);
@@ -82,7 +82,7 @@ int test_sched_act(struct __sk_buff *skb)
}
SEC("classifier")
-__success
+__failure __msg("cannot update sockmap in this context")
int test_sched_cls(struct __sk_buff *skb)
{
test_sockmap_mutate(skb->sk);
@@ -90,7 +90,7 @@ int test_sched_cls(struct __sk_buff *skb)
}
SEC("flow_dissector")
-__success
+__failure __msg("cannot update sockmap in this context")
int test_flow_dissector_delete(struct __sk_buff *skb __always_unused)
{
test_sockmap_delete();
@@ -98,7 +98,7 @@ int test_flow_dissector_delete(struct __sk_buff *skb __always_unused)
}
SEC("flow_dissector")
-__failure __msg("program of this type cannot use helper bpf_sk_release")
+__failure __msg("cannot update sockmap in this context")
int test_flow_dissector_update(struct __sk_buff *skb __always_unused)
{
test_sockmap_lookup_and_update(); /* no access to skb->sk */
@@ -179,7 +179,7 @@ int test_sockops_update_dedicated(struct bpf_sock_ops *ctx)
}
SEC("xdp")
-__success
+__failure __msg("cannot update sockmap in this context")
int test_xdp(struct xdp_md *ctx __always_unused)
{
test_sockmap_lookup_and_mutate();
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector
2026-06-29 17:27 ` [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete " Sechang Lim
@ 2026-06-29 17:42 ` sashiko-bot
2026-06-29 18:06 ` bot+bpf-ci
2026-06-29 18:24 ` Alexei Starovoitov
2 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-06-29 17:42 UTC (permalink / raw)
To: Sechang Lim; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Incomplete Deadlock Fix / Bypass Vector
--
commit 11c2a6f499580e1830b9a3c86513eb4f154d7710
Author: Sechang Lim <rhkrqnwk98@gmail.com>
bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector
This commit aims to fix a circular locking dependency between
sk_callback_lock and stab->lock by disallowing sockmap updates and deletes
from tc, xdp, and flow_dissector programs.
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -8489,11 +8489,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;
[Severity: High]
This is a pre-existing issue, but does leaving BPF_PROG_TYPE_SOCKET_FILTER
and BPF_PROG_TYPE_SOCK_OPS (located just above this block) allowed to
update/delete sockmaps leave the deadlock unresolved?
The commit message describes a deadlock where sk_callback_lock is held for
read, a TCP ACK goes out egress, and a BPF program modifies the sockmap,
acquiring stab->lock.
Can BPF_PROG_TYPE_SOCK_OPS execute synchronously during this same egress
path and trigger the same issue?
sk_psock_strp_data_ready() // holds sk_callback_lock
__tcp_cleanup_rbuf()
tcp_send_ack()
__tcp_transmit_skb()
bpf_skops_write_hdr_opt() // runs BPF_PROG_TYPE_SOCK_OPS
Similarly, can BPF_PROG_TYPE_SOCKET_FILTER be executed synchronously during
egress when dev_queue_xmit_nit() calls run_filter()?
If either of these programs calls a sockmap map update or delete helper,
wouldn't they acquire stab->lock and recreate the exact same circular
locking dependency?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260629172704.1302218-1-rhkrqnwk98@gmail.com?part=1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v3 2/2] selftests/bpf: drop tc/xdp/flow_dissector sockmap mutation tests
2026-06-29 17:27 ` [PATCH bpf-next v3 2/2] selftests/bpf: drop tc/xdp/flow_dissector sockmap mutation tests Sechang Lim
@ 2026-06-29 17:48 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-06-29 17:48 UTC (permalink / raw)
To: Sechang Lim; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The commit deletes the selftest file test_sockmap_update.c but leaves broken references to it in the sockmap documentation.
--
commit ccad6ec599cc55c0fbdeda989132b9073df009d1
Author: Sechang Lim <rhkrqnwk98@gmail.com>
selftests/bpf: drop tc/xdp/flow_dissector sockmap mutation tests
tc, xdp and flow_dissector programs can no longer update or delete a
sockmap. Adjust the tests and remove the now-unused test_sockmap_update.c
and freplace_cls_redirect.c files.
> diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_update.c b/tools/testing/selftests/bpf/progs/test_sockmap_update.c
> deleted file mode 100644
> index 6d64ea536e3d8..0000000000000
> --- a/tools/testing/selftests/bpf/progs/test_sockmap_update.c
> +++ /dev/null
[Severity: Low]
Does this commit also need to update Documentation/bpf/map_sockmap.rst?
Since test_sockmap_update.c is being removed here, the literal references
and hyperlinks to it in Documentation/bpf/map_sockmap.rst (around lines
352 and 498) are left as broken links.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260629172704.1302218-1-rhkrqnwk98@gmail.com?part=2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector
2026-06-29 17:27 ` [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete " Sechang Lim
2026-06-29 17:42 ` sashiko-bot
@ 2026-06-29 18:06 ` bot+bpf-ci
2026-06-29 18:24 ` Alexei Starovoitov
2 siblings, 0 replies; 9+ messages in thread
From: bot+bpf-ci @ 2026-06-29 18:06 UTC (permalink / raw)
To: rhkrqnwk98, ast, daniel, andrii, john.fastabend, eddyz87, memxor,
davem, kuba, hawk
Cc: martin.lau, song, yonghong.song, jolsa, sdf, emil, lmb, jakub,
jiayuan.chen, shuah, bpf, netdev, linux-kselftest, linux-kernel,
ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]
> 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:
>
> 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.
>
> Suggested-by: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
This fixes the behaviour added by commit 0126240f448d ("bpf: sockmap: Allow
update from BPF"), which introduced may_update_sockmap() and added the four
BPF_PROG_TYPE_* cases that this change removes. Should it carry:
Fixes: 0126240f448d ("bpf: sockmap: Allow update from BPF")
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/28391303635
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector
2026-06-29 17:27 ` [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete " Sechang Lim
2026-06-29 17:42 ` sashiko-bot
2026-06-29 18:06 ` bot+bpf-ci
@ 2026-06-29 18:24 ` Alexei Starovoitov
2026-06-30 5:47 ` John Fastabend
2 siblings, 1 reply; 9+ messages in thread
From: Alexei Starovoitov @ 2026-06-29 18:24 UTC (permalink / raw)
To: Sechang Lim, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
John Fastabend, Eduard Zingerman, Kumar Kartikeya Dwivedi,
David S . Miller, Jakub Kicinski, Jesper Dangaard Brouer
Cc: Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
Stanislav Fomichev, Emil Tsalapatis, Lorenz Bauer, Jakub Sitnicki,
Jiayuan Chen, Shuah Khan, bpf, netdev, linux-kselftest,
linux-kernel
On Mon Jun 29, 2026 at 10:27 AM PDT, 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
> ------------------------------------------------------
> 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.
>
> Suggested-by: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
John,
please ack.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector
2026-06-29 18:24 ` Alexei Starovoitov
@ 2026-06-30 5:47 ` John Fastabend
2026-06-30 7:14 ` Sechang Lim
0 siblings, 1 reply; 9+ messages in thread
From: John Fastabend @ 2026-06-30 5:47 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Sechang Lim, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, David S . Miller,
Jakub Kicinski, Jesper Dangaard Brouer, Martin KaFai Lau,
Song Liu, Yonghong Song, Jiri Olsa, Stanislav Fomichev,
Emil Tsalapatis, Lorenz Bauer, Jakub Sitnicki, Jiayuan Chen,
Shuah Khan, bpf, netdev, linux-kselftest, linux-kernel
On Mon, Jun 29, 2026 at 11:24:49AM -0700, Alexei Starovoitov wrote:
>On Mon Jun 29, 2026 at 10:27 AM PDT, 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
>> ------------------------------------------------------
>> 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.
>>
>> Suggested-by: John Fastabend <john.fastabend@gmail.com>
>> Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
>
>John,
>
>please ack.
Hi Sechang,
I think we additionally need to also block BPF_PROG_TYPE_SOCKET_FILTER?
Did you check this case I guess the same case is possible there?
Then another patch needs to restrict BPF_SOCK_OPS users. For that
we need to block BPF_SOCK_OPS_HDR_OPT_LEN_CB and BPF_SOCK_OPS_WRITE_*.
Let me know if you want to do those as well. Let me know if you want
to do both patches or just the prog blocking above with the possible
addition of SOCKET_FILTER. I didn't search very hard so probably need
to check all the BPF_SOCK_OPS_* to find the valid cases.
Thanks,
John
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector
2026-06-30 5:47 ` John Fastabend
@ 2026-06-30 7:14 ` Sechang Lim
0 siblings, 0 replies; 9+ messages in thread
From: Sechang Lim @ 2026-06-30 7:14 UTC (permalink / raw)
To: John Fastabend
Cc: Alexei Starovoitov, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
David S . Miller, Jakub Kicinski, Jesper Dangaard Brouer,
Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
Stanislav Fomichev, Emil Tsalapatis, Lorenz Bauer, Jakub Sitnicki,
Jiayuan Chen, Shuah Khan, bpf, netdev, linux-kselftest,
linux-kernel
On Mon, Jun 29, 2026 at 10:47:41PM -0700, John Fastabend wrote:
>I think we additionally need to also block
>BPF_PROG_TYPE_SOCKET_FILTER? Did you check this case I guess the same
>case is possible there?
>
Yes, same inversion. I'll block BPF_PROG_TYPE_SOCKET_FILTER in v4.
>Then another patch needs to restrict BPF_SOCK_OPS users. For that
>we need to block BPF_SOCK_OPS_HDR_OPT_LEN_CB and BPF_SOCK_OPS_WRITE_*.
>Let me know if you want to do those as well. Let me know if you want
>to do both patches or just the prog blocking above with the possible
>addition of SOCKET_FILTER. I didn't search very hard so probably need
>to check all the BPF_SOCK_OPS_* to find the valid cases.
>
Just the prog blocking with BPF_PROG_TYPE_SOCKET_FILTER added. I'll
leave SOCK_OPS out of this series.
Bests,
Sechang
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-06-30 7:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-29 17:26 [PATCH bpf-next v3 0/2] bpf, sockmap: disallow sockmap mutation from tc, xdp and flow_dissector Sechang Lim
2026-06-29 17:27 ` [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete " Sechang Lim
2026-06-29 17:42 ` sashiko-bot
2026-06-29 18:06 ` bot+bpf-ci
2026-06-29 18:24 ` Alexei Starovoitov
2026-06-30 5:47 ` John Fastabend
2026-06-30 7:14 ` Sechang Lim
2026-06-29 17:27 ` [PATCH bpf-next v3 2/2] selftests/bpf: drop tc/xdp/flow_dissector sockmap mutation tests Sechang Lim
2026-06-29 17:48 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox