Cover skb_ext behavior when an skb is cloned, by TC mirred (mirror to a dummy device) and by bpf_clone_redirect(): - clone_ext_read: the extension written at tap ingress is readable from the mirred clone at dummy ingress -- the clone shares the extension with the original - clone_ext_cow: opening the extension with F_CREATE on the clone triggers copy-on-write, so overwriting the clone's data does not affect the original -- verified by a tp_btf/kfree_skb probe that checks the original skb still carries meta_want - clone_redir_ext_write_after / clone_redir_ext_slice_write_after: a write to an skb_ext dynptr after bpf_clone_redirect() fails with -EBUSY/NULL while the clone is queued on a netem-delayed loopback and keeps the ext block shared, and dynptrs and pointers to backing memory get invalidated on skb clone; re-acquiring the dynptr with F_CREATE COWs the block and the write succeeds Signed-off-by: Jakub Sitnicki --- tools/testing/selftests/bpf/config | 1 + .../bpf/prog_tests/xdp_context_test_run.c | 182 +++++++++++++++++++++ tools/testing/selftests/bpf/progs/test_xdp_meta.c | 126 ++++++++++++++ .../testing/selftests/bpf/progs/verifier_skb_ext.c | 45 +++++ 4 files changed, 354 insertions(+) diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config index 502f4504bdba..9bbfa351bced 100644 --- a/tools/testing/selftests/bpf/config +++ b/tools/testing/selftests/bpf/config @@ -137,3 +137,4 @@ CONFIG_SMC_HS_CTRL_BPF=y CONFIG_DIBS=y CONFIG_DIBS_LO=y CONFIG_PM_WAKELOCKS=y +CONFIG_NET_SCH_NETEM=y diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c index 252ac02de81b..f7617aef4d35 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c @@ -713,6 +713,178 @@ static void test_skb_ext_tuntap(struct bpf_program *tc_prio_1_prog, tc_prio_2_prog, test_pass); } +/* + * Test if skb_ext survives skb clone (via tc mirred). + * dummy_prog runs on the clone (dummy ingress). + */ +static void test_mirred_clone_ext(struct test_xdp_meta *skel, + struct bpf_program *dummy_prog) +{ + LIBBPF_OPTS(bpf_tc_hook, tc_hook, .attach_point = BPF_TC_INGRESS); + LIBBPF_OPTS(bpf_tc_opts, tc_opts, .handle = 1, .priority = 1); + struct netns_obj *ns = NULL; + int dummy_ifindex; + int tap_ifindex; + int tap_fd = -1; + int ret; + + skel->bss->write_done = false; + skel->bss->test_pass = false; + + ns = netns_new("mirred_clone", true); + if (!ASSERT_OK_PTR(ns, "netns_new")) + return; + + /* Dummy dev: attach reader */ + SYS(close, "ip link add name " DUMMY_NAME " type dummy"); + SYS(close, "ip link set dev " DUMMY_NAME " up"); + + dummy_ifindex = if_nametoindex(DUMMY_NAME); + if (!ASSERT_GT(dummy_ifindex, 0, "dummy_ifindex")) + goto close; + + tc_hook.ifindex = dummy_ifindex; + ret = bpf_tc_hook_create(&tc_hook); + if (!ASSERT_OK(ret, "dummy_hook_create")) + goto close; + + tc_opts.prog_fd = bpf_program__fd(dummy_prog); + ret = bpf_tc_attach(&tc_hook, &tc_opts); + if (!ASSERT_OK(ret, "dummy_attach")) + goto close; + + /* TAP dev: attach writer + mirred to dummy */ + tap_fd = open_tuntap(TAP_NAME, true); + if (!ASSERT_GE(tap_fd, 0, "open_tuntap")) + goto close; + + SYS(close, "ip link set dev " TAP_NAME " up"); + + tap_ifindex = if_nametoindex(TAP_NAME); + if (!ASSERT_GT(tap_ifindex, 0, "tap_ifindex")) + goto close; + + tc_hook.ifindex = tap_ifindex; + ret = bpf_tc_hook_create(&tc_hook); + if (!ASSERT_OK(ret, "tap_hook_create")) + goto close; + + tc_opts.prog_id = 0; + tc_opts.prog_fd = bpf_program__fd(skel->progs.tc_skb_ext_write); + ret = bpf_tc_attach(&tc_hook, &tc_opts); + if (!ASSERT_OK(ret, "tap_attach")) + goto close; + + SYS(close, "tc filter add dev " TAP_NAME " ingress " + "protocol all matchall " + "action mirred ingress mirror dev " DUMMY_NAME); + + ret = write_test_packet(tap_fd); + if (!ASSERT_OK(ret, "write_test_packet")) + goto close; + + ASSERT_TRUE(skel->bss->write_done, "write_done"); + ASSERT_TRUE(skel->bss->test_pass, "test_pass"); + +close: + if (tap_fd >= 0) + close(tap_fd); + netns_free(ns); +} + +static void test_mirred_clone_ext_cow(struct test_xdp_meta *skel) +{ + struct bpf_link *tp_link; + + skel->bss->clone_cow_done = false; + tp_link = bpf_program__attach(skel->progs.tp_kfree_skb_cow_check); + if (!ASSERT_OK_PTR(tp_link, "attach_tp")) + return; + + test_mirred_clone_ext(skel, skel->progs.tc_skb_ext_clone_redir_cow); + bpf_link__destroy(tp_link); +} + +/* + * Writer clones via bpf_clone_redirect() from the prog itself to loopback + * whose netem qdisc delays the clone, so it stays queued and keeps sharing + * the ext block while the prog writes again. When the qdisc finally leaks + * the clone back to lo ingress, the reader there must still see the + * clone-time snapshot. + */ +static void test_clone_redir_ext_write_after(struct test_xdp_meta *skel, + struct bpf_program *writer) +{ + LIBBPF_OPTS(bpf_tc_hook, tc_hook, .attach_point = BPF_TC_INGRESS); + LIBBPF_OPTS(bpf_tc_opts, tc_opts, .handle = 1, .priority = 1); + struct netns_obj *ns = NULL; + int tap_ifindex; + int tap_fd = -1; + int ret, i; + + skel->bss->test_pass = false; + skel->bss->write_blocked = false; + + ns = netns_new("clone_redir_ext", true); + if (!ASSERT_OK_PTR(ns, "netns_new")) + return; + + /* Redirect target: lo held back by netem delay */ + SYS(close, "ip link set dev lo up"); + SYS(close, "tc qdisc add dev lo root netem delay 50ms"); + + /* Reader on the clone: lo ingress */ + tc_hook.ifindex = if_nametoindex("lo"); + ret = bpf_tc_hook_create(&tc_hook); + if (!ASSERT_OK(ret, "lo_hook_create")) + goto close; + + tc_opts.prog_fd = bpf_program__fd(skel->progs.tc_skb_ext_read); + ret = bpf_tc_attach(&tc_hook, &tc_opts); + if (!ASSERT_OK(ret, "lo_attach")) + goto close; + + /* TAP dev: attach writer which clone_redirects to lo */ + tap_fd = open_tuntap(TAP_NAME, true); + if (!ASSERT_GE(tap_fd, 0, "open_tuntap")) + goto close; + + SYS(close, "ip link set dev " TAP_NAME " up"); + + tap_ifindex = if_nametoindex(TAP_NAME); + if (!ASSERT_GE(tap_ifindex, 0, "tap_ifindex")) + goto close; + + skel->bss->clone_redir_ifindex = tc_hook.ifindex; + + tc_hook.ifindex = tap_ifindex; + ret = bpf_tc_hook_create(&tc_hook); + if (!ASSERT_OK(ret, "tap_hook_create")) + goto close; + + tc_opts.prog_id = 0; + tc_opts.prog_fd = bpf_program__fd(writer); + ret = bpf_tc_attach(&tc_hook, &tc_opts); + if (!ASSERT_OK(ret, "tap_attach")) + goto close; + + ret = write_test_packet(tap_fd); + if (!ASSERT_OK(ret, "write_test_packet")) + goto close; + + ASSERT_TRUE(skel->bss->write_blocked, "write_blocked"); + + /* Clone arrives after the netem delay; poll every 10 msec up to 1 sec */ + for (i = 0; i < 100 && !skel->bss->test_pass; i++) + usleep(10000); + ASSERT_TRUE(skel->bss->test_pass, "test_pass"); + +close: + if (tap_fd >= 0) + close(tap_fd); + netns_free(ns); +} + void test_skb_ext_basic(void) { struct test_xdp_meta *skel = NULL; @@ -753,6 +925,16 @@ void test_skb_ext_basic(void) test_skb_ext_tuntap(skel->progs.tc_skb_ext_double_alloc, NULL, /* tc prio 2 */ &skel->bss->test_pass); + if (test__start_subtest("clone_ext_read")) + test_mirred_clone_ext(skel, skel->progs.tc_skb_ext_read); + if (test__start_subtest("clone_ext_cow")) + test_mirred_clone_ext_cow(skel); + if (test__start_subtest("clone_redir_ext_write_after")) + test_clone_redir_ext_write_after(skel, + skel->progs.tc_skb_ext_write_after_clone_redir); + if (test__start_subtest("clone_redir_ext_slice_write_after")) + test_clone_redir_ext_write_after(skel, + skel->progs.tc_skb_ext_slice_write_after_clone_redir); test_xdp_meta__destroy(skel); } diff --git a/tools/testing/selftests/bpf/progs/test_xdp_meta.c b/tools/testing/selftests/bpf/progs/test_xdp_meta.c index 43840ee32d35..ff134204a33e 100644 --- a/tools/testing/selftests/bpf/progs/test_xdp_meta.c +++ b/tools/testing/selftests/bpf/progs/test_xdp_meta.c @@ -3,6 +3,7 @@ #include #include +#include #include #include "bpf_kfuncs.h" @@ -690,6 +691,8 @@ int helper_skb_change_proto(struct __sk_buff *ctx) return TC_ACT_SHOT; } +bool write_done; + /* Write to skb_ext using bpf_dynptr_write helper */ SEC("tc") int tc_skb_ext_write(struct __sk_buff *ctx) @@ -703,6 +706,7 @@ int tc_skb_ext_write(struct __sk_buff *ctx) if (bpf_dynptr_write(&meta, 0, (void *)meta_want, ARRAY_SIZE(meta_want), 0)) return TC_ACT_SHOT; + write_done = true; return TC_ACT_UNSPEC; } @@ -874,4 +878,126 @@ int tc_skb_ext_double_alloc(struct __sk_buff *ctx) return TC_ACT_UNSPEC; } +static const __u8 meta_zero[META_SIZE] = {}; + +bool clone_cow_done; + +/* + * Overwrite skb_ext on the clone via F_CREATE (COW) -- must not affect original. + * Runs on the dummy ingress (clone side), synchronously during tc mirred. + */ +SEC("tc") +int tc_skb_ext_clone_redir_cow(struct __sk_buff *ctx) +{ + struct bpf_dynptr meta; + + if (bpf_dynptr_from_skb_ext(ctx, 0, BPF_SKB_EXT_F_CREATE, &meta)) + return TC_ACT_SHOT; + + /* Zero out the clone's ext -- must not affect original */ + if (bpf_dynptr_write(&meta, 0, (void *)meta_zero, META_SIZE, 0)) + return TC_ACT_SHOT; + + clone_cow_done = true; + return TC_ACT_SHOT; +} + +/* + * Verify COW isolation at kfree_skb time: once clone_cow_done is set, + * check that the original skb still has meta_want. + */ +SEC("tp_btf/kfree_skb") +int BPF_PROG(tp_kfree_skb_cow_check, struct sk_buff *skb) +{ + __u8 meta_have[META_SIZE]; + struct bpf_dynptr meta; + + if (!clone_cow_done) + return 0; + + if (bpf_dynptr_from_skb_ext((struct __sk_buff *)skb, 0, 0, &meta)) + return 0; + if (bpf_dynptr_read(meta_have, META_SIZE, &meta, 0, 0)) + return 0; + if (!check_metadata(meta_have)) + return 0; + + test_pass = true; + return 0; +} + +__u32 clone_redir_ifindex; +bool write_blocked; + +/* + * Write skb_ext, clone the skb to a delayed qdisc so the clone keeps the ext + * block shared, then try to write to the same dynptr again. The write must fail + * with -EBUSY. Re-acquiring the dynptr with F_CREATE COWs the block and + * succeeds. + */ +SEC("tc") +int tc_skb_ext_write_after_clone_redir(struct __sk_buff *ctx) +{ + struct bpf_dynptr meta; + + if (!is_test_packet_tc(ctx)) + return TC_ACT_UNSPEC; + if (bpf_dynptr_from_skb_ext(ctx, 0, BPF_SKB_EXT_F_CREATE, &meta)) + return TC_ACT_SHOT; + if (bpf_dynptr_write(&meta, 0, (void *)meta_want, META_SIZE, 0)) + return TC_ACT_SHOT; + + if (bpf_clone_redirect(ctx, clone_redir_ifindex, 0 /* egress */)) + return TC_ACT_SHOT; + + /* ext now shared with the queued clone -- write must fail */ + if (bpf_dynptr_write(&meta, 0, (void *)meta_zero, META_SIZE, 0) != -EBUSY) + return TC_ACT_SHOT; + + /* Re-acquiring COWs the block -- write must succeed */ + if (bpf_dynptr_from_skb_ext(ctx, 0, BPF_SKB_EXT_F_CREATE, &meta)) + return TC_ACT_SHOT; + if (bpf_dynptr_write(&meta, 0, (void *)meta_zero, META_SIZE, 0)) + return TC_ACT_SHOT; + + write_blocked = true; + return TC_ACT_UNSPEC; +} + +/* + * Same as above but using bpf_dynptr_slice_rdwr. + */ +SEC("tc") +int tc_skb_ext_slice_write_after_clone_redir(struct __sk_buff *ctx) +{ + struct bpf_dynptr meta; + void *slice; + + if (!is_test_packet_tc(ctx)) + return TC_ACT_UNSPEC; + if (bpf_dynptr_from_skb_ext(ctx, 0, BPF_SKB_EXT_F_CREATE, &meta)) + return TC_ACT_SHOT; + if (bpf_dynptr_write(&meta, 0, (void *)meta_want, META_SIZE, 0)) + return TC_ACT_SHOT; + + if (bpf_clone_redirect(ctx, clone_redir_ifindex, 0 /* egress */)) + return TC_ACT_SHOT; + + /* ext now shared with the queued clone -- slice must fail */ + slice = bpf_dynptr_slice_rdwr(&meta, 0, NULL, META_SIZE); + if (slice) + return TC_ACT_SHOT; + + /* Re-acquiring COWs the block -- slice must succeed */ + if (bpf_dynptr_from_skb_ext(ctx, 0, BPF_SKB_EXT_F_CREATE, &meta)) + return TC_ACT_SHOT; + slice = bpf_dynptr_slice_rdwr(&meta, 0, NULL, META_SIZE); + if (!slice) + return TC_ACT_SHOT; + __builtin_memcpy(slice, meta_zero, META_SIZE); + + write_blocked = true; + return TC_ACT_UNSPEC; +} + char _license[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/bpf/progs/verifier_skb_ext.c b/tools/testing/selftests/bpf/progs/verifier_skb_ext.c index db43f1051400..c2611c9315aa 100644 --- a/tools/testing/selftests/bpf/progs/verifier_skb_ext.c +++ b/tools/testing/selftests/bpf/progs/verifier_skb_ext.c @@ -7,6 +7,51 @@ __u64 flags; +SEC("tc") +__description("skb_ext slice is invalidated by bpf_clone_redirect") +__failure +int skb_ext_stale_slice_after_clone_redirect(struct __sk_buff *ctx) +{ + struct bpf_dynptr meta; + __u8 *slice; + + if (bpf_dynptr_from_skb_ext(ctx, 0, BPF_SKB_EXT_F_CREATE, &meta)) + return 0; + + slice = bpf_dynptr_slice_rdwr(&meta, 0, NULL, 8); + if (!slice) + return 0; + + bpf_clone_redirect(ctx, 1, 0); + + /* Stale: the clone shares the ext block the slice points into. */ + *slice = 0; + + return 0; +} + +SEC("tc") +__description("skb_ext slice is invalidated when ext is re-opened with F_CREATE") +__failure +int skb_ext_stale_slice_after_recreate(struct __sk_buff *ctx) +{ + struct bpf_dynptr d1, d2; + __u8 *slice; + + if (bpf_dynptr_from_skb_ext(ctx, 0, 0, &d1)) + return 0; + + slice = bpf_dynptr_slice(&d1, 0, NULL, 8); + if (!slice) + return 0; + + /* May COW the ext block, leaving the slice pointing at the old one. */ + if (bpf_dynptr_from_skb_ext(ctx, 0, BPF_SKB_EXT_F_CREATE, &d2)) + return 0; + + return *slice; +} + SEC("tp_btf/kfree_skb") __description("F_CREATE is rejected in tracing programs") __failure __msg("is not allowed in lsm/tracing programs") -- 2.43.0