netdev - Re: [PATCH bpf-next 5/6] libbpf: support libbpf-provided extern variables

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: <20191121001811.eyksi2acyhvy4skr@ast-mbp.dhcp.thefacebook.com>
Date:   Wed, 20 Nov 2019 16:18:13 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Alexei Starovoitov <ast@...com>, Andrii Nakryiko <andriin@...com>,
        bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH bpf-next 5/6] libbpf: support libbpf-provided extern
 variables

On Wed, Nov 20, 2019 at 02:47:58PM -0800, Andrii Nakryiko wrote:
> 
> Given all this, I think realistically we can pick few combinations:
> 
> 1. Only support int/hex as uint64_t. Anything y/n/m or a string will
> fail in runtime.
> Pros:
>   - we get at least something to use in practice (LINUX_KERNEL_VERSION
> and int CONFIG_XXXs).
> Cons:
>   - undefined weak extern will have to be assumed either uint64_t 0
> (and succeed) or undefined string (and fail). No clearly best behavior
> here.

what that uint64 going to be when config_ is defined?
If your answer is 1 than it's not extensible.

>   - no ability to do "feature detection" logic in BPF program.
> 2. Stick to uint64_t for bool/tristate/int/hex. Don't support strings
> yet. Improve in backwards compatible way once we get BTF type info for
> externs (adding strings at that time).
> Pros:
>   - well-defined and logical behavior
>   - easily extensible once we get BTF for externs. No new flags, no
> changes in behavior.

extensible with new flag == not extensible.
The choices for bpf program that we pick for extern must keep working
when llvm starts generating BTF for externs.

> My preferences would be 2, if not, then 1.

I'm proposing something else.
I see libbpf as a tool to pass /boot/config.gz into bpf program. From program
pov CONFIG_FOO is a label. It's not a variable of type u8 or type u64.
Example:
CONFIG_A=100
CONFIG_B=y
CONFIG_C="abcd"
will be a map of one element with value:
char ar[]= { 0x64, 0, 0, 0, 'y', 'a', 'b', 'c', 'd', 0};

CONFIG_A = &ar[0];
CONFIG_B = &ar[4];
CONFIG_C = &ar[5];

libbpf parses config.gz and converts all int/hex into 4 byte or 8 byte
integers depending on number of text digits it sees in config.gz
with alignment. All other strings and characters are passed as-is.
If program says
extern u8 CONFIG_A, CONFIG_B, CONFIG_C;
It will read 1st byte from these three labels.
Later when llvm emits BTF those u8 swill stay as-is and will read the same
things. With BTF if program says 'extern _Bool CONFIG_B;' then it will be an
explicit direction for libbpf to convert that CONFIG_B value into _Bool at
program load time and represent it as sizeof(_Bool) in map element. If program
says 'extern uint32_t CONFIG_C;' the libbpf will keep first 4 bytes of that
string in there. If program says 'extern uint64_t CONFIG_C;' the libbpf will
keep first 4 character plus one byte for zero plus 3 bytes of padding in map
element.
'extern char CONFIG_C[5];' is also ok. Either with or without BTF.
In both cases it will be 5 bytes in map element.
Without BTF doing 'extern char *CONFIG_C;' will read garbage 8 bytes from that
label. With BTF 'extern char *CONFIG_C;' will be converted to pointer to inner
bytes of map element.
In other words BTF types will be directives of how libbpf should convert
strings in text file to be consumed by bpf program. Since we don't have types
now int/hex are the only ones that libbpf will convert unconditionally. The
logic of parsing config.gz is generic. It can parse any file with 'var=value'
lines this way. All names will be preserved.
In that sense LINUX_KERNEL_VERSION is a text file that has one line:
LINUX_KERNEL_VESRION=1234
If digits fit into u32 it should be u32.
This way users can feed any configuration.txt file into libbpf and into their
programs. Without BTF the map element is a collection of raw bytes and the
program needs to be smart about reading them with correct sizes. With BTF
libbpf will be converting .txt into requested types and failing to load
if libbpf cannot convert string from text into requested type.