netdev - Re: [PATCH bpf-next] bpf: stub out skb metadata dynptr read/write ops when CONFIG

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87plca5xpo.fsf@cloudflare.com>
Date: Mon, 01 Sep 2025 11:27:47 +0200
From: Jakub Sitnicki <jakub@...udflare.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: bpf <bpf@...r.kernel.org>,  Martin KaFai Lau <martin.lau@...ux.dev>,
  Alexei Starovoitov <ast@...nel.org>,  Andrii Nakryiko
 <andrii@...nel.org>,  Daniel Borkmann <daniel@...earbox.net>,  kernel-team
 <kernel-team@...udflare.com>,  Network Development
 <netdev@...r.kernel.org>,  kernel test robot <lkp@...el.com>
Subject: Re: [PATCH bpf-next] bpf: stub out skb metadata dynptr read/write
 ops when CONFIG_NET=n

On Wed, Aug 27, 2025 at 09:05 AM -07, Alexei Starovoitov wrote:
> On Wed, Aug 27, 2025 at 3:48 AM Jakub Sitnicki <jakub@...udflare.com> wrote:
>>
>> Kernel Test Robot reported a compiler warning - a null pointer may be
>> passed to memmove in __bpf_dynptr_{read,write} when building without
>> networking support.
>>
>> The warning is correct from a static analysis standpoint, but not actually
>> reachable. Without CONFIG_NET, creating dynptrs to skb metadata is
>> impossible since the constructor kfunc is missing.
>>
>> Fix this the same way as for skb and xdp data dynptrs. Add wrappers for
>> loading and storing bytes to skb metadata, and stub them out to return an
>> error when CONFIG_NET=n.
>>
>> Fixes: 6877cd392bae ("bpf: Enable read/write access to skb metadata through a dynptr")
>> Reported-by: kernel test robot <lkp@...el.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202508212031.ir9b3B6Q-lkp@intel.com/
>> Signed-off-by: Jakub Sitnicki <jakub@...udflare.com>
>> ---
>>  include/linux/filter.h | 26 ++++++++++++++++++++++++++
>>  kernel/bpf/helpers.c   |  6 ++----
>>  2 files changed, 28 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index 9092d8ea95c8..5b0d7c5824ac 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> @@ -1779,6 +1779,20 @@ void *bpf_xdp_pointer(struct xdp_buff *xdp, u32 offset, u32 len);
>>  void bpf_xdp_copy_buf(struct xdp_buff *xdp, unsigned long off,
>>                       void *buf, unsigned long len, bool flush);
>>  void *bpf_skb_meta_pointer(struct sk_buff *skb, u32 offset);
>> +
>> +static inline int __bpf_skb_meta_load_bytes(struct sk_buff *skb,
>> +                                           u32 offset, void *to, u32 len)
>> +{
>> +       memmove(to, bpf_skb_meta_pointer(skb, offset), len);
>> +       return 0;
>> +}
>> +
>> +static inline int __bpf_skb_meta_store_bytes(struct sk_buff *skb, u32 offset,
>> +                                            const void *from, u32 len)
>> +{
>> +       memmove(bpf_skb_meta_pointer(skb, offset), from, len);
>> +       return 0;
>> +}
>>  #else /* CONFIG_NET */
>>  static inline int __bpf_skb_load_bytes(const struct sk_buff *skb, u32 offset,
>>                                        void *to, u32 len)
>> @@ -1818,6 +1832,18 @@ static inline void *bpf_skb_meta_pointer(struct sk_buff *skb, u32 offset)
>>  {
>>         return NULL;
>>  }
>> +
>> +static inline int __bpf_skb_meta_load_bytes(struct sk_buff *skb, u32 offset,
>> +                                           void *to, u32 len)
>> +{
>> +       return -EOPNOTSUPP;
>> +}
>> +
>> +static inline int __bpf_skb_meta_store_bytes(struct sk_buff *skb, u32 offset,
>> +                                            const void *from, u32 len)
>> +{
>> +       return -EOPNOTSUPP;
>> +}
>
> imo that's too much to shut up the warn.
> Maybe make:
> static inline void *bpf_skb_meta_pointer(struct sk_buff *skb, u32 offset)
> {
>         return NULL;
> }
>
> to return ERR_PTR(-EOPNOTSUPP);
>
> instead?

Much nicer. Thanks for the suggestion.