Re: [PATCH net-next v2 05/14] selftests/bpf: Test skb_ext on cloned skbs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing __msg() macro in __failure verifier tests.
--

commit 3dc92388d2fd6ba878aeec16668062f704faf681
Author: Jakub Sitnicki <jakub@cloudflare.com>

selftests/bpf: Test skb_ext on cloned skbs

This commit adds coverage for skb_ext behavior when an skb is cloned,
using TC mirred and bpf_clone_redirect(). It tests whether extensions are
readable across clones and verifies copy-on-write isolation.

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/verifier_skb_ext.c b/tools/testing/selftests/bpf/progs/verifier_skb_ext.c
> index db43f10514001..c2611c9315aa7 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

[Severity: Medium]
Should this test include a __msg("...") macro to verify the exact error
string from the verifier?

If the test fails to load due to an unrelated verifier error (such as a
syntax issue, invalid BTF, or missing kfunc declarations), the test loader
merely checks if loading fails for any reason. This can incorrectly report
a PASS, hiding regressions in the specific slice invalidation logic it is
designed to test.

> +int skb_ext_stale_slice_after_clone_redirect(struct __sk_buff *ctx)
> +{
> +	struct bpf_dynptr meta;
> +	__u8 *slice;

[ ... ]

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

[Severity: Medium]
Similarly, does this test also need a __msg() macro? 

Without exact error message string verification, the test infrastructure
will pass this test if it fails to load for any reason, rather than
verifying the intended failure reason.

> +int skb_ext_stale_slice_after_recreate(struct __sk_buff *ctx)
> +{
> +	struct bpf_dynptr d1, d2;
> +	__u8 *slice;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-bpf-meta-inside-skb-ext-v2-0-0b21e42180b0@cloudflare.com?part=5