netdev - Re: [PATCH v10 bpf-next 6/9] bpf: Add skb dynptrs

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]

Date:   Fri, 17 Feb 2023 16:55:02 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Joanne Koong <joannelkoong@...il.com>
Cc:     bpf@...r.kernel.org, martin.lau@...nel.org, andrii@...nel.org,
        memxor@...il.com, ast@...nel.org, daniel@...earbox.net,
        netdev@...r.kernel.org, kernel-team@...com
Subject: Re: [PATCH v10 bpf-next 6/9] bpf: Add skb dynptrs

On Thu, Feb 16, 2023 at 2:56 PM Joanne Koong <joannelkoong@...il.com> wrote:
>
> Add skb dynptrs, which are dynptrs whose underlying pointer points
> to a skb. The dynptr acts on skb data. skb dynptrs have two main
> benefits. One is that they allow operations on sizes that are not
> statically known at compile-time (eg variable-sized accesses).
> Another is that parsing the packet data through dynptrs (instead of
> through direct access of skb->data and skb->data_end) can be more
> ergonomic and less brittle (eg does not need manual if checking for
> being within bounds of data_end).
>
> For bpf prog types that don't support writes on skb data, the dynptr is
> read-only (bpf_dynptr_write() will return an error)
>
> For reads and writes through the bpf_dynptr_read() and bpf_dynptr_write()
> interfaces, reading and writing from/to data in the head as well as from/to
> non-linear paged buffers is supported. Data slices through the
> bpf_dynptr_data API are not supported; instead bpf_dynptr_slice() and
> bpf_dynptr_slice_rdwr() (added in subsequent commit) should be used.
>
> For examples of how skb dynptrs can be used, please see the attached
> selftests.
>
> Signed-off-by: Joanne Koong <joannelkoong@...il.com>
> ---
>  include/linux/bpf.h            | 14 ++++++-
>  include/linux/filter.h         | 18 ++++++++
>  include/uapi/linux/bpf.h       | 14 ++++++-
>  kernel/bpf/btf.c               | 18 ++++++++
>  kernel/bpf/helpers.c           | 76 +++++++++++++++++++++++++++-------
>  kernel/bpf/verifier.c          | 70 ++++++++++++++++++++++++++++++-
>  net/core/filter.c              | 67 ++++++++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h | 14 ++++++-
>  8 files changed, 270 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 296841a31749..3d18be35a5e6 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -607,11 +607,14 @@ enum bpf_type_flag {
>          */
>         NON_OWN_REF             = BIT(14 + BPF_BASE_TYPE_BITS),
>
> +       /* DYNPTR points to sk_buff */
> +       DYNPTR_TYPE_SKB         = BIT(15 + BPF_BASE_TYPE_BITS),
> +
>         __BPF_TYPE_FLAG_MAX,
>         __BPF_TYPE_LAST_FLAG    = __BPF_TYPE_FLAG_MAX - 1,
>  };
>
> -#define DYNPTR_TYPE_FLAG_MASK  (DYNPTR_TYPE_LOCAL | DYNPTR_TYPE_RINGBUF)
> +#define DYNPTR_TYPE_FLAG_MASK  (DYNPTR_TYPE_LOCAL | DYNPTR_TYPE_RINGBUF | DYNPTR_TYPE_SKB)
>
>  /* Max number of base types. */
>  #define BPF_BASE_TYPE_LIMIT    (1UL << BPF_BASE_TYPE_BITS)
> @@ -1146,6 +1149,8 @@ enum bpf_dynptr_type {
>         BPF_DYNPTR_TYPE_LOCAL,
>         /* Underlying data is a ringbuf record */
>         BPF_DYNPTR_TYPE_RINGBUF,
> +       /* Underlying data is a sk_buff */
> +       BPF_DYNPTR_TYPE_SKB,
>  };
>
>  int bpf_dynptr_check_size(u32 size);
> @@ -2846,6 +2851,8 @@ u32 bpf_sock_convert_ctx_access(enum bpf_access_type type,
>                                 struct bpf_insn *insn_buf,
>                                 struct bpf_prog *prog,
>                                 u32 *target_size);
> +int bpf_dynptr_from_skb_rdonly(struct sk_buff *skb, u64 flags,
> +                              struct bpf_dynptr_kern *ptr);
>  #else
>  static inline bool bpf_sock_common_is_valid_access(int off, int size,
>                                                    enum bpf_access_type type,
> @@ -2867,6 +2874,11 @@ static inline u32 bpf_sock_convert_ctx_access(enum bpf_access_type type,
>  {
>         return 0;
>  }
> +static inline int bpf_dynptr_from_skb_rdonly(struct sk_buff *skb, u64 flags,
> +                                            struct bpf_dynptr_kern *ptr)
> +{
> +       return 0;


should this return -EOPNOTSUPP instead?

> +}
>  #endif
>
>  #ifdef CONFIG_INET

[...]