On Fri, Nov 11, 2022 at 6:33 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> Adding bpf_vma_build_id_parse function to retrieve build id from
> passed vma object and making it available as bpf kfunc.
>
> We can't use build_id_parse directly as kfunc, because we would
> not have control over the build id buffer size provided by user.
>
> Instead we are adding new bpf_vma_build_id_parse function with
> 'build_id__sz' argument that instructs verifier to check for the
> available space in build_id buffer.
>
> This way we check that there's always available memory space
> behind build_id pointer. We also check that the build_id__sz is
> at least BUILD_ID_SIZE_MAX so we can place any buildid in.
>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> include/linux/bpf.h | 5 +++++
> kernel/bpf/helpers.c | 16 ++++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 798aec816970..5e7c4c50da8e 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2779,4 +2779,9 @@ struct bpf_key {
> bool has_ref;
> };
> #endif /* CONFIG_KEYS */
> +
> +extern int bpf_vma_build_id_parse(struct vm_area_struct *vma,
> + unsigned char *build_id,
> + size_t build_id__sz);
> +
> #endif /* _LINUX_BPF_H */
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 283f55bbeb70..af7a30dafff3 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -19,6 +19,7 @@
> #include <linux/proc_ns.h>
> #include <linux/security.h>
> #include <linux/btf_ids.h>
> +#include <linux/buildid.h>
>
> #include "../../lib/kstrtox.h"
>
> @@ -1706,10 +1707,25 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> }
> }
>
> +int bpf_vma_build_id_parse(struct vm_area_struct *vma,
> + unsigned char *build_id,
> + size_t build_id__sz)
> +{
> + __u32 size;
> + int err;
> +
> + if (build_id__sz < BUILD_ID_SIZE_MAX)
> + return -EINVAL;
> +
> + err = build_id_parse(vma, build_id, &size);
> + return err ?: (int) size;
> +}
And you'll allow any tracing prog to call it like this?
Feels obviously unsafe unless I'm missing something big here.
See the amount of safety checks that
stack_map_get_build_id_offset() does.
Why can we get away without them here?
The use case is not clear to me as well.
Do you alwasy expect to call this kfunc from bpf_find_vma callback ?
> +
> BTF_SET8_START(tracing_btf_ids)
> #ifdef CONFIG_KEXEC_CORE
> BTF_ID_FLAGS(func, crash_kexec, KF_DESTRUCTIVE)
> #endif
> +BTF_ID_FLAGS(func, bpf_vma_build_id_parse)
> BTF_SET8_END(tracing_btf_ids)
>
> static const struct btf_kfunc_id_set tracing_kfunc_set = {
> --
> 2.38.1
>
