netdev - Re: [PATCH bpf-next 01/13] bpf: Ignore dynptr offset in skb data access

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: <87bjq3nguv.fsf@cloudflare.com>
Date: Wed, 02 Jul 2025 10:20:56 +0200
From: Jakub Sitnicki <jakub@...udflare.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: bpf@...r.kernel.org,  Alexei Starovoitov <ast@...nel.org>,  Arthur Fabre
 <arthur@...hurfabre.com>,  Eric Dumazet <edumazet@...gle.com>,  Jakub
 Kicinski <kuba@...nel.org>,  Jesper Dangaard Brouer <hawk@...nel.org>,
  Jesse Brandeburg <jbrandeburg@...udflare.com>,  Joanne Koong
 <joannelkoong@...il.com>,  Lorenzo Bianconi <lorenzo@...nel.org>,  Toke
 Høiland-Jørgensen <thoiland@...hat.com>,  Yan Zhai
 <yan@...udflare.com>,
  netdev@...r.kernel.org,  kernel-team@...udflare.com,  Stanislav Fomichev
 <sdf@...ichev.me>
Subject: Re: [PATCH bpf-next 01/13] bpf: Ignore dynptr offset in skb data
 access

On Tue, Jul 01, 2025 at 01:55 PM -07, Andrii Nakryiko wrote:
> On Mon, Jun 30, 2025 at 8:23 AM Jakub Sitnicki <jakub@...udflare.com> wrote:
>>
>> Prepare to use (struct bpf_dynptr)->offset to distinguish between an skb
>> dynptr for the payload vs the metadata area.
>>
>> ptr->offset is always set to zero by bpf_dynptr_from_skb(). We don't need
>> to account for it on access.
>
> Huh?.. What about bpf_dynptr_adjust()? This is a wrong approach to
> have some magical offset values.

Crap. I'm not gonna lie. I totally missed that.

You're right. It completely breaks down.

I was hoping I could piggyback on skb dynptr, but doesn't look like it.

> More general question about your patch set: is there ever a need to
> work with both metadata and data as one area of memory (i.e., copying
> both metadata and data in the same single operation, or setting it as
> one thing?). If not, why not have two different dynptrs, one for data
> (what we have today) and one exclusively for packet's metadata?

Having two dynptr kinds, one for payload, one for metadata, sounds like
a much better direction. I will pivot to that.

Metadata and payload are logically separate, AFAIK. It just so happens
that the metadata is currently located in front of the payload.

I asked around to find out why is it so - it seems that the decision was
made to place the metadata like that becase it saves you one additional
pointer load. Otherwise you'd need something like
__sk_buff->data_meta_end to marks the end of metadata.