Re: [PATCH bpf-next v6 9/9] selftests/bpf: Cover metadata access from a modified skb clone

[Martin KaFai Lau <martin.lau@xxxxxxxxx>]

On 8/4/25 5:52 AM, Jakub Sitnicki wrote:
> +/* Check that skb_meta dynptr is empty */
> +SEC("tc")
> +int ing_cls_dynptr_empty(struct __sk_buff *ctx)
> +{
> +	struct bpf_dynptr data, meta;
> +	struct ethhdr *eth;
> +
> +	bpf_dynptr_from_skb(ctx, 0, &data);
> +	eth = bpf_dynptr_slice_rdwr(&data, 0, NULL, sizeof(*eth));

If this is bpf_dynptr_slice() instead of bpf_dynptr_slice_rdwr() and...

> +	if (!eth)
> +		goto out;
> +	/* Ignore non-test packets */
> +	if (eth->h_proto != 0)
> +		goto out;
> +	/* Packet write to trigger unclone in prologue */
> +	eth->h_proto = 42;

... remove this eth->h_proto write.

Then bpf_dynptr_write() will succeed. like,

        bpf_dynptr_from_skb(ctx, 0, &data);
        eth = bpf_dynptr_slice(&data, 0, NULL, sizeof(*eth));
	if (!eth)
                goto out;

	/* Ignore non-test packets */
        if (eth->h_proto != 0)
		goto out;

        bpf_dynptr_from_skb_meta(ctx, 0, &meta);
        /* Expect write to fail because skb is a clone. */
        err = bpf_dynptr_write(&meta, 0, (void *)eth, sizeof(*eth), 0);

The bpf_dynptr_write for a skb dynptr will do the pskb_expand_head(). The 
skb_meta dynptr write is only a memmove. It probably can also do 
pskb_expand_head() and change it to keep the data_meta.

Another option is to set the DYNPTR_RDONLY_BIT in bpf_dynptr_from_skb_meta() for 
a clone skb. This restriction can be removed in the future.

> +
> +	/* Expect no metadata */
> +	bpf_dynptr_from_skb_meta(ctx, 0, &meta);
> +	if (bpf_dynptr_size(&meta) > 0)
> +		goto out;
> +
> +	test_pass = true;
> +out:
> +	return TC_ACT_SHOT;
> +}