netdev - Re: [PATCH bpf-next 2/8] tools: bpftool: add probes for /proc/ eBPF parameters

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]

Date:   Fri, 14 Dec 2018 11:27:55 +0000
From:   Quentin Monnet <quentin.monnet@...ronome.com>
To:     Stanislav Fomichev <sdf@...ichev.me>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        oss-drivers@...ronome.com,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Stanislav Fomichev <sdf@...gle.com>
Subject: Re: [PATCH bpf-next 2/8] tools: bpftool: add probes for /proc/ eBPF
 parameters

2018-12-13 18:58 UTC-0800 ~ Stanislav Fomichev <sdf@...ichev.me>
> On 12/13, Quentin Monnet wrote:
>> Add a set of probes to dump the eBPF-related parameters available from
>> /proc/: availability of bpf() syscall for unprivileged users,
>> JIT compiler status and hardening status, kallsyms exports status.
>>
>> Sample output:
>>
>>     # bpftool feature probe kernel
>>     Scanning system configuration...
>>     bpf() syscall for unprivileged users is enabled
>>     JIT compiler is disabled
>>     JIT compiler hardening is disabled
>>     JIT compiler kallsyms exports are disabled
>>     ...
>>
>>     # bpftool --json --pretty feature probe kernel
>>     {
>>         "system_config": {
>>             "unprivileged_bpf_disabled": 0,
>>             "bpf_jit_enable": 0,
>>             "bpf_jit_harden": 0,
>>             "bpf_jit_kallsyms": 0
>>         },
>>         ...
>>     }
>>
> 
> [..]
> 
>>     # bpftool feature probe kernel macros prefix BPFTOOL_
>>     #define UNPRIVILEGED_BPF_DISABLED UNPRIVILEGED_BPF_DISABLED_OFF
>>     #define  UNPRIVILEGED_BPF_DISABLED_OFF 0
>>     #define  UNPRIVILEGED_BPF_DISABLED_ON 1
>>     #define  UNPRIVILEGED_BPF_DISABLED_UNKNOWN -1
> This looks a bit complicated. For example, why not simply define:
> 
> #define UNPRIVILEGED_BPF_DISABLED 1 /* when it's explicitly disabled */
> #define UNPRIVILEGED_BPF_ENABLED  1 /* when it's explicitly enabled */
> #define UNPRIVILEGED_BPF_UNKNOWN  1 /* when unknown - maybe even skip
> this altogether and treat unknown == disabled (worst case) */
> 
> Then, I, as a potential user, can do:
> 
> #if defined(UNPRIVILEGED_BPF_ENABLED)
> /* do something useful */
> #elif defined(UNPRIVILEGED_BPF_DISABLED)
> /* print an error asking to use root */
> #else
> /* try anyway, fallback to error ? */
> #endif
> 
> IMO, if don't want to do stuff like:
> 
> #if UNPRIVILEGED_BPF_DISABLED == UNPRIVILEGED_BPF_DISABLED_OFF
> #elif UNPRIVILEGED_BPF_DISABLED == UNPRIVILEGED_BPF_DISABLED_ON
> #else
> #endif
> 
> I live in my mental model if ifdefs, not complicated cpp #if
> comparisons.
> 
> Just a suggestion, I feel like we can keep it simple.

It's true that it make things look simpler. We loose the direct
correspondence between macro name and procfs file name, but I suppose we
can live with it.

I'm more concerned about the loss of information for the "unknown" case,
however. If we successfully retrieved a value from the procfs, I find it
harsh not to give it to the user, even if we cannot interpret it :/. And
I don't see a way of doing that without having a UNPRIVILEGED_BPF_VALUE
macro of some kind.