| 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
| ||
|
Message-ID: <20181220182952.GT20955@mini-arch.hsd1.ca.comcast.net>
Date: Thu, 20 Dec 2018 10:29:52 -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: [oss-drivers] Re: [RFC PATCH bpf-next v2 4/9] tools: bpftool:
add probes for eBPF program types
On 12/20, Quentin Monnet wrote:
> 2018-12-20 10:17 UTC-0800 ~ Stanislav Fomichev <sdf@...ichev.me>
> > On 12/20, Quentin Monnet wrote:
> > > 2018-12-20 09:45 UTC-0800 ~ Stanislav Fomichev <sdf@...ichev.me>
> > > > 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?
> > >
> > > It does go away and should be left aside for new applications. The objective
> > > with bpftool is to be able to probe systems possibly running older kernels
> > > though, and the only way to probe for kprobes support on anything older than
> > > a 4.21 is to keep the version number.
> > But isn't kernel_version irrelevant for probing? For probing, we always want
> > to have kernel_version == <running kernel version>. So I don't
> > understand why libbpf user should care about the version.
>
> Oh, do you mean it should be collected in libbpf instead of bpftool and not
> passed by the caller? Sorry I didn't understand in the first place. If so,
> you're absolutely right, I should fix that, thanks!
Yes, I was basically trying to understand why the user needs to pass it.
Querying/setting it for type==trace_point in prog_load seems like a way to go.
Powered by blists - more mailing lists