netdev - Re: [PATCH 09/11] bpf: Add d

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 18 Jun 2020 21:35:10 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Jiri Olsa <jolsa@...nel.org>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Martin KaFai Lau <kafai@...com>,
        David Miller <davem@...hat.com>,
        John Fastabend <john.fastabend@...il.com>,
        Wenbo Zhang <ethercflow@...il.com>,
        KP Singh <kpsingh@...omium.org>,
        Andrii Nakryiko <andriin@...com>,
        Brendan Gregg <bgregg@...flix.com>,
        Florent Revest <revest@...omium.org>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH 09/11] bpf: Add d_path helper

On Tue, Jun 16, 2020 at 3:07 AM Jiri Olsa <jolsa@...nel.org> wrote:
>
> Adding d_path helper function that returns full path
> for give 'struct path' object, which needs to be the
> kernel BTF 'path' object.
>
> The helper calls directly d_path function.
>
> Updating also bpf.h tools uapi header and adding
> 'path' to bpf_helpers_doc.py script.
>
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
>  include/linux/bpf.h            |  4 ++++
>  include/uapi/linux/bpf.h       | 14 ++++++++++++-
>  kernel/bpf/btf_ids.c           | 11 ++++++++++
>  kernel/trace/bpf_trace.c       | 38 ++++++++++++++++++++++++++++++++++
>  scripts/bpf_helpers_doc.py     |  2 ++
>  tools/include/uapi/linux/bpf.h | 14 ++++++++++++-
>  6 files changed, 81 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index a94e85c2ec50..d35265b6c574 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1752,5 +1752,9 @@ extern int bpf_skb_output_btf_ids[];
>  extern int bpf_seq_printf_btf_ids[];
>  extern int bpf_seq_write_btf_ids[];
>  extern int bpf_xdp_output_btf_ids[];
> +extern int bpf_d_path_btf_ids[];
> +
> +extern int btf_whitelist_d_path[];
> +extern int btf_whitelist_d_path_cnt;

So with suggestion from previous patch, this would be declared as:

extern const struct btf_id_set btf_whitelist_d_path;

>
>  #endif /* _LINUX_BPF_H */
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index c65b374a5090..e308746b9344 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3252,6 +3252,17 @@ union bpf_attr {
>   *             case of **BPF_CSUM_LEVEL_QUERY**, the current skb->csum_level
>   *             is returned or the error code -EACCES in case the skb is not
>   *             subject to CHECKSUM_UNNECESSARY.
> + *
> + * int bpf_d_path(struct path *path, char *buf, u32 sz)
> + *     Description
> + *             Return full path for given 'struct path' object, which
> + *             needs to be the kernel BTF 'path' object. The path is
> + *             returned in buffer provided 'buf' of size 'sz'.
> + *
> + *     Return
> + *             length of returned string on success, or a negative
> + *             error in case of failure
> + *
>   */
>  #define __BPF_FUNC_MAPPER(FN)          \
>         FN(unspec),                     \
> @@ -3389,7 +3400,8 @@ union bpf_attr {
>         FN(ringbuf_submit),             \
>         FN(ringbuf_discard),            \
>         FN(ringbuf_query),              \
> -       FN(csum_level),
> +       FN(csum_level),                 \
> +       FN(d_path),
>
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/kernel/bpf/btf_ids.c b/kernel/bpf/btf_ids.c
> index d8d0df162f04..853c8fd59b06 100644
> --- a/kernel/bpf/btf_ids.c
> +++ b/kernel/bpf/btf_ids.c
> @@ -13,3 +13,14 @@ BTF_ID(struct, seq_file)
>
>  BTF_ID_LIST(bpf_xdp_output_btf_ids)
>  BTF_ID(struct, xdp_buff)
> +
> +BTF_ID_LIST(bpf_d_path_btf_ids)
> +BTF_ID(struct, path)
> +
> +BTF_WHITELIST_ENTRY(btf_whitelist_d_path)
> +BTF_ID(func, vfs_truncate)
> +BTF_ID(func, vfs_fallocate)
> +BTF_ID(func, dentry_open)
> +BTF_ID(func, vfs_getattr)
> +BTF_ID(func, filp_close)
> +BTF_WHITELIST_END(btf_whitelist_d_path)

Oh, so that's why you added btf_ids.c. Do you think centralizing all
those BTF ID lists in one file is going to be more convenient? I lean
towards keeping them closer to where they are used, as it was with all
those helper BTF IDS. But I wonder what others think...

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index c1866d76041f..0ff5d8434d40 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1016,6 +1016,42 @@ static const struct bpf_func_proto bpf_send_signal_thread_proto = {
>         .arg1_type      = ARG_ANYTHING,
>  };
>

[...]