All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Sitnicki <jakub@cloudflare.com>
To: bot+bpf-ci@kernel.org
Cc: bpf@vger.kernel.org,  davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, horms@kernel.org,
	martin.lau@linux.dev, daniel@iogearbox.net,
	john.fastabend@gmail.com, sdf@fomichev.me, ast@kernel.org,
	andrii@kernel.org, eddyz87@gmail.com, song@kernel.org,
	yonghong.song@linux.dev, kpsingh@kernel.org, haoluo@google.com,
	jolsa@kernel.org, arthur@arthurfabre.com, hawk@kernel.org,
	netdev@vger.kernel.org, kernel-team@cloudflare.com,
	martin.lau@kernel.org, clm@meta.com, ihor.solodrai@linux.dev
Subject: Re: [PATCH bpf-next v4 07/16] bpf: Make bpf_skb_adjust_room metadata-safe
Date: Mon, 10 Nov 2025 13:17:48 +0100	[thread overview]
Message-ID: <87bjlaf5eb.fsf@cloudflare.com> (raw)
In-Reply-To: <9b3ca8c01aac5510f1ff8738b3856e5e2bbe43075333c27642eb69a6847a50ef@mail.kernel.org> (bot's message of "Wed, 5 Nov 2025 20:42:03 +0000 (UTC)")

On Wed, Nov 05, 2025 at 08:42 PM GMT, bot+bpf-ci@kernel.org wrote:
>> 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?

bpf_skb_proto_4_to_6() is handled by the next patch (8/16).

LWT and other encap facilities are out of scope for this series.
bpf_lwt_seg6_adjust_srh() is on my todo list.

[...]

  reply	other threads:[~2025-11-10 12:17 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-05 20:19 [PATCH bpf-next v4 00/16] Make TC BPF helpers preserve skb metadata Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 01/16] net: Helper to move packet data and metadata after skb_push/pull Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 02/16] net: Preserve metadata on pskb_expand_head Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 03/16] bpf: Unclone skb head on bpf_dynptr_write to skb metadata Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 04/16] vlan: Make vlan_remove_tag return nothing Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 05/16] bpf: Make bpf_skb_vlan_pop helper metadata-safe Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 06/16] bpf: Make bpf_skb_vlan_push " Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 07/16] bpf: Make bpf_skb_adjust_room metadata-safe Jakub Sitnicki
2025-11-05 20:42   ` bot+bpf-ci
2025-11-10 12:17     ` Jakub Sitnicki [this message]
2025-11-05 20:19 ` [PATCH bpf-next v4 08/16] bpf: Make bpf_skb_change_proto helper metadata-safe Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 09/16] bpf: Make bpf_skb_change_head " Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 10/16] selftests/bpf: Verify skb metadata in BPF instead of userspace Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 11/16] selftests/bpf: Dump skb metadata on verification failure Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 12/16] selftests/bpf: Expect unclone to preserve skb metadata Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 13/16] selftests/bpf: Cover skb metadata access after vlan push/pop helper Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 14/16] selftests/bpf: Cover skb metadata access after bpf_skb_adjust_room Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 15/16] selftests/bpf: Cover skb metadata access after change_head/tail helper Jakub Sitnicki
2025-11-05 20:19 ` [PATCH bpf-next v4 16/16] selftests/bpf: Cover skb metadata access after bpf_skb_change_proto Jakub Sitnicki
2025-11-10 19:30 ` [PATCH bpf-next v4 00/16] Make TC BPF helpers preserve skb metadata patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bjlaf5eb.fsf@cloudflare.com \
    --to=jakub@cloudflare.com \
    --cc=andrii@kernel.org \
    --cc=arthur@arthurfabre.com \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=clm@meta.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=eddyz87@gmail.com \
    --cc=edumazet@google.com \
    --cc=haoluo@google.com \
    --cc=hawk@kernel.org \
    --cc=horms@kernel.org \
    --cc=ihor.solodrai@linux.dev \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kernel-team@cloudflare.com \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=martin.lau@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.