netdev - Re: [PATCH bpf-next v4 2/8] bpf: Enable read/write access to skb metadata through a dynptr

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: <87jz3xwf1h.fsf@cloudflare.com>
Date: Thu, 24 Jul 2025 21:43:22 +0200
From: Jakub Sitnicki <jakub@...udflare.com>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: Jakub Kicinski <kuba@...nel.org>,  bpf@...r.kernel.org,  Alexei
 Starovoitov <ast@...nel.org>,  Andrii Nakryiko <andrii@...nel.org>,
  Arthur Fabre <arthur@...hurfabre.com>,  Daniel Borkmann
 <daniel@...earbox.net>,  Eduard Zingerman <eddyz87@...il.com>,  Eric
 Dumazet <edumazet@...gle.com>,  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>,
  kernel-team@...udflare.com,  netdev@...r.kernel.org,  Stanislav Fomichev
 <sdf@...ichev.me>
Subject: Re: [PATCH bpf-next v4 2/8] bpf: Enable read/write access to skb
 metadata through a dynptr

On Thu, Jul 24, 2025 at 08:52 AM -07, Martin KaFai Lau wrote:
> On 7/24/25 4:53 AM, Jakub Sitnicki wrote:
>> In this series we maintain the status quo. Access metadata dynptr is
>> limited to TC BPF hook only, so we provide the same guarntees as the
>> existing __sk_buff->data_meta.
>
> The verifier tracks if the __sk_buff->data_meta is written in
> "seen_direct_write". tc_cls_act_prologue is called and that should have
> triggered skb_metadata_clear for a clone skb. Meaning, for a clone skb, I think
> __sk_buff->data_meta is read-only.
>
> bpf_dynptr_from_skb_meta can set the DYNPTR_RDONLY_BIT if the skb is a clone.

Oh that it clever. TIL. So if we end up calling:

tc_cls_act_prologue
  bpf_skb_pull_data
    bpf_try_make_writable(skb, skb_headlen(skb))
      __bpf_try_make_writable
        skb_ensure_writable
          pskb_expand_head
            skb_metadata_clear
              skb_metadata_set(skb, 0)
                skb_shinfo(skb)->meta_len = 0;

... then the metadata is not so much read-only but inaccessible for the
clone. The dynptr will reflect this state, so all seems right.

Let me see if I can capture that in a test, though.

BTW. I've wondered why pskb_expand_head doesn't just copy the metadata,
but left dealing with it till the next step. If it did, we could just
operate on a copy.