Message ID | 20181213121922.6652-3-quentin.monnet@netronome.com |
---|---|
State | Changes Requested, archived |
Delegated to: | BPF Maintainers |
Headers | show |
Series | tools: bpftool: add probes for system and device | expand |
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. > #define JIT_COMPILER_ENABLE JIT_COMPILER_ENABLE_OFF > #define JIT_COMPILER_ENABLE_OFF 0 > #define JIT_COMPILER_ENABLE_ON 1 > #define JIT_COMPILER_ENABLE_ON_WITH_DEBUG 2 > #define JIT_COMPILER_ENABLE_UNKNOWN -1 Same here: JIT_COMPILER_ENABLED JIT_COMPILER_ENABLED_WITH_DEBUG JIT_COMPILER_DISABLED JIT_COMPILER_UNKNOWN And so on... > #define JIT_COMPILER_HARDEN JIT_COMPILER_HARDEN_OFF > #define JIT_COMPILER_HARDEN_OFF 0 > #define JIT_COMPILER_HARDEN_FOR_UNPRIVILEGED 1 > #define JIT_COMPILER_HARDEN_FOR_ALL_USERS 2 > #define JIT_COMPILER_HARDEN_UNKNOWN -1 > #define JIT_COMPILER_KALLSYMS JIT_COMPILER_KALLSYMS_OFF > #define JIT_COMPILER_KALLSYMS_OFF 0 > #define JIT_COMPILER_KALLSYMS_FOR_ROOT 1 > #define JIT_COMPILER_KALLSYMS_UNKNOWN -1 > ... > > These probes are skipped if procfs is not mounted. > > Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com> > Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> > --- > tools/bpf/bpftool/feature.c | 271 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 271 insertions(+) > > diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c > index e1784611575d..9fa7016c7d21 100644 > --- a/tools/bpf/bpftool/feature.c > +++ b/tools/bpf/bpftool/feature.c > @@ -5,6 +5,7 @@ > #include <string.h> > #include <unistd.h> > #include <sys/utsname.h> > +#include <sys/vfs.h> > > #include <linux/filter.h> > #include <linux/limits.h> > @@ -13,11 +14,29 @@ > > #include "main.h" > > +#ifndef PROC_SUPER_MAGIC > +# define PROC_SUPER_MAGIC 0x9fa0 > +#endif > + > enum probe_component { > COMPONENT_UNSPEC, > COMPONENT_KERNEL, > }; > > +/* Miscellaneous utility functions */ > + > +static bool check_procfs(void) > +{ > + struct statfs st_fs; > + > + if (statfs("/proc", &st_fs) < 0) > + return false; > + if ((unsigned long)st_fs.f_type != PROC_SUPER_MAGIC) > + return false; > + > + return true; > +} > + > /* Printing utility functions */ > > static void > @@ -49,6 +68,236 @@ print_start_section(const char *json_title, const char *define_comment, > > /* Probing functions */ > > +static int read_procfs(const char *path) > +{ > + char *endptr, *line = NULL; > + size_t len = 0; > + FILE *fd; > + int res; > + > + fd = fopen(path, "r"); > + if (!fd) > + return -1; > + > + res = getline(&line, &len, fd); > + fclose(fd); > + if (res < 0) > + return -1; > + > + errno = 0; > + res = strtol(line, &endptr, 10); > + if (errno || *line == '\0' || *endptr != '\n') > + res = -1; > + free(line); > + > + return res; > +} > + > +static void probe_unprivileged_disabled(const char *define_prefix) > +{ > + int res; > + > + res = read_procfs("/proc/sys/kernel/unprivileged_bpf_disabled"); > + if (json_output) { > + jsonw_int_field(json_wtr, "unprivileged_bpf_disabled", res); > + } else if (define_prefix) { > + printf("#define %sUNPRIVILEGED_BPF_DISABLED ", define_prefix); > + switch (res) { > + case 0: > + printf("%sUNPRIVILEGED_BPF_DISABLED_OFF\n", > + define_prefix); > + break; > + case 1: > + printf("%sUNPRIVILEGED_BPF_DISABLED_ON\n", > + define_prefix); > + break; > + case -1: > + printf("%sUNPRIVILEGED_BPF_DISABLED_UNKNOWN\n", > + define_prefix); > + break; > + default: > + printf("%d\n", res); > + } > + printf("#define %sUNPRIVILEGED_BPF_DISABLED_OFF 0\n", > + define_prefix); > + printf("#define %sUNPRIVILEGED_BPF_DISABLED_ON 1\n", > + define_prefix); > + printf("#define %sUNPRIVILEGED_BPF_DISABLED_UNKNOWN -1\n", > + define_prefix); > + } else { > + switch (res) { > + case 0: > + printf("bpf() syscall for unprivileged users is enabled\n"); > + break; > + case 1: > + printf("bpf() syscall restricted to privileged users\n"); > + break; > + case -1: > + printf("Unable to retrieve required privileges for bpf() syscall\n"); > + break; > + default: > + printf("bpf() syscall restriction has unknown value %d\n", res); > + } > + } > +} > + > +static void probe_jit_enable(const char *define_prefix) > +{ > + int res; > + > + res = read_procfs("/proc/sys/net/core/bpf_jit_enable"); > + if (json_output) { > + jsonw_int_field(json_wtr, "bpf_jit_enable", res); > + } else if (define_prefix) { > + printf("#define %sJIT_COMPILER_ENABLE ", define_prefix); > + switch (res) { > + case 0: > + printf("%sJIT_COMPILER_ENABLE_OFF\n", define_prefix); > + break; > + case 1: > + printf("%sJIT_COMPILER_ENABLE_ON\n", define_prefix); > + break; > + case 2: > + printf("%sJIT_COMPILER_ENABLE_ON_WITH_DEBUG\n", > + define_prefix); > + break; > + case -1: > + printf("%sJIT_COMPILER_ENABLE_UNKNOWN\n", > + define_prefix); > + break; > + default: > + printf("%d\n", res); > + } > + printf("#define %sJIT_COMPILER_ENABLE_OFF 0\n", define_prefix); > + printf("#define %sJIT_COMPILER_ENABLE_ON 1\n", define_prefix); > + printf("#define %sJIT_COMPILER_ENABLE_ON_WITH_DEBUG 2\n", > + define_prefix); > + printf("#define %sJIT_COMPILER_ENABLE_UNKNOWN -1\n", > + define_prefix); > + } else { > + switch (res) { > + case 0: > + printf("JIT compiler is disabled\n"); > + break; > + case 1: > + printf("JIT compiler is enabled\n"); > + break; > + case 2: > + printf("JIT compiler is enabled with debugging traces in kernel logs\n"); > + break; > + case -1: > + printf("Unable to retrieve JIT-compiler status\n"); > + break; > + default: > + printf("JIT-compiler status has unknown value %d\n", > + res); > + } > + } > +} > + > +static void probe_jit_harden(const char *define_prefix) > +{ > + int res; > + > + res = read_procfs("/proc/sys/net/core/bpf_jit_harden"); > + if (json_output) { > + jsonw_int_field(json_wtr, "bpf_jit_harden", res); > + } else if (define_prefix) { > + printf("#define %sJIT_COMPILER_HARDEN ", define_prefix); > + switch (res) { > + case 0: > + printf("%sJIT_COMPILER_HARDEN_OFF\n", define_prefix); > + break; > + case 1: > + printf("%sJIT_COMPILER_HARDEN_FOR_UNPRIVILEGED\n", > + define_prefix); > + break; > + case 2: > + printf("%sJIT_COMPILER_HARDEN_FOR_ALL_USERS\n", > + define_prefix); > + break; > + case -1: > + printf("%sJIT_COMPILER_HARDEN_UNKNOWN\n", > + define_prefix); > + break; > + default: > + printf("%d\n", res); > + } > + printf("#define %sJIT_COMPILER_HARDEN_OFF 0\n", define_prefix); > + printf("#define %sJIT_COMPILER_HARDEN_FOR_UNPRIVILEGED 1\n", > + define_prefix); > + printf("#define %sJIT_COMPILER_HARDEN_FOR_ALL_USERS 2\n", > + define_prefix); > + printf("#define %sJIT_COMPILER_HARDEN_UNKNOWN -1\n", > + define_prefix); > + } else { > + switch (res) { > + case 0: > + printf("JIT compiler hardening is disabled\n"); > + break; > + case 1: > + printf("JIT compiler hardening is enabled for unprivileged users\n"); > + break; > + case 2: > + printf("JIT compiler hardening is enabled for all users\n"); > + break; > + case -1: > + printf("Unable to retrieve JIT hardening status\n"); > + break; > + default: > + printf("JIT hardening status has unknown value %d\n", > + res); > + } > + } > +} > + > +static void probe_jit_kallsyms(const char *define_prefix) > +{ > + int res; > + > + res = read_procfs("/proc/sys/net/core/bpf_jit_kallsyms"); > + if (json_output) { > + jsonw_int_field(json_wtr, "bpf_jit_kallsyms", res); > + } else if (define_prefix) { > + printf("#define %sJIT_COMPILER_KALLSYMS ", define_prefix); > + switch (res) { > + case 0: > + printf("%sJIT_COMPILER_KALLSYMS_OFF\n", define_prefix); > + break; > + case 1: > + printf("%sJIT_COMPILER_KALLSYMS_FOR_ROOT\n", > + define_prefix); > + break; > + case -1: > + printf("%sJIT_COMPILER_KALLSYMS_UNKNOWN\n", > + define_prefix); > + break; > + default: > + printf("%d\n", res); > + } > + printf("#define %sJIT_COMPILER_KALLSYMS_OFF 0\n", > + define_prefix); > + printf("#define %sJIT_COMPILER_KALLSYMS_FOR_ROOT 1\n", > + define_prefix); > + printf("#define %sJIT_COMPILER_KALLSYMS_UNKNOWN -1\n", > + define_prefix); > + } else { > + switch (res) { > + case 0: > + printf("JIT compiler kallsyms exports are disabled\n"); > + break; > + case 1: > + printf("JIT compiler kallsyms exports are enabled for root\n"); > + break; > + case -1: > + printf("Unable to retrieve JIT kallsyms export status\n"); > + break; > + default: > + printf("JIT kallsyms exports status has unknown value %d\n", res); > + } > + } > +} > + > static int probe_kernel_version(const char *define_prefix) > { > int version, subversion, patchlevel, code = 0; > @@ -138,6 +387,28 @@ static int do_probe(int argc, char **argv) > if (json_output) > jsonw_start_object(json_wtr); > > + switch (target) { > + case COMPONENT_KERNEL: > + case COMPONENT_UNSPEC: > + print_start_section("system_config", > + "/*** System configuration ***/", > + "Scanning system configuration...", > + define_prefix); > + if (check_procfs()) { > + probe_unprivileged_disabled(define_prefix); > + probe_jit_enable(define_prefix); > + probe_jit_harden(define_prefix); > + probe_jit_kallsyms(define_prefix); > + } else { > + p_info("/* procfs not mounted, skipping related probes */"); > + } > + if (json_output) > + jsonw_end_object(json_wtr); > + else > + printf("\n"); > + break; > + } > + > print_start_section("syscall_config", > "/*** System call and kernel version ***/", > "Scanning system call and kernel version...", > -- > 2.17.1 >
2018-12-13 18:58 UTC-0800 ~ Stanislav Fomichev <sdf@fomichev.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.
On 12/13/2018 01:19 PM, 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 > #define JIT_COMPILER_ENABLE JIT_COMPILER_ENABLE_OFF > #define JIT_COMPILER_ENABLE_OFF 0 > #define JIT_COMPILER_ENABLE_ON 1 > #define JIT_COMPILER_ENABLE_ON_WITH_DEBUG 2 > #define JIT_COMPILER_ENABLE_UNKNOWN -1 > #define JIT_COMPILER_HARDEN JIT_COMPILER_HARDEN_OFF > #define JIT_COMPILER_HARDEN_OFF 0 > #define JIT_COMPILER_HARDEN_FOR_UNPRIVILEGED 1 > #define JIT_COMPILER_HARDEN_FOR_ALL_USERS 2 > #define JIT_COMPILER_HARDEN_UNKNOWN -1 > #define JIT_COMPILER_KALLSYMS JIT_COMPILER_KALLSYMS_OFF > #define JIT_COMPILER_KALLSYMS_OFF 0 > #define JIT_COMPILER_KALLSYMS_FOR_ROOT 1 > #define JIT_COMPILER_KALLSYMS_UNKNOWN -1 > ... Hm, given these knobs may change at any point in time, what would be a use case in an application for these if they cannot be relied upon? (At least the jit_enable and jit_harden are transparent to the user.)
2018-12-15 00:40 UTC+0100 ~ Daniel Borkmann <daniel@iogearbox.net> > On 12/13/2018 01:19 PM, 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 >> #define JIT_COMPILER_ENABLE JIT_COMPILER_ENABLE_OFF >> #define JIT_COMPILER_ENABLE_OFF 0 >> #define JIT_COMPILER_ENABLE_ON 1 >> #define JIT_COMPILER_ENABLE_ON_WITH_DEBUG 2 >> #define JIT_COMPILER_ENABLE_UNKNOWN -1 >> #define JIT_COMPILER_HARDEN JIT_COMPILER_HARDEN_OFF >> #define JIT_COMPILER_HARDEN_OFF 0 >> #define JIT_COMPILER_HARDEN_FOR_UNPRIVILEGED 1 >> #define JIT_COMPILER_HARDEN_FOR_ALL_USERS 2 >> #define JIT_COMPILER_HARDEN_UNKNOWN -1 >> #define JIT_COMPILER_KALLSYMS JIT_COMPILER_KALLSYMS_OFF >> #define JIT_COMPILER_KALLSYMS_OFF 0 >> #define JIT_COMPILER_KALLSYMS_FOR_ROOT 1 >> #define JIT_COMPILER_KALLSYMS_UNKNOWN -1 >> ... > > Hm, given these knobs may change at any point in time, what would > be a use case in an application for these if they cannot be relied > upon? (At least the jit_enable and jit_harden are transparent to > the user.) > Granted, for those parameters it's a snapshot of the system at the time the probes are run. It can be useful, I suppose, if a server is not expected to change them often... And the plain output might be useful to a sysadmin who wants to have a quick look at BPF-related parameters, maybe?
On 12/15/2018 04:31 AM, Quentin Monnet wrote: > 2018-12-15 00:40 UTC+0100 ~ Daniel Borkmann <daniel@iogearbox.net> >> On 12/13/2018 01:19 PM, 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 >>> #define JIT_COMPILER_ENABLE JIT_COMPILER_ENABLE_OFF >>> #define JIT_COMPILER_ENABLE_OFF 0 >>> #define JIT_COMPILER_ENABLE_ON 1 >>> #define JIT_COMPILER_ENABLE_ON_WITH_DEBUG 2 >>> #define JIT_COMPILER_ENABLE_UNKNOWN -1 >>> #define JIT_COMPILER_HARDEN JIT_COMPILER_HARDEN_OFF >>> #define JIT_COMPILER_HARDEN_OFF 0 >>> #define JIT_COMPILER_HARDEN_FOR_UNPRIVILEGED 1 >>> #define JIT_COMPILER_HARDEN_FOR_ALL_USERS 2 >>> #define JIT_COMPILER_HARDEN_UNKNOWN -1 >>> #define JIT_COMPILER_KALLSYMS JIT_COMPILER_KALLSYMS_OFF >>> #define JIT_COMPILER_KALLSYMS_OFF 0 >>> #define JIT_COMPILER_KALLSYMS_FOR_ROOT 1 >>> #define JIT_COMPILER_KALLSYMS_UNKNOWN -1 >>> ... >> >> Hm, given these knobs may change at any point in time, what would >> be a use case in an application for these if they cannot be relied >> upon? (At least the jit_enable and jit_harden are transparent to >> the user.) > > Granted, for those parameters it's a snapshot of the system at the time > the probes are run. It can be useful, I suppose, if a server is not > expected to change them often... And the plain output might be useful to > a sysadmin who wants to have a quick look at BPF-related parameters, maybe? Hmm, but wouldn't the main purpose of this header file be to include it into a BPF program to selectively enable / disable features (e.g. LPM map vs hashtab when kernel does not support LPM type as one example)? What would a use-case be for the above defines used inside such BPF prog? (Similarly for the kernel config defines in the other patch, how would a BPF prog use them?) I think perhaps the 'issue' is that the C-style header generation and json dump are dumping the /exact/ same information. Is this a requirement? Wouldn't it be better to evolve the two /independently/? E.g. the system_config bits from the json dump and BPF-related kernel config, perhaps also a listing of available maps, progs with supported helpers for a prog would be useful for the json dump for an admin or orchestration daemon to adapt to the underlying kernel where it could just parse the json and doesn't have to do the queries by itself. But for the header generation, I would only place defines in there that are strictly relevant for the BPF program author. Available maps, progs and helpers is a good start there, later we could also put others in there such as [0] and similar specifics or quirks to verifier behavior that would be relevant in terms of work-arounds for supporting different kernel versions; but on a case by case basis. There things might potentially be less interesting for a json dump (though the json dump could overall be a superset of the info from the header file). [0] https://github.com/cilium/cilium/blob/master/bpf/probes/raw_mark_map_val.t
2018-12-16 01:14 UTC+0100 ~ Daniel Borkmann <daniel@iogearbox.net> > On 12/15/2018 04:31 AM, Quentin Monnet wrote: >> 2018-12-15 00:40 UTC+0100 ~ Daniel Borkmann <daniel@iogearbox.net> >>> On 12/13/2018 01:19 PM, 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 >>>> #define JIT_COMPILER_ENABLE JIT_COMPILER_ENABLE_OFF >>>> #define JIT_COMPILER_ENABLE_OFF 0 >>>> #define JIT_COMPILER_ENABLE_ON 1 >>>> #define JIT_COMPILER_ENABLE_ON_WITH_DEBUG 2 >>>> #define JIT_COMPILER_ENABLE_UNKNOWN -1 >>>> #define JIT_COMPILER_HARDEN JIT_COMPILER_HARDEN_OFF >>>> #define JIT_COMPILER_HARDEN_OFF 0 >>>> #define JIT_COMPILER_HARDEN_FOR_UNPRIVILEGED 1 >>>> #define JIT_COMPILER_HARDEN_FOR_ALL_USERS 2 >>>> #define JIT_COMPILER_HARDEN_UNKNOWN -1 >>>> #define JIT_COMPILER_KALLSYMS JIT_COMPILER_KALLSYMS_OFF >>>> #define JIT_COMPILER_KALLSYMS_OFF 0 >>>> #define JIT_COMPILER_KALLSYMS_FOR_ROOT 1 >>>> #define JIT_COMPILER_KALLSYMS_UNKNOWN -1 >>>> ... >>> >>> Hm, given these knobs may change at any point in time, what would >>> be a use case in an application for these if they cannot be relied >>> upon? (At least the jit_enable and jit_harden are transparent to >>> the user.) >> >> Granted, for those parameters it's a snapshot of the system at the time >> the probes are run. It can be useful, I suppose, if a server is not >> expected to change them often... And the plain output might be useful to >> a sysadmin who wants to have a quick look at BPF-related parameters, maybe? > > Hmm, but wouldn't the main purpose of this header file be to include it > into a BPF program to selectively enable / disable features (e.g. LPM > map vs hashtab when kernel does not support LPM type as one example)? > What would a use-case be for the above defines used inside such BPF prog? > (Similarly for the kernel config defines in the other patch, how would > a BPF prog use them?) > > I think perhaps the 'issue' is that the C-style header generation and json > dump are dumping the /exact/ same information. Is this a requirement? > Wouldn't it be better to evolve the two /independently/? > > E.g. the system_config bits from the json dump and BPF-related kernel > config, perhaps also a listing of available maps, progs with supported > helpers for a prog would be useful for the json dump for an admin or > orchestration daemon to adapt to the underlying kernel where it could > just parse the json and doesn't have to do the queries by itself. > > But for the header generation, I would only place defines in there that > are strictly relevant for the BPF program author. Available maps, progs > and helpers is a good start there, later we could also put others in there > such as [0] and similar specifics or quirks to verifier behavior that > would be relevant in terms of work-arounds for supporting different kernel > versions; but on a case by case basis. There things might potentially be > less interesting for a json dump (though the json dump could overall be > a superset of the info from the header file). > > [0] https://github.com/cilium/cilium/blob/master/bpf/probes/raw_mark_map_val.t > For the use case about kernel config options, I was thinking about Cilium which collects some of them as well [0], but maybe it's not worth having it in the C-style header for now. For procfs parameters, maybe it's not so relevant indeed to have them at all in this output. So you have a point, I suppose. I do not have any hard requirement about having the #define and the JSON similar; I argued with Stanislav that I didn't want to introduce small losses of information between the two, but if we consider them entirely different from the start it is not the same thing... So maybe I should just stick to the basics for the #define output, as you suggest. I've seen the other probes used by Cilium, but I intentionally left the most specific one aside for now, there's enough to do with the current probes :). But yeah, it would make sense to have them added in the future. And for the record, I like the idea of keeping JSON a superset of the available information indeed. I'll go with just prog/map types and helpers for the #define in my next version. This should also settle the discussion on the format of the macros used in this first version for the procfs parameters. Thanks! Quentin [0] https://github.com/cilium/cilium/blob/master/bpf/run_probes.sh#L37
On 12/17/2018 11:44 AM, Quentin Monnet wrote: > 2018-12-16 01:14 UTC+0100 ~ Daniel Borkmann <daniel@iogearbox.net> >> On 12/15/2018 04:31 AM, Quentin Monnet wrote: >>> 2018-12-15 00:40 UTC+0100 ~ Daniel Borkmann <daniel@iogearbox.net> >>>> On 12/13/2018 01:19 PM, 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 >>>>> #define JIT_COMPILER_ENABLE JIT_COMPILER_ENABLE_OFF >>>>> #define JIT_COMPILER_ENABLE_OFF 0 >>>>> #define JIT_COMPILER_ENABLE_ON 1 >>>>> #define JIT_COMPILER_ENABLE_ON_WITH_DEBUG 2 >>>>> #define JIT_COMPILER_ENABLE_UNKNOWN -1 >>>>> #define JIT_COMPILER_HARDEN JIT_COMPILER_HARDEN_OFF >>>>> #define JIT_COMPILER_HARDEN_OFF 0 >>>>> #define JIT_COMPILER_HARDEN_FOR_UNPRIVILEGED 1 >>>>> #define JIT_COMPILER_HARDEN_FOR_ALL_USERS 2 >>>>> #define JIT_COMPILER_HARDEN_UNKNOWN -1 >>>>> #define JIT_COMPILER_KALLSYMS JIT_COMPILER_KALLSYMS_OFF >>>>> #define JIT_COMPILER_KALLSYMS_OFF 0 >>>>> #define JIT_COMPILER_KALLSYMS_FOR_ROOT 1 >>>>> #define JIT_COMPILER_KALLSYMS_UNKNOWN -1 >>>>> ... >>>> >>>> Hm, given these knobs may change at any point in time, what would >>>> be a use case in an application for these if they cannot be relied >>>> upon? (At least the jit_enable and jit_harden are transparent to >>>> the user.) >>> >>> Granted, for those parameters it's a snapshot of the system at the time >>> the probes are run. It can be useful, I suppose, if a server is not >>> expected to change them often... And the plain output might be useful to >>> a sysadmin who wants to have a quick look at BPF-related parameters, maybe? >> >> Hmm, but wouldn't the main purpose of this header file be to include it >> into a BPF program to selectively enable / disable features (e.g. LPM >> map vs hashtab when kernel does not support LPM type as one example)? >> What would a use-case be for the above defines used inside such BPF prog? >> (Similarly for the kernel config defines in the other patch, how would >> a BPF prog use them?) >> >> I think perhaps the 'issue' is that the C-style header generation and json >> dump are dumping the /exact/ same information. Is this a requirement? >> Wouldn't it be better to evolve the two /independently/? >> >> E.g. the system_config bits from the json dump and BPF-related kernel >> config, perhaps also a listing of available maps, progs with supported >> helpers for a prog would be useful for the json dump for an admin or >> orchestration daemon to adapt to the underlying kernel where it could >> just parse the json and doesn't have to do the queries by itself. >> >> But for the header generation, I would only place defines in there that >> are strictly relevant for the BPF program author. Available maps, progs >> and helpers is a good start there, later we could also put others in there >> such as [0] and similar specifics or quirks to verifier behavior that >> would be relevant in terms of work-arounds for supporting different kernel >> versions; but on a case by case basis. There things might potentially be >> less interesting for a json dump (though the json dump could overall be >> a superset of the info from the header file). >> >> [0] https://github.com/cilium/cilium/blob/master/bpf/probes/raw_mark_map_val.t > > For the use case about kernel config options, I was thinking about Cilium which collects some of them as well [0], but maybe it's not worth having it in the C-style header for now. For procfs parameters, maybe it's not so relevant indeed to have them at all in this output. > > So you have a point, I suppose. I do not have any hard requirement about having the #define and the JSON similar; I argued with Stanislav that I didn't want to introduce small losses of information between the two, but if we consider them entirely different from the start it is not the same thing... So maybe I should just stick to the basics for the #define output, as you suggest. > > I've seen the other probes used by Cilium, but I intentionally left the most specific one aside for now, there's enough to do with the current probes :). But yeah, it would make sense to have them added in the future. And for the record, I like the idea of keeping JSON a superset of the available information indeed. Sounds good to me, I also think it's better to keep the json output a superset; definitely allows for more flexibility and if there's a real world use case to have some of that additional information also in the header output as defines, we can add that at a later point in time. > I'll go with just prog/map types and helpers for the #define in my next version. This should also settle the discussion on the format of the macros used in this first version for the procfs parameters. Yes makes sense, this would be great as a start, and we can extend it as needed. Thanks, Daniel
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c index e1784611575d..9fa7016c7d21 100644 --- a/tools/bpf/bpftool/feature.c +++ b/tools/bpf/bpftool/feature.c @@ -5,6 +5,7 @@ #include <string.h> #include <unistd.h> #include <sys/utsname.h> +#include <sys/vfs.h> #include <linux/filter.h> #include <linux/limits.h> @@ -13,11 +14,29 @@ #include "main.h" +#ifndef PROC_SUPER_MAGIC +# define PROC_SUPER_MAGIC 0x9fa0 +#endif + enum probe_component { COMPONENT_UNSPEC, COMPONENT_KERNEL, }; +/* Miscellaneous utility functions */ + +static bool check_procfs(void) +{ + struct statfs st_fs; + + if (statfs("/proc", &st_fs) < 0) + return false; + if ((unsigned long)st_fs.f_type != PROC_SUPER_MAGIC) + return false; + + return true; +} + /* Printing utility functions */ static void @@ -49,6 +68,236 @@ print_start_section(const char *json_title, const char *define_comment, /* Probing functions */ +static int read_procfs(const char *path) +{ + char *endptr, *line = NULL; + size_t len = 0; + FILE *fd; + int res; + + fd = fopen(path, "r"); + if (!fd) + return -1; + + res = getline(&line, &len, fd); + fclose(fd); + if (res < 0) + return -1; + + errno = 0; + res = strtol(line, &endptr, 10); + if (errno || *line == '\0' || *endptr != '\n') + res = -1; + free(line); + + return res; +} + +static void probe_unprivileged_disabled(const char *define_prefix) +{ + int res; + + res = read_procfs("/proc/sys/kernel/unprivileged_bpf_disabled"); + if (json_output) { + jsonw_int_field(json_wtr, "unprivileged_bpf_disabled", res); + } else if (define_prefix) { + printf("#define %sUNPRIVILEGED_BPF_DISABLED ", define_prefix); + switch (res) { + case 0: + printf("%sUNPRIVILEGED_BPF_DISABLED_OFF\n", + define_prefix); + break; + case 1: + printf("%sUNPRIVILEGED_BPF_DISABLED_ON\n", + define_prefix); + break; + case -1: + printf("%sUNPRIVILEGED_BPF_DISABLED_UNKNOWN\n", + define_prefix); + break; + default: + printf("%d\n", res); + } + printf("#define %sUNPRIVILEGED_BPF_DISABLED_OFF 0\n", + define_prefix); + printf("#define %sUNPRIVILEGED_BPF_DISABLED_ON 1\n", + define_prefix); + printf("#define %sUNPRIVILEGED_BPF_DISABLED_UNKNOWN -1\n", + define_prefix); + } else { + switch (res) { + case 0: + printf("bpf() syscall for unprivileged users is enabled\n"); + break; + case 1: + printf("bpf() syscall restricted to privileged users\n"); + break; + case -1: + printf("Unable to retrieve required privileges for bpf() syscall\n"); + break; + default: + printf("bpf() syscall restriction has unknown value %d\n", res); + } + } +} + +static void probe_jit_enable(const char *define_prefix) +{ + int res; + + res = read_procfs("/proc/sys/net/core/bpf_jit_enable"); + if (json_output) { + jsonw_int_field(json_wtr, "bpf_jit_enable", res); + } else if (define_prefix) { + printf("#define %sJIT_COMPILER_ENABLE ", define_prefix); + switch (res) { + case 0: + printf("%sJIT_COMPILER_ENABLE_OFF\n", define_prefix); + break; + case 1: + printf("%sJIT_COMPILER_ENABLE_ON\n", define_prefix); + break; + case 2: + printf("%sJIT_COMPILER_ENABLE_ON_WITH_DEBUG\n", + define_prefix); + break; + case -1: + printf("%sJIT_COMPILER_ENABLE_UNKNOWN\n", + define_prefix); + break; + default: + printf("%d\n", res); + } + printf("#define %sJIT_COMPILER_ENABLE_OFF 0\n", define_prefix); + printf("#define %sJIT_COMPILER_ENABLE_ON 1\n", define_prefix); + printf("#define %sJIT_COMPILER_ENABLE_ON_WITH_DEBUG 2\n", + define_prefix); + printf("#define %sJIT_COMPILER_ENABLE_UNKNOWN -1\n", + define_prefix); + } else { + switch (res) { + case 0: + printf("JIT compiler is disabled\n"); + break; + case 1: + printf("JIT compiler is enabled\n"); + break; + case 2: + printf("JIT compiler is enabled with debugging traces in kernel logs\n"); + break; + case -1: + printf("Unable to retrieve JIT-compiler status\n"); + break; + default: + printf("JIT-compiler status has unknown value %d\n", + res); + } + } +} + +static void probe_jit_harden(const char *define_prefix) +{ + int res; + + res = read_procfs("/proc/sys/net/core/bpf_jit_harden"); + if (json_output) { + jsonw_int_field(json_wtr, "bpf_jit_harden", res); + } else if (define_prefix) { + printf("#define %sJIT_COMPILER_HARDEN ", define_prefix); + switch (res) { + case 0: + printf("%sJIT_COMPILER_HARDEN_OFF\n", define_prefix); + break; + case 1: + printf("%sJIT_COMPILER_HARDEN_FOR_UNPRIVILEGED\n", + define_prefix); + break; + case 2: + printf("%sJIT_COMPILER_HARDEN_FOR_ALL_USERS\n", + define_prefix); + break; + case -1: + printf("%sJIT_COMPILER_HARDEN_UNKNOWN\n", + define_prefix); + break; + default: + printf("%d\n", res); + } + printf("#define %sJIT_COMPILER_HARDEN_OFF 0\n", define_prefix); + printf("#define %sJIT_COMPILER_HARDEN_FOR_UNPRIVILEGED 1\n", + define_prefix); + printf("#define %sJIT_COMPILER_HARDEN_FOR_ALL_USERS 2\n", + define_prefix); + printf("#define %sJIT_COMPILER_HARDEN_UNKNOWN -1\n", + define_prefix); + } else { + switch (res) { + case 0: + printf("JIT compiler hardening is disabled\n"); + break; + case 1: + printf("JIT compiler hardening is enabled for unprivileged users\n"); + break; + case 2: + printf("JIT compiler hardening is enabled for all users\n"); + break; + case -1: + printf("Unable to retrieve JIT hardening status\n"); + break; + default: + printf("JIT hardening status has unknown value %d\n", + res); + } + } +} + +static void probe_jit_kallsyms(const char *define_prefix) +{ + int res; + + res = read_procfs("/proc/sys/net/core/bpf_jit_kallsyms"); + if (json_output) { + jsonw_int_field(json_wtr, "bpf_jit_kallsyms", res); + } else if (define_prefix) { + printf("#define %sJIT_COMPILER_KALLSYMS ", define_prefix); + switch (res) { + case 0: + printf("%sJIT_COMPILER_KALLSYMS_OFF\n", define_prefix); + break; + case 1: + printf("%sJIT_COMPILER_KALLSYMS_FOR_ROOT\n", + define_prefix); + break; + case -1: + printf("%sJIT_COMPILER_KALLSYMS_UNKNOWN\n", + define_prefix); + break; + default: + printf("%d\n", res); + } + printf("#define %sJIT_COMPILER_KALLSYMS_OFF 0\n", + define_prefix); + printf("#define %sJIT_COMPILER_KALLSYMS_FOR_ROOT 1\n", + define_prefix); + printf("#define %sJIT_COMPILER_KALLSYMS_UNKNOWN -1\n", + define_prefix); + } else { + switch (res) { + case 0: + printf("JIT compiler kallsyms exports are disabled\n"); + break; + case 1: + printf("JIT compiler kallsyms exports are enabled for root\n"); + break; + case -1: + printf("Unable to retrieve JIT kallsyms export status\n"); + break; + default: + printf("JIT kallsyms exports status has unknown value %d\n", res); + } + } +} + static int probe_kernel_version(const char *define_prefix) { int version, subversion, patchlevel, code = 0; @@ -138,6 +387,28 @@ static int do_probe(int argc, char **argv) if (json_output) jsonw_start_object(json_wtr); + switch (target) { + case COMPONENT_KERNEL: + case COMPONENT_UNSPEC: + print_start_section("system_config", + "/*** System configuration ***/", + "Scanning system configuration...", + define_prefix); + if (check_procfs()) { + probe_unprivileged_disabled(define_prefix); + probe_jit_enable(define_prefix); + probe_jit_harden(define_prefix); + probe_jit_kallsyms(define_prefix); + } else { + p_info("/* procfs not mounted, skipping related probes */"); + } + if (json_output) + jsonw_end_object(json_wtr); + else + printf("\n"); + break; + } + print_start_section("syscall_config", "/*** System call and kernel version ***/", "Scanning system call and kernel version...",