netdev - Re: [PATCH bpf-next v4 07/16] bpf: Make bpf_skb_adjust

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] [day] [month] [year] [list]

Message-Id: <9b3ca8c01aac5510f1ff8738b3856e5e2bbe43075333c27642eb69a6847a50ef@mail.kernel.org>
Date: Wed,  5 Nov 2025 20:42:03 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: jakub@...udflare.com,bpf@...r.kernel.org
Cc: davem@...emloft.net,edumazet@...gle.com,kuba@...nel.org,pabeni@...hat.com,horms@...nel.org,martin.lau@...ux.dev,daniel@...earbox.net,john.fastabend@...il.com,sdf@...ichev.me,ast@...nel.org,andrii@...nel.org,eddyz87@...il.com,song@...nel.org,yonghong.song@...ux.dev,kpsingh@...nel.org,haoluo@...gle.com,jolsa@...nel.org,arthur@...hurfabre.com,hawk@...nel.org,netdev@...r.kernel.org,kernel-team@...udflare.com,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [PATCH bpf-next v4 07/16] bpf: Make bpf_skb_adjust_room metadata-safe

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 96714eab9..370ddc61b 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3260,11 +3260,11 @@ static void bpf_skb_change_protocol(struct sk_buff *skb, u16 proto)
>
>  static int bpf_skb_generic_push(struct sk_buff *skb, u32 off, u32 len)
>  {
> -	/* Caller already did skb_cow() with len as headroom,
> +	/* Caller already did skb_cow() with meta_len+len as headroom,
>  	 * so no need to do it here.
>  	 */
>  	skb_push(skb, len);
> -	memmove(skb->data, skb->data + len, off);
> +	skb_postpush_data_move(skb, len, off);
>  	memset(skb->data + off, 0, len);

The comment update says callers provide meta_len+len as headroom, but
is this true for all callers?

bpf_skb_proto_4_to_6() calls skb_cow(skb, len_diff) then calls
bpf_skb_net_hdr_push()->bpf_skb_generic_push(). Similarly,
bpf_lwt_seg6_adjust_srh() calls skb_cow_head(skb, len) then calls
bpf_skb_net_hdr_push()->bpf_skb_generic_push().

Neither accounts for meta_len in their headroom calculation. When
skb_postpush_data_move()->skb_data_move() runs, if meta_len >
skb_headroom(skb) after the push, the metadata gets silently cleared
(see the WARN_ON_ONCE check in skb_data_move() at skbuff.h).

Can the metadata be lost when these functions are called on skbs with
metadata?

> @@ -3496,6 +3496,7 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 off, u32 len_diff,
>  	u8 inner_mac_len = flags >> BPF_ADJ_ROOM_ENCAP_L2_SHIFT;
>  	bool encap = flags & BPF_F_ADJ_ROOM_ENCAP_L3_MASK;
>  	u16 mac_len = 0, inner_net = 0, inner_trans = 0;
> +	const u8 meta_len = skb_metadata_len(skb);
>  	unsigned int gso_type = SKB_GSO_DODGY;
>  	int ret;
>
> @@ -3506,7 +3507,7 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 off, u32 len_diff,
>  			return -ENOTSUPP;
>  	}
>
> -	ret = skb_cow_head(skb, len_diff);
> +	ret = skb_cow_head(skb, meta_len + len_diff);
>  	if (unlikely(ret < 0))
>  		return ret;


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19115341225