On Fri, Nov 18, 2022 at 7:40 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 | 4 ++++
> kernel/bpf/verifier.c | 26 ++++++++++++++++++++++++++
> kernel/trace/bpf_trace.c | 31 +++++++++++++++++++++++++++++++
> 3 files changed, 61 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 8b32376ce746..7648188faa2c 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2805,4 +2805,8 @@ static inline bool type_is_alloc(u32 type)
> return type & MEM_ALLOC;
> }
>
> +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/verifier.c b/kernel/bpf/verifier.c
> index 195d24316750..e20bad754a3a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -8746,6 +8746,29 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> return 0;
> }
>
> +BTF_ID_LIST_SINGLE(bpf_vma_build_id_parse_id, func, bpf_vma_build_id_parse)
> +
> +static int check_kfunc_caller(struct bpf_verifier_env *env, u32 func_id)
> +{
> + struct bpf_func_state *cur;
> + struct bpf_insn *insn;
> +
> + /* Allow bpf_vma_build_id_parse only from bpf_find_vma callback */
> + if (func_id == bpf_vma_build_id_parse_id[0]) {
> + cur = env->cur_state->frame[env->cur_state->curframe];
> + if (cur->callsite != BPF_MAIN_FUNC) {
> + insn = &env->prog->insnsi[cur->callsite];
> + if (insn->imm == BPF_FUNC_find_vma)
> + return 0;
> + }
> + verbose(env, "calling bpf_vma_build_id_parse outside bpf_find_vma "
> + "callback is not allowed\n");
> + return -1;
> + }
> +
> + return 0;
> +}
I understand that calling bpf_vma_build_id_parse from find_vma
is your only use case, but put yourself in the maintainer's shoes.
We just did an arbitrary restriction and helped a single user.
How are we going to explain this to other users?
Let's figure out a more generic way where this call is safe.
Have you looked at PTR_TRUSTED approach that David is doing
for task_struct ? Can something like this be used here?
