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