| 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
| ||
|
Date: Thu, 13 Dec 2018 18:58:56 -0800
From: Stanislav Fomichev <sdf@...ichev.me>
To: Quentin Monnet <quentin.monnet@...ronome.com>
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
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@...ronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@...ronome.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
>
Powered by blists - more mailing lists