netdev - Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC

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: <20181107214922.xjqcacj5rc5hmepw@ast-mbp.dhcp.thefacebook.com>
Date:   Wed, 7 Nov 2018 13:49:24 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Edward Cree <ecree@...arflare.com>
Cc:     Martin Lau <kafai@...com>, Yonghong Song <yhs@...com>,
        Alexei Starovoitov <ast@...com>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and
 BTF_KIND_FUNC_PROTO

On Wed, Nov 07, 2018 at 07:29:31PM +0000, Edward Cree wrote:
> Whereas I don't, and I don't feel like my core criticisms have
>  been addressed _at all_.  The only answer I get to "BTF should
>  store type and instance information in separate records" is
>  "it's a debuginfo",
...
>  I am just trying to organise
>  BTF to consist of separate _parts_ for types and instances,
>  rather than forcing both into the same Procrustean bed.

BTF does not have and should not have instances.
BTF is debug info only.
This is not negotiable.

Loading BTF into the kernel does not create an instance of anything.
The kernel verifies BTF and BTF blob sits there waiting to be pointed at
by the actual instance.
Today these instances are maps and programs.
A single program can have multiple instances of the functions
which debug info is described in BTF.
Multiple prograrms (with multiple functions) can point
to the same BTF blob description.
Consider that in single C file there will be multliple functions,
many maps, global variables (in the future) they will be created
by individual prog_load/map_create syscalls.
Before that the _single_ blob of BTF data for the whole C file
will be loaded, since functions, maps share most of
the type and name information.

In elf files the instances of functions have names.
These names go into symbol table of the elf. Dynamic linker
is using these symbols to connect different instances.
In BPF world we use IDs to connect instances.
There is btd if, map id, prog id.
You can argue that bpf_prog_load has prog_name attribute.
So the instance of the program sort of have name already,
but this name is for debug only. It doesn't have to be unique
and the kernel is not relying on the name to connect instances.

Example:
1: int foo(struct __sk_buff *skb, char arg2)
2: {
3:  return skb->len;
4: }

Line 1 goes into BTF to describe the function (with names and types).
Line 1 also goes into line_info .BTF.ext elf section as a string.
Line 3 and 4 go into line_info as well as strings.

llvm compiles it into something:
  .text
  .globl  foo
  .type   foo, @function
foo:
  r0 = *(u32 *)(r1 + 0)
  exit
  .section .BTF
  .byte X
  .byte Y
  .section .BTF.ext
  .byte X
  .byte Y

That 'foo' name goes into symbol table and together with the body
form the future instance of the function.
libbpf takes 'foo' from symbol table of elf and passes it
as bpf_attr.prog_name to the kernel along with bpf instructions
to create an instance.
The name 'foo' in symbol table doesn't have to match with
BTF 'foo' name and '.*foo.*' substring in line_info.

If you decide to use the sequence:
"""
  .globl  foo
  .type   foo, @function
foo:
""
in your ebpf_asm to parse and store into BTF as a function name
I would strongly argue against, since it would be incorrectly
conflating the instance of the function with debug purpose
of BTF.

Right now we do not have a concrete proposal for assembler text of BTF.
When LLVM emits BTF into .s it emits them as raw bytes.
So I'm looking forward to your ideas how to describe BTF in .s
Note such .s must have freedom to describe 'int bar(struct __sk_buff *a1, char a2)'
as debug info while having '.globl foo; foo:' as symbol name.

Your other 'criticism' was about libbpf's bpf_map_find_btf_info()
and ____btf_map_* hack. Yes. It is a hack and I'm open to change it
if there are better suggestions. It's a convention between
libbpf and program writers that produce elf. It's not a kernel abi.
Nothing to do with BTF and this instance vs debug info discussion.

> (I don't feel like we're making progress in understanding one
>  another here; maybe we should have a telephone discussion?
>  Sadly I'm not going to Plumbers, else that would be the
>  perfect place to thrash this out.)

Happy to jump on the call to explain it again.
10:30am pacific time works for me tomorrow.