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: <20181220174537.GM20955@mini-arch.hsd1.ca.comcast.net>
Date:   Thu, 20 Dec 2018 09:45:37 -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: [RFC PATCH bpf-next v2 4/9] tools: bpftool: add probes for eBPF
 program types

On 12/20, Quentin Monnet wrote:
> Introduce probes for supported BPF program types in libbpf, and call it
> from bpftool to test what types are available on the system. The probe
> simply consists in loading a very simple program of that type and see if
> the verifier complains or not.
> 
> Sample output:
> 
>     # bpftool feature probe kernel
>     ...
>     Scanning eBPF program types...
>     eBPF program_type socket_filter is available
>     eBPF program_type kprobe is available
>     eBPF program_type sched_cls is available
>     ...
> 
>     # bpftool --json --pretty feature probe kernel
>     {
>         ...
>         "program_types": {
>             "have_socket_filter_prog_type": true,
>             "have_kprobe_prog_type": true,
>             "have_sched_cls_prog_type": true,
>             ...
>         }
>     }
> 
> v2:
> - Move probes from bpftool to libbpf.
> - Remove C-style macros output from this patch.
> 
> Signed-off-by: Quentin Monnet <quentin.monnet@...ronome.com>
> ---
>  tools/bpf/bpftool/feature.c   | 53 +++++++++++++++++++++++++++++++--
>  tools/lib/bpf/Build           |  2 +-
>  tools/lib/bpf/libbpf.h        |  6 ++++
>  tools/lib/bpf/libbpf.map      |  1 +
>  tools/lib/bpf/libbpf_probes.c | 55 +++++++++++++++++++++++++++++++++++
>  5 files changed, 114 insertions(+), 3 deletions(-)
>  create mode 100644 tools/lib/bpf/libbpf_probes.c
> 
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 238d7b80f426..3ba0a0a5904c 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>  /* Copyright (c) 2018 Netronome Systems, Inc. */
>  
> +#include <ctype.h>
>  #include <errno.h>
>  #include <string.h>
>  #include <unistd.h>
> @@ -11,6 +12,7 @@
>  #include <linux/limits.h>
>  
>  #include <bpf.h>
> +#include <libbpf.h>
>  
>  #include "main.h"
>  
> @@ -83,6 +85,17 @@ print_start_section(const char *json_title, const char *plain_title)
>  	}
>  }
>  
> +static void
> +print_end_then_start_section(const char *json_title, const char *plain_title)
> +{
> +	if (json_output)
> +		jsonw_end_object(json_wtr);
> +	else
> +		printf("\n");
> +
> +	print_start_section(json_title, plain_title);
> +}
> +
>  /* Probing functions */
>  
>  static int read_procfs(const char *path)
> @@ -361,9 +374,36 @@ static bool probe_bpf_syscall(void)
>  	return res;
>  }
>  
> +static void
> +probe_prog_type(enum bpf_prog_type prog_type, int kernel_version,
> +		bool *supported_types)
> +{
> +	const char *plain_comment = "eBPF program_type ";
> +	char feat_name[128], plain_desc[128];
> +	size_t maxlen;
> +	bool res;
> +
> +	res = bpf_probe_prog_type(prog_type, kernel_version, 0);
> +
> +	supported_types[prog_type] |= res;
> +
> +	maxlen = sizeof(plain_desc) - strlen(plain_comment) - 1;
> +	if (strlen(prog_type_name[prog_type]) > maxlen) {
> +		p_info("program type name too long");
> +		return;
> +	}
> +
> +	sprintf(feat_name, "have_%s_prog_type", prog_type_name[prog_type]);
> +	sprintf(plain_desc, "%s%s", plain_comment, prog_type_name[prog_type]);
> +	print_bool_feature(feat_name, plain_desc, res);
> +}
> +
>  static int do_probe(int argc, char **argv)
>  {
>  	enum probe_component target = COMPONENT_UNSPEC;
> +	bool supported_types[128] = {};
> +	int kernel_version;
> +	unsigned int i;
>  
>  	/* Detection assumes user has sufficient privileges (CAP_SYS_ADMIN).
>  	 * Let's approximate, and restrict usage to root user only.
> @@ -417,9 +457,18 @@ static int do_probe(int argc, char **argv)
>  	print_start_section("syscall_config",
>  			    "Scanning system call and kernel version...");
>  
> -	probe_kernel_version();
> -	probe_bpf_syscall();
> +	kernel_version = probe_kernel_version();
> +	if (!probe_bpf_syscall())
> +		/* bpf() syscall unavailable, don't probe other BPF features */
> +		goto exit_close_json;
> +
> +	print_end_then_start_section("program_types",
> +				     "Scanning eBPF program types...");
> +
> +	for (i = BPF_PROG_TYPE_UNSPEC + 1; i < ARRAY_SIZE(prog_type_name); i++)
> +		probe_prog_type(i, kernel_version, supported_types);
>  
> +exit_close_json:
>  	if (json_output) {
>  		/* End current "section" of probes */
>  		jsonw_end_object(json_wtr);
> diff --git a/tools/lib/bpf/Build b/tools/lib/bpf/Build
> index 197b40f5b5c6..bfd9bfc82c3b 100644
> --- a/tools/lib/bpf/Build
> +++ b/tools/lib/bpf/Build
> @@ -1 +1 @@
> -libbpf-y := libbpf.o bpf.o nlattr.o btf.o libbpf_errno.o str_error.o netlink.o bpf_prog_linfo.o
> +libbpf-y := libbpf.o bpf.o nlattr.o btf.o libbpf_errno.o str_error.o netlink.o bpf_prog_linfo.o libbpf_probes.o
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5f68d7b75215..f4bb2764ca9a 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -355,6 +355,12 @@ LIBBPF_API const struct bpf_line_info *
>  bpf_prog_linfo__lfind(const struct bpf_prog_linfo *prog_linfo,
>  		      __u32 insn_off, __u32 nr_skip);
>  
> +/*
> + * Probe for supported system features
> + */
> +LIBBPF_API bool bpf_probe_prog_type(enum bpf_prog_type prog_type,
> +				    int kernel_version, __u32 ifindex);
> +
>  #ifdef __cplusplus
>  } /* extern "C" */
>  #endif
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index cd02cd4e2cc3..6355e4c80a86 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -56,6 +56,7 @@ LIBBPF_0.0.1 {
>  		bpf_object__unpin_maps;
>  		bpf_object__unpin_programs;
>  		bpf_perf_event_read_simple;
> +		bpf_probe_prog_type;
>  		bpf_prog_attach;
>  		bpf_prog_detach;
>  		bpf_prog_detach2;
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> new file mode 100644
> index 000000000000..2c5e0cdc9f2f
> --- /dev/null
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> +
> +/* Copyright (c) 2018 Netronome Systems, Inc. */
> +
> +#include <errno.h>
> +#include <unistd.h>
> +
> +#include <linux/filter.h>
> +#include <linux/kernel.h>
> +
> +#include "bpf.h"
> +#include "libbpf.h"
> +
> +static void
> +prog_load(enum bpf_prog_type prog_type, const struct bpf_insn *insns,
> +	  size_t insns_cnt, int kernel_version, char *buf, size_t buf_len,
> +	  __u32 ifindex)
> +{
> +	struct bpf_load_program_attr xattr = {};
> +	int fd;
> +
> +	/* Some prog type require an expected_attach_type */
> +	if (prog_type == BPF_PROG_TYPE_CGROUP_SOCK_ADDR)
> +		xattr.expected_attach_type = BPF_CGROUP_INET4_CONNECT;
> +
> +	xattr.prog_type = prog_type;
> +	xattr.insns = insns;
> +	xattr.insns_cnt = insns_cnt;
> +	xattr.license = "GPL";
> +	xattr.kern_version = kernel_version;
> +	xattr.prog_ifindex = ifindex;
> +
> +	fd = bpf_load_program_xattr(&xattr, buf, buf_len);
> +	if (fd >= 0)
> +		close(fd);
> +}
> +
> +bool bpf_probe_prog_type(enum bpf_prog_type prog_type, int kernel_version,
Do we really need this kernel_version argument? Isn't it going away in
the future (I saw a patch from Daniel that drops kernel check under
sys_bpf). Going forward, does it make sense to have it at the API
level?
> +			 __u32 ifindex)
> +{
> +	struct bpf_insn insns[2] = {
> +		BPF_MOV64_IMM(BPF_REG_0, 0),
> +		BPF_EXIT_INSN()
> +	};
> +
> +	if (ifindex && prog_type == BPF_PROG_TYPE_SCHED_CLS)
> +		/* nfp returns -EINVAL on exit(0) with TC offload */
> +		insns[0].imm = 2;
> +
> +	errno = 0;
> +	prog_load(prog_type, insns, ARRAY_SIZE(insns), kernel_version,
> +		  NULL, 0, ifindex);
> +
> +	return errno != EINVAL && errno != EOPNOTSUPP;
> +}
> -- 
> 2.17.1
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ