> On Nov 18, 2022, at 3:45 PM, Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote:
>
> 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?
I guess that won't work, as the vma is not refcounted. :( This is
why we have to hold mmap_lock when calling task_vma programs.
OTOH, I would image bpf_vma_build_id_parse being quite useful for
task_vma programs.
Thanks,
Song
