netdev - Re: [PATCH v2 bpf-next 10/16] bpf: Add bpf_btf_find_by_name

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]

Message-ID: <CAEf4BzYkzzN=ZD2X1bOg8U39Whbe6oTPuUEMOpACw6NPEW69NA@mail.gmail.com>
Date:   Mon, 26 Apr 2021 15:46:29 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH v2 bpf-next 10/16] bpf: Add bpf_btf_find_by_name_kind() helper.

On Thu, Apr 22, 2021 at 5:27 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> From: Alexei Starovoitov <ast@...nel.org>
>
> Add new helper:
>
> long bpf_btf_find_by_name_kind(u32 btf_fd, char *name, u32 kind, int flags)
>         Description
>                 Find given name with given type in BTF pointed to by btf_fd.
>                 If btf_fd is zero look for the name in vmlinux BTF and in module's BTFs.
>         Return
>                 Returns btf_id and btf_obj_fd in lower and upper 32 bits.
>
> Signed-off-by: Alexei Starovoitov <ast@...nel.org>
> ---
>  include/linux/bpf.h            |  1 +
>  include/uapi/linux/bpf.h       |  8 ++++
>  kernel/bpf/btf.c               | 68 ++++++++++++++++++++++++++++++++++
>  kernel/bpf/syscall.c           |  2 +
>  tools/include/uapi/linux/bpf.h |  8 ++++
>  5 files changed, 87 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 0f841bd0cb85..4cf361eb6a80 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1972,6 +1972,7 @@ extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto;
>  extern const struct bpf_func_proto bpf_task_storage_get_proto;
>  extern const struct bpf_func_proto bpf_task_storage_delete_proto;
>  extern const struct bpf_func_proto bpf_for_each_map_elem_proto;
> +extern const struct bpf_func_proto bpf_btf_find_by_name_kind_proto;
>
>  const struct bpf_func_proto *bpf_tracing_func_proto(
>         enum bpf_func_id func_id, const struct bpf_prog *prog);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index de58a714ed36..253f5f031f08 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -4748,6 +4748,13 @@ union bpf_attr {
>   *             Execute bpf syscall with given arguments.
>   *     Return
>   *             A syscall result.
> + *
> + * long bpf_btf_find_by_name_kind(u32 btf_fd, char *name, u32 kind, int flags)
> + *     Description
> + *             Find given name with given type in BTF pointed to by btf_fd.

"Find BTF type with given name"? Should the limits on name length be
specified? KSYM_NAME_LEN is a pretty arbitrary restriction. Also,
would it still work fine if the caller provides a pointer to a much
shorter piece of memory?

Why not add name_sz right after name, as we do with a lot of other
arguments like this?

> + *             If btf_fd is zero look for the name in vmlinux BTF and in module's BTFs.
> + *     Return
> + *             Returns btf_id and btf_obj_fd in lower and upper 32 bits.

Mention that for vmlinux BTF btf_obj_fd will be zero? Also who "owns"
the FD? If the BPF program doesn't close it, when are they going to be
cleaned up?

>   */
>  #define __BPF_FUNC_MAPPER(FN)          \
>         FN(unspec),                     \
> @@ -4917,6 +4924,7 @@ union bpf_attr {
>         FN(for_each_map_elem),          \
>         FN(snprintf),                   \
>         FN(sys_bpf),                    \
> +       FN(btf_find_by_name_kind),      \
>         /* */
>

[...]