Re: [PATCH bpf-next] bpf: stub out skb metadata dynptr read/write ops when CONFIG_NET=n

[Jakub Sitnicki <jakub@cloudflare.com>]

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@cloudflare.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@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202508212031.ir9b3B6Q-lkp@intel.com/
>> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.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.