netdev - Re: [PATCH bpf-next 04/10] libbpf: auto-detect btf_id of raw

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: <0ea6bf09-12ca-e23a-0535-abc62ee7e914@fb.com>
Date:   Wed, 9 Oct 2019 02:26:58 +0000
From:   Alexei Starovoitov <ast@...com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Alexei Starovoitov <ast@...nel.org>
CC:     "David S. Miller" <davem@...emloft.net>,
        Daniel Borkmann <daniel@...earbox.net>,
        "x86@...nel.org" <x86@...nel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH bpf-next 04/10] libbpf: auto-detect btf_id of
 raw_tracepoint

On 10/7/19 4:41 PM, Andrii Nakryiko wrote:
> On Fri, Oct 4, 2019 at 10:04 PM Alexei Starovoitov <ast@...nel.org> wrote:
>>
>> For raw tracepoint program types libbpf will try to find
>> btf_id of raw tracepoint in vmlinux's BTF.
>> It's a responsiblity of bpf program author to annotate the program
>> with SEC("raw_tracepoint/name") where "name" is a valid raw tracepoint.
> 
> As an aside, I've been thinking about allowing to specify "raw_tp/"
> and "tp/" in section name as an "alias" for "raw_tracepoint/" and
> "tracepoint/", respectively. Any objections?

make sense.

>> If "name" is indeed a valid raw tracepoint then in-kernel BTF
>> will have "btf_trace_##name" typedef that points to function
>> prototype of that raw tracepoint. BTF description captures
>> exact argument the kernel C code is passing into raw tracepoint.
>> The kernel verifier will check the types while loading bpf program.
>>
>> Signed-off-by: Alexei Starovoitov <ast@...nel.org>
>> ---
>>   tools/lib/bpf/libbpf.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index e0276520171b..0e6f7b41c521 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -4591,6 +4591,22 @@ int libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
>>                          continue;
>>                  *prog_type = section_names[i].prog_type;
>>                  *expected_attach_type = section_names[i].expected_attach_type;
>> +               if (*prog_type == BPF_PROG_TYPE_RAW_TRACEPOINT) {
>> +                       struct btf *btf = bpf_core_find_kernel_btf();
>> +                       char raw_tp_btf_name[128] = "btf_trace_";
>> +                       int ret;
>> +
>> +                       if (IS_ERR(btf))
>> +                               /* lack of kernel BTF is not a failure */
>> +                               return 0;
>> +                       /* append "btf_trace_" prefix per kernel convention */
>> +                       strcpy(raw_tp_btf_name + sizeof("btf_trace_") - 1,
>> +                              name + section_names[i].len);
> 
> buffer overflow here? use strncat() instead?

128 is ksym_name and due to tp construction with other prefixes,
I think, it cannot overflow, but that's a good nit. Fixed it.