Re: [PATCH net-next 04/14] selftests/bpf: Add tests for bpf_dynptr_from_skb_ext

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] A Use-After-Free (UAF) read/write vulnerability exists because `bpf_dynptr_from_skb_ext` with `BPF_SKB_EXT_F_CREATE` can force a copy-on-write (COW) reallocation of `skb->extensions`. This drops the reference to the old extension buffer, potentially freeing it. However, the verifier does not invalidate existing `PTR_TO_MEM` pointers to the old extension buffer obtained via `bpf_dynptr_slice()`, allowing BPF programs to access freed kernel memory.
- [Medium] The new test `test_skb_ext_basic` implicitly uses hardcoded network namespace and interface names (`TAP_NETNS` and `TAP_NAME`) by calling the `test_tuntap` helper. This will clash with the existing `test_xdp_context_tuntap` test when run concurrently via `test_progs -j`.
--

--- Patch [4]: [PATCH net-next 04/14] selftests/bpf: Add tests for bpf_dynptr_from_skb_ext ---
commit 4d32fa867b7073dd6ac590aca746b7f46b659dce
Author: Jakub Sitnicki <jakub@cloudflare.com>

selftests/bpf: Add tests for bpf_dynptr_from_skb_ext

Cover the bpf_dynptr_from_skb_ext() kfunc and the dynptr interface
to skb_ext with TC-to-TC tests on a tuntap device:

- write/read via bpf_dynptr_read and bpf_dynptr_write
- write/read via bpf_dynptr_slice and bpf_dynptr_slice_rdwr
- clone read via bpf_dynptr_clone
- error paths: no allocation without F_CREATE, invalid flags, read-only
  enforcement without F_CREATE
- double allocation: data from first alloc survives second skb_ext_add

> 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 448807676176c..fd340e5538a34 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
> @@ -693,3 +693,55 @@ void test_xdp_context_lwt_encap(void)
>  
>  	test_xdp_meta__destroy(skel);
>  }
> +
> +void test_skb_ext_basic(void)
> +{
> +	struct test_xdp_meta *skel = NULL;
> +
> +	skel = test_xdp_meta__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "open and load skeleton"))
> +		return;
> +
> +	if (test__start_subtest("tc_write_read"))
> +		test_tuntap(NULL, /* xdp */

[Severity: Medium]
Might this new test cause a race condition when run concurrently?

The test_tuntap() helper creates a network namespace using a hardcoded
string:

tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c:test_tuntap() {
    ...
	ns = netns_new(TAP_NETNS, true);
    ...
}

If test_skb_ext_basic() and other tests like test_xdp_context_tuntap() are
executed in parallel worker processes via test_progs -j, will they clash by
attempting to create and delete the identically named network namespace,
causing spurious test failures?

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_meta.c b/tools/testing/selftests/bpf/progs/test_xdp_meta.c
> index 08b03be0b891c..43840ee32d359 100644
> --- a/tools/testing/selftests/bpf/progs/test_xdp_meta.c
> +++ b/tools/testing/selftests/bpf/progs/test_xdp_meta.c
[ ... ]
> @@ -689,4 +690,188 @@ int helper_skb_change_proto(struct __sk_buff *ctx)
>  	return TC_ACT_SHOT;
>  }
>  
> +/* Write to skb_ext using bpf_dynptr_write helper */
> +SEC("tc")
> +int tc_skb_ext_write(struct __sk_buff *ctx)
> +{
> +	struct bpf_dynptr meta;
> +
> +	if (!is_test_packet_tc(ctx))
> +		return TC_ACT_SHOT;
> +	if (bpf_dynptr_from_skb_ext(ctx, 0, BPF_SKB_EXT_F_CREATE, &meta))

[Severity: Critical]
Can the use of BPF_SKB_EXT_F_CREATE lead to a use-after-free regression?

When BPF_SKB_EXT_F_CREATE is used, bpf_dynptr_from_skb_ext() can force a
copy-on-write reallocation of skb->extensions:

net/core/filter.c:bpf_dynptr_from_skb_ext() {
    ...
	ext = skb_ext_add(skb, SKB_EXT_BPF);
    ...
}

This drops the reference to the old extension buffer, potentially freeing it.
However, the verifier does not currently treat bpf_dynptr_from_skb_ext() as a
packet-changing kfunc:

kernel/bpf/verifier.c:bpf_is_kfunc_pkt_changing() {
	return meta->func_id == special_kfunc_list[KF_bpf_xdp_pull_data];
}

If a BPF program obtains a direct pointer slice to skb->extensions via
bpf_dynptr_slice(), then calls bpf_dynptr_from_skb_ext() with
BPF_SKB_EXT_F_CREATE on a cloned skb, the old extension buffer could be freed.

Will the verifier fail to invalidate the existing PTR_TO_MEM pointers to the
old extension buffer, allowing the BPF program to access freed kernel memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814-bpf-meta-inside-skb-ext-v1-0-767edd862656@cloudflare.com?part=4