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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAADnVQL_8guWC9io1P5jhTgnyD3u=0WvTnHM3DJFVvE_Sy7DBw@mail.gmail.com>
Date: Wed, 27 Aug 2025 09:05:06 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Jakub Sitnicki <jakub@...udflare.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 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?