netdev - Re: [PATCH bpf-next v2 11/15] selftests/bpf: Expect unclone to preserve skb metadata

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2753c96b-48f9-480e-923c-60d2c20ebb03@linux.dev>
Date: Wed, 22 Oct 2025 16:12:39 -0700
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Jakub Sitnicki <jakub@...udflare.com>
Cc: "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>,
 John Fastabend <john.fastabend@...il.com>,
 Stanislav Fomichev <sdf@...ichev.me>, Alexei Starovoitov <ast@...nel.org>,
 Andrii Nakryiko <andrii@...nel.org>, Eduard Zingerman <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
 KP Singh <kpsingh@...nel.org>, Hao Luo <haoluo@...gle.com>,
 Jiri Olsa <jolsa@...nel.org>, Arthur Fabre <arthur@...hurfabre.com>,
 bpf@...r.kernel.org, netdev@...r.kernel.org, kernel-team@...udflare.com
Subject: Re: [PATCH bpf-next v2 11/15] selftests/bpf: Expect unclone to
 preserve skb metadata

On 10/19/25 5:45 AM, Jakub Sitnicki wrote:
> @@ -447,12 +448,14 @@ int clone_dynptr_empty_on_meta_slice_write(struct __sk_buff *ctx)
>   
>   /*
>    * Check that skb_meta dynptr is read-only before prog writes to packet payload
> - * using dynptr_write helper. Applies only to cloned skbs.
> + * using dynptr_write helper, and becomes read-write afterwards. Applies only to
> + * cloned skbs.
>    */
>   SEC("tc")
> -int clone_dynptr_rdonly_before_data_dynptr_write(struct __sk_buff *ctx)
> +int clone_dynptr_rdonly_before_data_dynptr_write_then_rw(struct __sk_buff *ctx)
>   {
>   	struct bpf_dynptr data, meta;
> +	__u8 meta_have[META_SIZE];
>   	const struct ethhdr *eth;
>   
>   	bpf_dynptr_from_skb(ctx, 0, &data);
> @@ -465,15 +468,23 @@ int clone_dynptr_rdonly_before_data_dynptr_write(struct __sk_buff *ctx)
>   
>   	/* Expect read-only metadata before unclone */
>   	bpf_dynptr_from_skb_meta(ctx, 0, &meta);
> -	if (!bpf_dynptr_is_rdonly(&meta) || bpf_dynptr_size(&meta) != META_SIZE)
> +	if (!bpf_dynptr_is_rdonly(&meta))

Can the bpf_dynptr_set_rdonly() be lifted from the 
bpf_dynptr_from_skb_meta()?

iiuc, the remaining thing left should be handling a cloned skb in 
__bpf_dynptr_write()? The __bpf_skb_store_bytes() is using 
bpf_try_make_writable, so maybe something similar can be done for the 
BPF_DYNPTR_TYPE_SKB_META?

> +		goto out;
> +
> +	bpf_dynptr_read(meta_have, META_SIZE, &meta, 0, 0);
> +	if (!check_metadata(meta_have))
>   		goto out;
>   
>   	/* Helper write to payload will unclone the packet */
>   	bpf_dynptr_write(&data, offsetof(struct ethhdr, h_proto), "x", 1, 0);
>   
> -	/* Expect no metadata after unclone */
> +	/* Expect r/w metadata after unclone */
>   	bpf_dynptr_from_skb_meta(ctx, 0, &meta);
> -	if (bpf_dynptr_is_rdonly(&meta) || bpf_dynptr_size(&meta) != 0)
> +	if (bpf_dynptr_is_rdonly(&meta))

then it does not have to rely on the bpf_dynptr_write(&data, ...) above 
to make the metadata writable.

I have a high level question about the set. I assume the skb_data_move() 
in patch 2 will be useful in the future to preserve the metadata across 
the stack. Preserving the metadata across different tc progs (which this 
set does) is nice to have but it is not the end goal. Can you shed some 
light on the plan for building on top of this set?