bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFCv2 0/9] bpf: Add d_path helper
@ 2020-05-06 13:29 Jiri Olsa
  2020-05-06 13:29 ` [PATCH 1/9] " Jiri Olsa
                   ` (8 more replies)
  0 siblings, 9 replies; 28+ messages in thread
From: Jiri Olsa @ 2020-05-06 13:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Yonghong Song, Martin KaFai Lau, David Miller,
	John Fastabend, Jesper Dangaard Brouer, Wenbo Zhang, KP Singh,
	Andrii Nakryiko, bgregg, Florent Revest, Al Viro

hi,
adding d_path helper to return full path for 'path' object.

I originally added and used 'file_path' helper, which did the same,
but used 'struct file' object. Then realized that file_path is just
a wrapper for d_path, so we'd cover more calling sites if we add
d_path helper and allowed resolving BTF object within another object,
so we could call d_path also with file pointer, like:

  bpf_d_path(&file->f_path, buf, size);

This feature is mainly to be able to add dpath (filepath originally)
function to bpftrace, which seems to work nicely now, like:

  # bpftrace -e 'kfunc:vfs_open { printf("%s\n", dpath(args->path)); }'


RFC v2 changes:

  - added whitelist support, d_path helper is allowed only for
    list of functions, the whitelist checking works as follows:

      - helper's whitelist is defined as list of functions in file:
        kernel/bpf/helpers-whitelist/helper
      - at vmlinux linking time, the bpfwl tool reads the whitelist
        and translates functions into BTF IDs, which are then compiled
        as following data section into vmlinux object:

          .BTF_whitelist
              BTF_whitelist_<helper1>
              BTF_whitelist_<helper2>
              BTF_whitelist_<helper3>

        Each BTF_whitelist_<helperX> data is a sorted array of BTF ids.
      - new 'allowed' function is added to 'struct bpf_func_proto',
        which is used by verifier code to check (if defined) on whether
        the helper is called from allowed function (from whitelist).

    Currently it's needed and implemented only for d_path helper,
    but it's easy to add support for another helper.

  - I don't change the btf_id value in check_func_arg as suggested by Alexei
  - added new test_verifier test

Also available at:
  https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  bpf/d_path

thoughts? thanks,
jirka


---
Jiri Olsa (9):
      bpf: Add d_path helper
      bpf: Add d_path whitelist
      bpf: Add bpfwl tool to construct bpf whitelists
      bpf: Allow nested BTF object to be refferenced by BTF object + offset
      bpf: Add support to check on BTF id whitelist for d_path helper
      bpf: Compile bpfwl tool at kernel compilation start
      bpf: Compile the BTF id whitelist data in vmlinux
      selftests/bpf: Add test for d_path helper
      selftests/bpf: Add verifier test for d_path helper

 Makefile                                        |  24 +++++++--
 include/asm-generic/vmlinux.lds.h               |   5 ++
 include/linux/bpf.h                             |   4 ++
 include/uapi/linux/bpf.h                        |  14 +++++-
 kernel/bpf/btf.c                                |  69 +++++++++++++++++++++++++
 kernel/bpf/helpers-whitelist/d_path             |   8 +++
 kernel/bpf/verifier.c                           |  37 ++++++++++----
 kernel/trace/bpf_trace.c                        |  54 ++++++++++++++++++++
 scripts/bpf_helpers_doc.py                      |   2 +
 scripts/link-vmlinux.sh                         |  20 ++++++--
 tools/Makefile                                  |   3 ++
 tools/bpf/Makefile                              |   5 +-
 tools/bpf/bpfwl/Build                           |  11 ++++
 tools/bpf/bpfwl/Makefile                        |  60 ++++++++++++++++++++++
 tools/bpf/bpfwl/bpfwl.c                         | 285 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h                  |  14 +++++-
 tools/testing/selftests/bpf/prog_tests/d_path.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/progs/test_d_path.c |  71 ++++++++++++++++++++++++++
 tools/testing/selftests/bpf/test_verifier.c     |  13 ++++-
 tools/testing/selftests/bpf/verifier/d_path.c   |  37 ++++++++++++++
 20 files changed, 908 insertions(+), 24 deletions(-)
 create mode 100644 kernel/bpf/helpers-whitelist/d_path
 create mode 100644 tools/bpf/bpfwl/Build
 create mode 100644 tools/bpf/bpfwl/Makefile
 create mode 100644 tools/bpf/bpfwl/bpfwl.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/d_path.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_d_path.c
 create mode 100644 tools/testing/selftests/bpf/verifier/d_path.c


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/9] bpf: Add d_path helper
  2020-05-06 13:29 [RFCv2 0/9] bpf: Add d_path helper Jiri Olsa
@ 2020-05-06 13:29 ` Jiri Olsa
  2020-05-14 22:06   ` Andrii Nakryiko
  2020-05-06 13:29 ` [PATCH 2/9] bpf: Add d_path whitelist Jiri Olsa
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2020-05-06 13:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Yonghong Song, Martin KaFai Lau, David Miller,
	John Fastabend, Jesper Dangaard Brouer, Wenbo Zhang, KP Singh,
	Andrii Nakryiko, bgregg, Florent Revest, Al Viro

Adding d_path helper function that returns full path
for give 'struct path' object, which needs to be the
kernel BTF 'path' object.

The helper calls directly d_path function.

Updating also bpf.h tools uapi header and adding
'path' to bpf_helpers_doc.py script.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/uapi/linux/bpf.h       | 14 +++++++++++++-
 kernel/trace/bpf_trace.c       | 31 +++++++++++++++++++++++++++++++
 scripts/bpf_helpers_doc.py     |  2 ++
 tools/include/uapi/linux/bpf.h | 14 +++++++++++++-
 4 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b3643e27e264..bc13cad27872 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3068,6 +3068,17 @@ union bpf_attr {
  * 		See: clock_gettime(CLOCK_BOOTTIME)
  * 	Return
  * 		Current *ktime*.
+ *
+ * int bpf_d_path(struct path *path, char *buf, u32 sz)
+ *	Description
+ *		Return full path for given 'struct path' object, which
+ *		needs to be the kernel BTF 'path' object. The path is
+ *		returned in buffer provided 'buf' of size 'sz'.
+ *
+ *	Return
+ *		length of returned string on success, or a negative
+ *		error in case of failure
+ *
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -3195,7 +3206,8 @@ union bpf_attr {
 	FN(get_netns_cookie),		\
 	FN(get_current_ancestor_cgroup_id),	\
 	FN(sk_assign),			\
-	FN(ktime_get_boot_ns),
+	FN(ktime_get_boot_ns),		\
+	FN(d_path),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index e875c95d3ced..3097ab04bdc4 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -779,6 +779,35 @@ static const struct bpf_func_proto bpf_send_signal_thread_proto = {
 	.arg1_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz)
+{
+	char *p = d_path(path, buf, sz - 1);
+	int len;
+
+	if (IS_ERR(p)) {
+		len = PTR_ERR(p);
+	} else {
+		len = strlen(p);
+		if (len && p != buf) {
+			memmove(buf, p, len);
+			buf[len] = 0;
+		}
+	}
+
+	return len;
+}
+
+static u32 bpf_d_path_btf_ids[3];
+static const struct bpf_func_proto bpf_d_path_proto = {
+	.func		= bpf_d_path,
+	.gpl_only	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_BTF_ID,
+	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_CONST_SIZE,
+	.btf_id		= bpf_d_path_btf_ids,
+};
+
 const struct bpf_func_proto *
 bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -1226,6 +1255,8 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	case BPF_FUNC_xdp_output:
 		return &bpf_xdp_output_proto;
 #endif
+	case BPF_FUNC_d_path:
+		return &bpf_d_path_proto;
 	default:
 		return raw_tp_prog_func_proto(func_id, prog);
 	}
diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
index f43d193aff3a..8f62cbc4c3ff 100755
--- a/scripts/bpf_helpers_doc.py
+++ b/scripts/bpf_helpers_doc.py
@@ -418,6 +418,7 @@ class PrinterHelpers(Printer):
             'struct __sk_buff',
             'struct sk_msg_md',
             'struct xdp_md',
+            'struct path',
     ]
     known_types = {
             '...',
@@ -450,6 +451,7 @@ class PrinterHelpers(Printer):
             'struct sk_reuseport_md',
             'struct sockaddr',
             'struct tcphdr',
+            'struct path',
     }
     mapped_types = {
             'u8': '__u8',
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index b3643e27e264..bc13cad27872 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3068,6 +3068,17 @@ union bpf_attr {
  * 		See: clock_gettime(CLOCK_BOOTTIME)
  * 	Return
  * 		Current *ktime*.
+ *
+ * int bpf_d_path(struct path *path, char *buf, u32 sz)
+ *	Description
+ *		Return full path for given 'struct path' object, which
+ *		needs to be the kernel BTF 'path' object. The path is
+ *		returned in buffer provided 'buf' of size 'sz'.
+ *
+ *	Return
+ *		length of returned string on success, or a negative
+ *		error in case of failure
+ *
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -3195,7 +3206,8 @@ union bpf_attr {
 	FN(get_netns_cookie),		\
 	FN(get_current_ancestor_cgroup_id),	\
 	FN(sk_assign),			\
-	FN(ktime_get_boot_ns),
+	FN(ktime_get_boot_ns),		\
+	FN(d_path),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/9] bpf: Add d_path whitelist
  2020-05-06 13:29 [RFCv2 0/9] bpf: Add d_path helper Jiri Olsa
  2020-05-06 13:29 ` [PATCH 1/9] " Jiri Olsa
@ 2020-05-06 13:29 ` Jiri Olsa
  2020-05-06 13:29 ` [PATCH 3/9] bpf: Add bpfwl tool to construct bpf whitelists Jiri Olsa
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2020-05-06 13:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Yonghong Song, Martin KaFai Lau, David Miller,
	John Fastabend, Jesper Dangaard Brouer, Wenbo Zhang, KP Singh,
	Andrii Nakryiko, bgregg, Florent Revest, Al Viro

Adding whitelist for d_path helper under:
  kernel/bpf/helpers-whitelist/d_path

Currently it's just list of test functions, which will
be replaced by list promised by Brendan ;-)

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/helpers-whitelist/d_path | 8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 kernel/bpf/helpers-whitelist/d_path

diff --git a/kernel/bpf/helpers-whitelist/d_path b/kernel/bpf/helpers-whitelist/d_path
new file mode 100644
index 000000000000..e5adf2a9e1cb
--- /dev/null
+++ b/kernel/bpf/helpers-whitelist/d_path
@@ -0,0 +1,8 @@
+do_truncate
+vfs_fallocate
+finish_open
+vfs_open
+generic_file_open
+filp_close
+dentry_open
+vfs_getattr
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 3/9] bpf: Add bpfwl tool to construct bpf whitelists
  2020-05-06 13:29 [RFCv2 0/9] bpf: Add d_path helper Jiri Olsa
  2020-05-06 13:29 ` [PATCH 1/9] " Jiri Olsa
  2020-05-06 13:29 ` [PATCH 2/9] bpf: Add d_path whitelist Jiri Olsa
@ 2020-05-06 13:29 ` Jiri Olsa
  2020-05-14 22:20   ` Andrii Nakryiko
  2020-05-06 13:29 ` [PATCH 4/9] bpf: Allow nested BTF object to be refferenced by BTF object + offset Jiri Olsa
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2020-05-06 13:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Yonghong Song, Martin KaFai Lau, David Miller,
	John Fastabend, Jesper Dangaard Brouer, Wenbo Zhang, KP Singh,
	Andrii Nakryiko, bgregg, Florent Revest, Al Viro

This tool takes vmlinux object and whitelist directory on input
and produces C source object with BPF whitelist data.

The vmlinux object needs to have a BTF information compiled in.

The whitelist directory is expected to contain files with helper
names, where each file contains list of functions/probes that
helper is allowed to be called from - whitelist.

The bpfwl tool has following output:

  $ bpfwl vmlinux dir
  unsigned long d_path[] __attribute__((section(".BTF_whitelist_d_path"))) = \
  { 24507, 24511, 24537, 24539, 24545, 24588, 24602, 24920 };

Each array are sorted BTF ids of the functions provided in the
helper file.

Each array will be compiled into kernel and used during the helper
check in verifier.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/bpf/bpfwl/Build    |  11 ++
 tools/bpf/bpfwl/Makefile |  60 +++++++++
 tools/bpf/bpfwl/bpfwl.c  | 285 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 356 insertions(+)
 create mode 100644 tools/bpf/bpfwl/Build
 create mode 100644 tools/bpf/bpfwl/Makefile
 create mode 100644 tools/bpf/bpfwl/bpfwl.c

diff --git a/tools/bpf/bpfwl/Build b/tools/bpf/bpfwl/Build
new file mode 100644
index 000000000000..667e30d6ce79
--- /dev/null
+++ b/tools/bpf/bpfwl/Build
@@ -0,0 +1,11 @@
+bpfwl-y += bpfwl.o
+bpfwl-y += rbtree.o
+bpfwl-y += zalloc.o
+
+$(OUTPUT)rbtree.o: ../../lib/rbtree.c FORCE
+	$(call rule_mkdir)
+	$(call if_changed_dep,cc_o_c)
+
+$(OUTPUT)zalloc.o: ../../lib/zalloc.c FORCE
+	$(call rule_mkdir)
+	$(call if_changed_dep,cc_o_c)
diff --git a/tools/bpf/bpfwl/Makefile b/tools/bpf/bpfwl/Makefile
new file mode 100644
index 000000000000..8174eeb7eea6
--- /dev/null
+++ b/tools/bpf/bpfwl/Makefile
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: GPL-2.0-only
+include ../../scripts/Makefile.include
+
+MAKEFLAGS=--no-print-directory
+
+ifeq ($(srctree),)
+srctree := $(patsubst %/,%,$(dir $(CURDIR)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+endif
+
+ifeq ($(V),1)
+  Q =
+else
+  Q = @
+endif
+
+BPF_DIR = $(srctree)/tools/lib/bpf/
+
+ifneq ($(OUTPUT),)
+  LIBBPF_PATH = $(OUTPUT)/libbpf/
+else
+  LIBBPF_PATH = $(BPF_DIR)
+endif
+
+LIBBPF    = $(LIBBPF_PATH)libbpf.a
+BPFWL     = $(OUTPUT)bpfwl
+BPFWL_IN  = $(BPFWL)-in.o
+
+all: $(OUTPUT)bpfwl
+
+$(LIBBPF): FORCE
+	$(if $(LIBBPF_PATH),@mkdir -p $(LIBBPF_PATH))
+	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_PATH) $(LIBBPF_PATH)libbpf.a
+
+$(LIBBPF)-clean:
+	$(call QUIET_CLEAN, libbpf)
+	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_PATH) clean >/dev/null
+
+CFLAGS := -g -I$(srctree)/tools/include -I$(BPF_DIR)
+
+LIBS = -lelf -lz
+
+export srctree OUTPUT CFLAGS
+include $(srctree)/tools/build/Makefile.include
+
+$(BPFWL_IN): fixdep FORCE
+	$(Q)$(MAKE) $(build)=bpfwl
+
+$(BPFWL): $(LIBBPF) $(BPFWL_IN)
+	$(QUIET_LINK)$(CC) $(BPFWL_IN) $(LDFLAGS) -o $@ $(LIBBPF) $(LIBS)
+
+clean: $(LIBBPF)-clean
+	$(call QUIET_CLEAN, bpfwl)
+	$(Q)$(RM) -f $(BPFWL)
+	$(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
+
+FORCE:
+
+.PHONY: all FORCE clean
diff --git a/tools/bpf/bpfwl/bpfwl.c b/tools/bpf/bpfwl/bpfwl.c
new file mode 100644
index 000000000000..495c2bcf620a
--- /dev/null
+++ b/tools/bpf/bpfwl/bpfwl.c
@@ -0,0 +1,285 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+#define  _GNU_SOURCE
+
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <stdlib.h>
+#include <linux/rbtree.h>
+#include <linux/list.h>
+#include <linux/kernel.h>
+#include <linux/zalloc.h>
+#include <linux/limits.h>
+#include <btf.h>
+#include <libbpf.h>
+
+struct func {
+	char			*name;
+	unsigned long		 id;
+	struct rb_node		 rb_node;
+	struct list_head	 list[];
+};
+
+struct helper {
+	char			*name;
+	int			 idx;
+	int			 count;
+	struct list_head	 node;
+	struct list_head	 funcs;
+};
+
+static struct rb_root funcs;
+static LIST_HEAD(helpers);
+static int idxs;
+
+static struct func *func__new(const char *name)
+{
+	size_t size = idxs * sizeof(struct list_head);
+	struct func *func;
+	int i;
+
+	func = zalloc(sizeof(*func) + size);
+	if (func) {
+		func->name = strdup(name);
+		for (i = 0; i < idxs; i++)
+			INIT_LIST_HEAD(&func->list[i]);
+	}
+	return func;
+}
+
+static struct helper *helper__new(const char *name)
+{
+	struct helper *helper = zalloc(sizeof(*helper));
+
+	if (helper) {
+		helper->idx = idxs++;
+		helper->name = strdup(name);
+		INIT_LIST_HEAD(&helper->funcs);
+		list_add_tail(&helper->node, &helpers);
+	}
+	return helper;
+}
+
+static struct func *func__add(char *name)
+{
+	struct rb_node **p = &funcs.rb_node;
+	struct rb_node *parent = NULL;
+	struct func *func;
+	int cmp;
+
+	while (*p != NULL) {
+		parent = *p;
+		func = rb_entry(parent, struct func, rb_node);
+		cmp = strcmp(func->name, name);
+		if (cmp < 0)
+			p = &(*p)->rb_left;
+		else if (cmp > 0)
+			p = &(*p)->rb_right;
+		else
+			return func;
+	}
+
+	func = func__new(name);
+	if (func) {
+		rb_link_node(&func->rb_node, parent, p);
+		rb_insert_color(&func->rb_node, &funcs);
+	}
+	return func;
+}
+
+static struct func *func__find(const char *name)
+{
+	struct rb_node *p = funcs.rb_node;
+	struct func *func;
+	int cmp;
+
+	while (p != NULL) {
+		func = rb_entry(p, struct func, rb_node);
+		cmp = strcmp(func->name, name);
+		if (cmp < 0)
+			p = p->rb_left;
+		else if (cmp > 0)
+			p = p->rb_right;
+		else
+			return func;
+	}
+	return NULL;
+}
+
+static int helper__read(struct helper *helper, const char *base)
+{
+	char path[PATH_MAX];
+	char *line = NULL;
+	size_t len = 0;
+	ssize_t nread;
+	int err = -1;
+	FILE *file;
+
+	snprintf(path, sizeof(path), "%s/%s", base, helper->name);
+
+	file = fopen(path, "r");
+	if (file == NULL) {
+		perror("FAILED fopen whitelist");
+		return -1;
+	}
+
+	while ((nread = getline(&line, &len, file)) != -1) {
+		struct func *func;
+		char *p;
+
+		p = strchr(line, '\n');
+		if (p)
+			*p = 0;
+
+		func = func__add(line);
+		if (!func)
+			goto err;
+
+		list_add_tail(&func->list[helper->idx], &helper->funcs);
+		helper->count++;
+	}
+
+	err = 0;
+err:
+	free(line);
+	fclose(file);
+	return err;
+}
+
+static int comp(const void *a, const void *b)
+{
+	const unsigned long *pa = a;
+	const unsigned long *pb = b;
+
+	return *pa - *pb;
+}
+
+int main(int argc, char **argv)
+{
+	struct helper *helper;
+	const char *wl_path;
+	const char *vmlinux;
+	struct dirent *d;
+	struct btf *btf;
+	int id, err;
+	DIR *dir;
+	__u32 nr;
+
+	if (argc != 3) {
+		fprintf(stderr, "%s <vmlinux> <dir>", argv[0]);
+		return -1;
+	}
+
+	vmlinux = argv[1];
+	wl_path = argv[2];
+
+	dir = opendir(wl_path);
+	if (dir == NULL) {
+		perror("FAILED: open directory");
+		return -1;
+	}
+
+	/*
+	 * Scan the whitelist directory and create 'struct helper'
+	 * object for every file. Read and put all the functions
+	 * into funcs rb tree and link them to each helper.
+	 */
+	while ((d = readdir(dir)) != NULL) {
+		if (!strcmp(d->d_name, ".") ||
+		    !strcmp(d->d_name, ".."))
+			continue;
+
+		helper = helper__new(d->d_name);
+		if (!helper) {
+			fprintf(stderr, "FAILED: not enough memory\n");
+			return -1;
+		}
+
+		if (helper__read(helper, wl_path))
+			return -1;
+	}
+
+	closedir(dir);
+
+	btf = btf__parse_elf(vmlinux, NULL);
+	err = libbpf_get_error(btf);
+	if (err) {
+		fprintf(stderr, "FAILED: load BTF from %s: %s",
+			vmlinux, strerror(err));
+		return -1;
+	}
+
+	nr = btf__get_nr_types(btf);
+
+	/* Iterate all the BTF types and resolve all the function IDs. */
+	for (id = 0; id < nr; id++) {
+		const struct btf_type *type;
+		struct func *func;
+		const char *str;
+
+		type = btf__type_by_id(btf, id);
+		if (!type)
+			continue;
+
+		if (BTF_INFO_KIND(type->info) != BTF_KIND_FUNC)
+			continue;
+
+		str = btf__name_by_offset(btf, type->name_off);
+		if (!str)
+			continue;
+
+		func = func__find(str);
+		if (func)
+			func->id = id;
+	}
+
+	/*
+	 * Load BTF IDs for each helper into array, sort it,
+	 * and dump the C code for the helper array.
+	 */
+	list_for_each_entry(helper, &helpers, node) {
+		unsigned long *ids;
+		bool first = true;
+		struct func *func;
+		int idx = 0;
+
+		ids = malloc(helper->count * sizeof(unsigned long));
+		if (!ids) {
+			fprintf(stderr, "FAILED: not enough memory\n");
+			return -1;
+		}
+
+		list_for_each_entry(func, &helper->funcs, list[helper->idx]) {
+			if (!func->id) {
+				fprintf(stderr, "FAILED: '%s' function not found in BTF data\n",
+					func->name);
+				return -1;
+			}
+
+			ids[idx++] = func->id;
+		}
+
+		qsort(ids, helper->count, sizeof(unsigned long), comp);
+
+		fprintf(stdout,
+			"unsigned long %s[] __attribute__((section(\".BTF_whitelist_%s\"))) = { ",
+			helper->name, helper->name);
+
+		for (idx = 0; idx < helper->count; idx++, first = false)
+			fprintf(stdout, "%s%lu", first ? "" : ", ", ids[idx]);
+
+		fprintf(stdout, " };\n");
+
+		free(ids);
+	}
+
+	/*
+	 * Not releasing anything intentionaly..
+	 *
+	 * btf__free(btf)
+	 * free helpers/funcs
+	 */
+	return 0;
+}
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 4/9] bpf: Allow nested BTF object to be refferenced by BTF object + offset
  2020-05-06 13:29 [RFCv2 0/9] bpf: Add d_path helper Jiri Olsa
                   ` (2 preceding siblings ...)
  2020-05-06 13:29 ` [PATCH 3/9] bpf: Add bpfwl tool to construct bpf whitelists Jiri Olsa
@ 2020-05-06 13:29 ` Jiri Olsa
  2020-05-14 22:32   ` Andrii Nakryiko
  2020-05-06 13:29 ` [PATCH 5/9] bpf: Add support to check on BTF id whitelist for d_path helper Jiri Olsa
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2020-05-06 13:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Yonghong Song, Martin KaFai Lau, David Miller,
	John Fastabend, Jesper Dangaard Brouer, Wenbo Zhang, KP Singh,
	Andrii Nakryiko, bgregg, Florent Revest, Al Viro

Adding btf_struct_address function that takes 2 BTF objects
and offset as arguments and checks wether object A is nested
in object B on given offset.

This function is be used when checking the helper function
PTR_TO_BTF_ID arguments. If the argument has an offset value,
the btf_struct_address will check if the final address is
the expected BTF ID.

This way we can access nested BTF objects under PTR_TO_BTF_ID
pointer type and pass them to helpers, while they still point
to valid kernel BTF objects.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/bpf.h   |  3 ++
 kernel/bpf/btf.c      | 69 +++++++++++++++++++++++++++++++++++++++++++
 kernel/bpf/verifier.c | 32 +++++++++++++-------
 3 files changed, 94 insertions(+), 10 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1262ec460ab3..bc589cdd8c34 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1213,6 +1213,9 @@ int btf_struct_access(struct bpf_verifier_log *log,
 		      const struct btf_type *t, int off, int size,
 		      enum bpf_access_type atype,
 		      u32 *next_btf_id);
+int btf_struct_address(struct bpf_verifier_log *log,
+		     const struct btf_type *t,
+		     u32 off, u32 exp_id);
 int btf_resolve_helper_id(struct bpf_verifier_log *log,
 			  const struct bpf_func_proto *fn, int);
 
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index a2cfba89a8e1..07f22469acab 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4004,6 +4004,75 @@ int btf_struct_access(struct bpf_verifier_log *log,
 	return -EINVAL;
 }
 
+int btf_struct_address(struct bpf_verifier_log *log,
+		       const struct btf_type *t,
+		       u32 off, u32 exp_id)
+{
+	u32 i, moff, mtrue_end, msize = 0;
+	const struct btf_member *member;
+	const struct btf_type *mtype;
+	const char *tname, *mname;
+
+again:
+	tname = __btf_name_by_offset(btf_vmlinux, t->name_off);
+	if (!btf_type_is_struct(t)) {
+		bpf_log(log, "Type '%s' is not a struct\n", tname);
+		return -EINVAL;
+	}
+
+	if (off > t->size) {
+		bpf_log(log, "address beyond struct %s at off %u size %u\n",
+			tname, off, t->size);
+		return -EACCES;
+	}
+
+	for_each_member(i, t, member) {
+		/* offset of the field in bytes */
+		moff = btf_member_bit_offset(t, member) / 8;
+		if (off < moff)
+			/* won't find anything, field is already too far */
+			break;
+
+		/* we found the member */
+		if (off == moff && member->type == exp_id)
+			return 0;
+
+		/* type of the field */
+		mtype = btf_type_by_id(btf_vmlinux, member->type);
+		mname = __btf_name_by_offset(btf_vmlinux, member->name_off);
+
+		mtype = btf_resolve_size(btf_vmlinux, mtype, &msize,
+					 NULL, NULL);
+		if (IS_ERR(mtype)) {
+			bpf_log(log, "field %s doesn't have size\n", mname);
+			return -EFAULT;
+		}
+
+		mtrue_end = moff + msize;
+		if (off >= mtrue_end)
+			/* no overlap with member, keep iterating */
+			continue;
+
+		/* the 'off' we're looking for is either equal to start
+		 * of this field or inside of this struct
+		 */
+		if (btf_type_is_struct(mtype)) {
+			/* our field must be inside that union or struct */
+			t = mtype;
+
+			/* adjust offset we're looking for */
+			off -= moff;
+			goto again;
+		}
+
+		bpf_log(log, "struct %s doesn't have struct field at offset %d\n", tname, off);
+		return -EACCES;
+	}
+
+	bpf_log(log, "struct %s doesn't have field at offset %d\n", tname, off);
+	return -EACCES;
+}
+
 static int __btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn,
 				   int arg)
 {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 70ad009577f8..b988df5ada20 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3665,6 +3665,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
 {
 	struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[regno];
 	enum bpf_reg_type expected_type, type = reg->type;
+	const struct btf_type *btf_type;
 	int err = 0;
 
 	if (arg_type == ARG_DONTCARE)
@@ -3743,17 +3744,28 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
 		expected_type = PTR_TO_BTF_ID;
 		if (type != expected_type)
 			goto err_type;
-		if (reg->btf_id != meta->btf_id) {
-			verbose(env, "Helper has type %s got %s in R%d\n",
-				kernel_type_name(meta->btf_id),
-				kernel_type_name(reg->btf_id), regno);
+		if (reg->off) {
+			btf_type = btf_type_by_id(btf_vmlinux, reg->btf_id);
+			if (btf_struct_address(&env->log, btf_type, reg->off, meta->btf_id)) {
+				verbose(env, "Helper has type %s got %s in R%d, off %d\n",
+					kernel_type_name(meta->btf_id),
+					kernel_type_name(reg->btf_id), regno, reg->off);
 
-			return -EACCES;
-		}
-		if (!tnum_is_const(reg->var_off) || reg->var_off.value || reg->off) {
-			verbose(env, "R%d is a pointer to in-kernel struct with non-zero offset\n",
-				regno);
-			return -EACCES;
+				return -EACCES;
+			}
+		} else {
+			if (reg->btf_id != meta->btf_id) {
+				verbose(env, "Helper has type %s got %s in R%d\n",
+					kernel_type_name(meta->btf_id),
+					kernel_type_name(reg->btf_id), regno);
+
+				return -EACCES;
+			}
+			if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
+				verbose(env, "R%d is a pointer to in-kernel struct with non-zero offset\n",
+					regno);
+				return -EACCES;
+			}
 		}
 	} else if (arg_type == ARG_PTR_TO_SPIN_LOCK) {
 		if (meta->func_id == BPF_FUNC_spin_lock) {
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 5/9] bpf: Add support to check on BTF id whitelist for d_path helper
  2020-05-06 13:29 [RFCv2 0/9] bpf: Add d_path helper Jiri Olsa
                   ` (3 preceding siblings ...)
  2020-05-06 13:29 ` [PATCH 4/9] bpf: Allow nested BTF object to be refferenced by BTF object + offset Jiri Olsa
@ 2020-05-06 13:29 ` Jiri Olsa
  2020-05-06 13:29 ` [PATCH 6/9] bpf: Compile bpfwl tool at kernel compilation start Jiri Olsa
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2020-05-06 13:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Yonghong Song, Martin KaFai Lau, David Miller,
	John Fastabend, Jesper Dangaard Brouer, Wenbo Zhang, KP Singh,
	Andrii Nakryiko, bgregg, Florent Revest, Al Viro

The BPF helper whitelist checking works as follows:

  - helper's whitelist is defined as list of functions in file:
    kernel/bpf/helpers-whitelist/helper

  - at vmlinux linking time, the bpfwl tool reads the whitelist
    and translates functions into BTF IDs, which are compiled as
    following data section into vmlinux object:

      .BTF_whitelist
          BTF_whitelist_<helper1>
          BTF_whitelist_<helper2>
          BTF_whitelist_<helper3>

    Each BTF_whitelist_<helperX> data is a sorted array of BTF ids.

  - new 'allowed' function is added to 'struct bpf_func_proto',
    which is used by verifier code to check (if defined) on whether
    the helper is called from allowed function (from whitelist).

Currently it's needed and implemented only for d_path helper,
but it's easy to add for another helper.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/asm-generic/vmlinux.lds.h |  5 +++++
 include/linux/bpf.h               |  1 +
 kernel/bpf/verifier.c             |  5 +++++
 kernel/trace/bpf_trace.c          | 23 +++++++++++++++++++++++
 4 files changed, 34 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 71e387a5fe90..6f7ca4f36ed7 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -631,6 +631,11 @@
 		__start_BTF = .;					\
 		*(.BTF)							\
 		__stop_BTF = .;						\
+	}								\
+	.BTF_whitelist : AT(ADDR(.BTF_whitelist) - LOAD_OFFSET) {	\
+		__start_BTF_whitelist_d_path = .;			\
+		*(.BTF_whitelist_d_path)				\
+		__stop_BTF_whitelist_d_path = .;			\
 	}
 #else
 #define BTF
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index bc589cdd8c34..ccacf488429f 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -275,6 +275,7 @@ struct bpf_func_proto {
 		enum bpf_arg_type arg_type[5];
 	};
 	int *btf_id; /* BTF ids of arguments */
+	bool (*allowed)(const struct bpf_prog *prog);
 };
 
 /* bpf_context is intentionally undefined structure. Pointer to bpf_context is
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b988df5ada20..55995de954a0 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4525,6 +4525,11 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
 		return -EINVAL;
 	}
 
+	if (fn->allowed && !fn->allowed(env->prog)) {
+		verbose(env, "helper call is not allowed in probe\n");
+		return -EINVAL;
+	}
+
 	/* With LD_ABS/IND some JITs save/restore skb from r1. */
 	changes_data = bpf_helper_changes_pkt_data(fn->func);
 	if (changes_data && fn->arg1_type != ARG_PTR_TO_CTX) {
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 3097ab04bdc4..55682d610534 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -13,6 +13,7 @@
 #include <linux/kprobes.h>
 #include <linux/syscalls.h>
 #include <linux/error-injection.h>
+#include <linux/bsearch.h>
 
 #include <asm/tlb.h>
 
@@ -797,6 +798,27 @@ BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz)
 	return len;
 }
 
+extern char __weak __start_BTF_whitelist_d_path[];
+extern char __weak __stop_BTF_whitelist_d_path[];
+
+static int btf_id_cmp_func(const void *a, const void *b)
+{
+	const unsigned long *pa = a;
+	const unsigned long *pb = b;
+
+	return *pa - *pb;
+}
+
+static bool bpf_d_path_allowed(const struct bpf_prog *prog)
+{
+	unsigned long *start = (unsigned long *) __start_BTF_whitelist_d_path;
+	unsigned long *stop  = (unsigned long *) __stop_BTF_whitelist_d_path;
+	unsigned long id = prog->aux->attach_btf_id;
+
+	return bsearch(&id, start, stop - start, sizeof(unsigned long),
+		       btf_id_cmp_func) != NULL;
+}
+
 static u32 bpf_d_path_btf_ids[3];
 static const struct bpf_func_proto bpf_d_path_proto = {
 	.func		= bpf_d_path,
@@ -806,6 +828,7 @@ static const struct bpf_func_proto bpf_d_path_proto = {
 	.arg2_type	= ARG_PTR_TO_MEM,
 	.arg3_type	= ARG_CONST_SIZE,
 	.btf_id		= bpf_d_path_btf_ids,
+	.allowed	= bpf_d_path_allowed,
 };
 
 const struct bpf_func_proto *
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 6/9] bpf: Compile bpfwl tool at kernel compilation start
  2020-05-06 13:29 [RFCv2 0/9] bpf: Add d_path helper Jiri Olsa
                   ` (4 preceding siblings ...)
  2020-05-06 13:29 ` [PATCH 5/9] bpf: Add support to check on BTF id whitelist for d_path helper Jiri Olsa
@ 2020-05-06 13:29 ` Jiri Olsa
  2020-05-14 22:38   ` Andrii Nakryiko
  2020-05-06 13:29 ` [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux Jiri Olsa
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2020-05-06 13:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Yonghong Song, Martin KaFai Lau, David Miller,
	John Fastabend, Jesper Dangaard Brouer, Wenbo Zhang, KP Singh,
	Andrii Nakryiko, bgregg, Florent Revest, Al Viro

The bpfwl tool will be used during the vmlinux linking,
so it's necessary it's ready.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 Makefile           | 21 +++++++++++++++++----
 tools/Makefile     |  3 +++
 tools/bpf/Makefile |  5 ++++-
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 49b2709ff44e..b0537af523dc 100644
--- a/Makefile
+++ b/Makefile
@@ -1017,9 +1017,10 @@ export mod_sign_cmd
 
 HOST_LIBELF_LIBS = $(shell pkg-config libelf --libs 2>/dev/null || echo -lelf)
 
+has_libelf = $(call try-run,\
+               echo "int main() {}" | $(HOSTCC) -xc -o /dev/null $(HOST_LIBELF_LIBS) -,1,0)
+
 ifdef CONFIG_STACK_VALIDATION
-  has_libelf := $(call try-run,\
-		echo "int main() {}" | $(HOSTCC) -xc -o /dev/null $(HOST_LIBELF_LIBS) -,1,0)
   ifeq ($(has_libelf),1)
     objtool_target := tools/objtool FORCE
   else
@@ -1028,6 +1029,14 @@ ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+ifdef CONFIG_DEBUG_INFO_BTF
+  ifeq ($(has_libelf),1)
+    bpfwl_target := tools/bpf/bpfwl FORCE
+  else
+    SKIP_BTF_WHITELIST_GENERATION := 1
+  endif
+endif
+
 PHONY += prepare0
 
 export MODORDER := $(extmod-prefix)modules.order
@@ -1141,7 +1150,7 @@ prepare0: archprepare
 	$(Q)$(MAKE) $(build)=.
 
 # All the preparing..
-prepare: prepare0 prepare-objtool
+prepare: prepare0 prepare-objtool prepare-bpfwl
 
 # Support for using generic headers in asm-generic
 asm-generic := -f $(srctree)/scripts/Makefile.asm-generic obj
@@ -1154,7 +1163,7 @@ uapi-asm-generic:
 	$(Q)$(MAKE) $(asm-generic)=arch/$(SRCARCH)/include/generated/uapi/asm \
 	generic=include/uapi/asm-generic
 
-PHONY += prepare-objtool
+PHONY += prepare-objtool prepare-bpfwl
 prepare-objtool: $(objtool_target)
 ifeq ($(SKIP_STACK_VALIDATION),1)
 ifdef CONFIG_UNWINDER_ORC
@@ -1165,6 +1174,10 @@ else
 endif
 endif
 
+prepare-bpfwl: $(bpfwl_target)
+ifeq ($(SKIP_BTF_WHITELIST_GENERATION),1)
+	@echo "warning: Cannot use BTF whitelist checks, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
+endif
 # Generate some files
 # ---------------------------------------------------------------------------
 
diff --git a/tools/Makefile b/tools/Makefile
index bd778812e915..85af6ebbce91 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -67,6 +67,9 @@ cpupower: FORCE
 cgroup firewire hv guest bootconfig spi usb virtio vm bpf iio gpio objtool leds wmi pci firmware debugging: FORCE
 	$(call descend,$@)
 
+bpf/%: FORCE
+	$(call descend,$@)
+
 liblockdep: FORCE
 	$(call descend,lib/lockdep)
 
diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index f897eeeb0b4f..d4ea2b5a2e58 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -124,5 +124,8 @@ runqslower_install:
 runqslower_clean:
 	$(call descend,runqslower,clean)
 
+bpfwl:
+	$(call descend,bpfwl)
+
 .PHONY: all install clean bpftool bpftool_install bpftool_clean \
-	runqslower runqslower_install runqslower_clean
+	runqslower runqslower_install runqslower_clean bpfwl
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux
  2020-05-06 13:29 [RFCv2 0/9] bpf: Add d_path helper Jiri Olsa
                   ` (5 preceding siblings ...)
  2020-05-06 13:29 ` [PATCH 6/9] bpf: Compile bpfwl tool at kernel compilation start Jiri Olsa
@ 2020-05-06 13:29 ` Jiri Olsa
  2020-05-13 18:29   ` Alexei Starovoitov
  2020-05-06 13:29 ` [PATCH 8/9] selftests/bpf: Add test for d_path helper Jiri Olsa
  2020-05-06 13:29 ` [PATCH 9/9] selftests/bpf: Add verifier " Jiri Olsa
  8 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2020-05-06 13:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Yonghong Song, Martin KaFai Lau, David Miller,
	John Fastabend, Jesper Dangaard Brouer, Wenbo Zhang, KP Singh,
	Andrii Nakryiko, bgregg, Florent Revest, Al Viro

Squeezing in the BTF id whitelist data into vmlinux object
with BTF section compiled in, with following steps:

  - generate whitelist data with bpfwl
    $ bpfwl .tmp_vmlinux.btf kernel/bpf/helpers-whitelist > ${whitelist}.c

  - compile whitelist.c
    $ gcc -c -o ${whitelist}.o ${whitelist}.c

  - keep only the whitelist data in ${whitelist}.o using objcopy

  - link .tmp_vmlinux.btf and ${whitelist}.o into $btf_vmlinux_bin_o}
    $ ld -r -o ${btf_vmlinux_bin_o} .tmp_vmlinux.btf ${whitelist}.o

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 Makefile                |  3 ++-
 scripts/link-vmlinux.sh | 20 +++++++++++++++-----
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index b0537af523dc..3bb995245592 100644
--- a/Makefile
+++ b/Makefile
@@ -437,6 +437,7 @@ OBJSIZE		= $(CROSS_COMPILE)size
 STRIP		= $(CROSS_COMPILE)strip
 endif
 PAHOLE		= pahole
+BPFWL		= $(srctree)/tools/bpf/bpfwl/bpfwl
 LEX		= flex
 YACC		= bison
 AWK		= awk
@@ -493,7 +494,7 @@ GCC_PLUGINS_CFLAGS :=
 CLANG_FLAGS :=
 
 export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC
-export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE LEX YACC AWK INSTALLKERNEL
+export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE BPFWL LEX YACC AWK INSTALLKERNEL
 export PERL PYTHON PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
 export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE
 
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index d09ab4afbda4..dee91c6bf450 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -130,16 +130,26 @@ gen_btf()
 	info "BTF" ${2}
 	LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1}
 
-	# Create ${2} which contains just .BTF section but no symbols. Add
+	# Create object which contains just .BTF section but no symbols. Add
 	# SHF_ALLOC because .BTF will be part of the vmlinux image. --strip-all
 	# deletes all symbols including __start_BTF and __stop_BTF, which will
 	# be redefined in the linker script. Add 2>/dev/null to suppress GNU
 	# objcopy warnings: "empty loadable segment detected at ..."
 	${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
-		--strip-all ${1} ${2} 2>/dev/null
-	# Change e_type to ET_REL so that it can be used to link final vmlinux.
-	# Unlike GNU ld, lld does not allow an ET_EXEC input.
-	printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none
+		--strip-all ${1} 2>/dev/null
+
+	# Create object that contains just .BTF_whitelist_* sections generated
+	# by bpfwl. Same as BTF section, BTF_whitelist_* data will be part of
+	# the vmlinux image, hence SHF_ALLOC.
+	whitelist=.btf.vmlinux.whitelist
+
+	${BPFWL} ${1} kernel/bpf/helpers-whitelist > ${whitelist}.c
+	${CC} -c -o ${whitelist}.o ${whitelist}.c
+	${OBJCOPY} --only-section=.BTF_whitelist* --set-section-flags .BTF=alloc,readonly \
+                --strip-all ${whitelist}.o 2>/dev/null
+
+	# Link BTF and BTF_whitelist objects together
+	${LD} -r -o ${2} ${1} ${whitelist}.o
 }
 
 # Create ${2} .o file with all symbols from the ${1} object file
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 8/9] selftests/bpf: Add test for d_path helper
  2020-05-06 13:29 [RFCv2 0/9] bpf: Add d_path helper Jiri Olsa
                   ` (6 preceding siblings ...)
  2020-05-06 13:29 ` [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux Jiri Olsa
@ 2020-05-06 13:29 ` Jiri Olsa
  2020-05-14 22:48   ` Andrii Nakryiko
  2020-05-06 13:29 ` [PATCH 9/9] selftests/bpf: Add verifier " Jiri Olsa
  8 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2020-05-06 13:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Wenbo Zhang, netdev, bpf, Yonghong Song, Martin KaFai Lau,
	David Miller, John Fastabend, Jesper Dangaard Brouer, KP Singh,
	Andrii Nakryiko, bgregg, Florent Revest, Al Viro

Adding test for d_path helper which is pretty much
copied from Wenbo Zhang's test for bpf_get_fd_path,
which never made it in.

I've failed so far to compile the test with <linux/fs.h>
kernel header, so for now adding 'struct file' with f_path
member that has same offset as kernel's file object.

Original-patch-by: Wenbo Zhang <ethercflow@gmail.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../testing/selftests/bpf/prog_tests/d_path.c | 196 ++++++++++++++++++
 .../testing/selftests/bpf/progs/test_d_path.c |  71 +++++++
 2 files changed, 267 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/d_path.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_d_path.c

diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
new file mode 100644
index 000000000000..6b69bfda6c19
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <test_progs.h>
+#include <sys/stat.h>
+#include <linux/sched.h>
+#include <sys/syscall.h>
+
+#define MAX_PATH_LEN		128
+#define MAX_FILES		7
+#define MAX_EVENT_NUM		16
+
+static struct d_path_test_data {
+	pid_t pid;
+	__u32 cnt_stat;
+	__u32 cnt_close;
+	char paths_stat[MAX_EVENT_NUM][MAX_PATH_LEN];
+	char paths_close[MAX_EVENT_NUM][MAX_PATH_LEN];
+} dst;
+
+static struct {
+	__u32 cnt;
+	char paths[MAX_EVENT_NUM][MAX_PATH_LEN];
+} src;
+
+static int set_pathname(int fd, pid_t pid)
+{
+	char buf[MAX_PATH_LEN];
+
+	snprintf(buf, MAX_PATH_LEN, "/proc/%d/fd/%d", pid, fd);
+	return readlink(buf, src.paths[src.cnt++], MAX_PATH_LEN);
+}
+
+static int trigger_fstat_events(pid_t pid)
+{
+	int sockfd = -1, procfd = -1, devfd = -1;
+	int localfd = -1, indicatorfd = -1;
+	int pipefd[2] = { -1, -1 };
+	struct stat fileStat;
+	int ret = -1;
+
+	/* unmountable pseudo-filesystems */
+	if (CHECK_FAIL(pipe(pipefd) < 0))
+		return ret;
+	/* unmountable pseudo-filesystems */
+	sockfd = socket(AF_INET, SOCK_STREAM, 0);
+	if (CHECK_FAIL(sockfd < 0))
+		goto out_close;
+	/* mountable pseudo-filesystems */
+	procfd = open("/proc/self/comm", O_RDONLY);
+	if (CHECK_FAIL(procfd < 0))
+		goto out_close;
+	devfd = open("/dev/urandom", O_RDONLY);
+	if (CHECK_FAIL(devfd < 0))
+		goto out_close;
+	localfd = open("/tmp/d_path_loadgen.txt", O_CREAT | O_RDONLY);
+	if (CHECK_FAIL(localfd < 0))
+		goto out_close;
+	/* bpf_d_path will return path with (deleted) */
+	remove("/tmp/d_path_loadgen.txt");
+	indicatorfd = open("/tmp/", O_PATH);
+	if (CHECK_FAIL(indicatorfd < 0))
+		goto out_close;
+
+	ret = set_pathname(pipefd[0], pid);
+	if (CHECK_FAIL(ret < 0))
+		goto out_close;
+	ret = set_pathname(pipefd[1], pid);
+	if (CHECK_FAIL(ret < 0))
+		goto out_close;
+	ret = set_pathname(sockfd, pid);
+	if (CHECK_FAIL(ret < 0))
+		goto out_close;
+	ret = set_pathname(procfd, pid);
+	if (CHECK_FAIL(ret < 0))
+		goto out_close;
+	ret = set_pathname(devfd, pid);
+	if (CHECK_FAIL(ret < 0))
+		goto out_close;
+	ret = set_pathname(localfd, pid);
+	if (CHECK_FAIL(ret < 0))
+		goto out_close;
+	ret = set_pathname(indicatorfd, pid);
+	if (CHECK_FAIL(ret < 0))
+		goto out_close;
+
+	/* triggers vfs_getattr */
+	fstat(pipefd[0], &fileStat);
+	fstat(pipefd[1], &fileStat);
+	fstat(sockfd, &fileStat);
+	fstat(procfd, &fileStat);
+	fstat(devfd, &fileStat);
+	fstat(localfd, &fileStat);
+	fstat(indicatorfd, &fileStat);
+
+out_close:
+	/* triggers filp_close */
+	close(pipefd[0]);
+	close(pipefd[1]);
+	close(sockfd);
+	close(procfd);
+	close(devfd);
+	close(localfd);
+	close(indicatorfd);
+	return ret;
+}
+
+void test_d_path(void)
+{
+	const char *prog_name_1 = "fentry/vfs_getattr";
+	const char *prog_name_2 = "fentry/filp_close";
+	const char *obj_file = "test_d_path.o";
+	int err, results_map_fd, duration = 0;
+	struct bpf_program *tp_prog1 = NULL;
+	struct bpf_program *tp_prog2 = NULL;
+	struct bpf_link *tp_link1 = NULL;
+	struct bpf_link *tp_link2 = NULL;
+	struct bpf_object *obj = NULL;
+	const int zero = 0;
+
+	obj = bpf_object__open_file(obj_file, NULL);
+	if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
+		return;
+
+	err = bpf_object__load(obj);
+	if (CHECK(err, "obj_load", "err %d\n", err))
+		goto cleanup;
+
+	tp_prog1 = bpf_object__find_program_by_title(obj, prog_name_1);
+	if (CHECK(!tp_prog1, "find_tp",
+		  "prog '%s' not found\n", prog_name_1))
+		goto cleanup;
+
+	tp_prog2 = bpf_object__find_program_by_title(obj, prog_name_2);
+	if (CHECK(!tp_prog2, "find_tp",
+		  "prog '%s' not found\n", prog_name_2))
+		goto cleanup;
+
+	tp_link1 = bpf_program__attach_trace(tp_prog1);
+	if (CHECK(IS_ERR(tp_link1), "attach_tp",
+		  "err %ld\n", PTR_ERR(tp_link1))) {
+		tp_link1 = NULL;
+		goto cleanup;
+	}
+
+	tp_link2 = bpf_program__attach_trace(tp_prog2);
+	if (CHECK(IS_ERR(tp_link2), "attach_tp",
+		  "err %ld\n", PTR_ERR(tp_link2))) {
+		tp_link2 = NULL;
+		goto cleanup;
+	}
+
+	results_map_fd = bpf_find_map(__func__, obj, "test_d_p.bss");
+	if (CHECK(results_map_fd < 0, "find_bss_map",
+		  "err %d\n", results_map_fd))
+		goto cleanup;
+
+	dst.pid = getpid();
+	err = bpf_map_update_elem(results_map_fd, &zero, &dst, 0);
+	if (CHECK(err, "update_elem",
+		  "failed to set pid filter: %d\n", err))
+		goto cleanup;
+
+	err = trigger_fstat_events(dst.pid);
+	if (CHECK_FAIL(err < 0))
+		goto cleanup;
+
+	err = bpf_map_lookup_elem(results_map_fd, &zero, &dst);
+	if (CHECK(err, "get_results",
+		  "failed to get results: %d\n", err))
+		goto cleanup;
+
+	for (int i = 0; i < MAX_FILES; i++) {
+		if (i < 3) {
+			CHECK((dst.paths_stat[i][0] == 0), "d_path",
+			      "failed to filter fs [%d]: %s vs %s\n",
+			      i, src.paths[i], dst.paths_stat[i]);
+			CHECK((dst.paths_close[i][0] == 0), "d_path",
+			      "failed to filter fs [%d]: %s vs %s\n",
+			      i, src.paths[i], dst.paths_close[i]);
+		} else {
+			CHECK(strncmp(src.paths[i], dst.paths_stat[i], MAX_PATH_LEN),
+			      "d_path",
+			      "failed to get stat path[%d]: %s vs %s\n",
+			      i, src.paths[i], dst.paths_stat[i]);
+			CHECK(strncmp(src.paths[i], dst.paths_close[i], MAX_PATH_LEN),
+			      "d_path",
+			      "failed to get close path[%d]: %s vs %s\n",
+			      i, src.paths[i], dst.paths_close[i]);
+		}
+	}
+
+cleanup:
+	bpf_link__destroy(tp_link2);
+	bpf_link__destroy(tp_link1);
+	bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c
new file mode 100644
index 000000000000..f75c108a5773
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_d_path.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <linux/ptrace.h>
+#include <linux/fs.h>
+#include <string.h>
+#include <unistd.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <linux/fs.h>
+
+#define MAX_PATH_LEN		128
+#define MAX_EVENT_NUM		16
+
+static struct d_path_test_data {
+	pid_t pid;
+	__u32 cnt_stat;
+	__u32 cnt_close;
+	char paths_stat[MAX_EVENT_NUM][MAX_PATH_LEN];
+	char paths_close[MAX_EVENT_NUM][MAX_PATH_LEN];
+} data;
+
+struct path;
+struct kstat;
+
+SEC("fentry/vfs_getattr")
+int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
+	     __u32 request_mask, unsigned int query_flags)
+{
+	pid_t pid = bpf_get_current_pid_tgid() >> 32;
+
+	if (pid != data.pid)
+		return 0;
+
+	if (data.cnt_stat >= MAX_EVENT_NUM)
+		return 0;
+
+	bpf_d_path(path, data.paths_stat[data.cnt_stat], MAX_PATH_LEN);
+	data.cnt_stat++;
+	return 0;
+}
+
+/*
+ * TODO
+ * I've failed so far to compile the test with <linux/fs.h>
+ * kernel header, so for now adding 'struct file' with f_path
+ * member that has same offset as kernel's file object.
+ */
+struct file {
+	__u64	 foo1;
+	__u64	 foo2;
+	void	*f_path;
+};
+
+SEC("fentry/filp_close")
+int BPF_PROG(prog_close, struct file *file, void *id)
+{
+	pid_t pid = bpf_get_current_pid_tgid() >> 32;
+
+	if (pid != data.pid)
+		return 0;
+
+	if (data.cnt_close >= MAX_EVENT_NUM)
+		return 0;
+
+	bpf_d_path((struct path *) &file->f_path, data.paths_close[data.cnt_close], MAX_PATH_LEN);
+	data.cnt_close++;
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 9/9] selftests/bpf: Add verifier test for d_path helper
  2020-05-06 13:29 [RFCv2 0/9] bpf: Add d_path helper Jiri Olsa
                   ` (7 preceding siblings ...)
  2020-05-06 13:29 ` [PATCH 8/9] selftests/bpf: Add test for d_path helper Jiri Olsa
@ 2020-05-06 13:29 ` Jiri Olsa
  8 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2020-05-06 13:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: netdev, bpf, Yonghong Song, Martin KaFai Lau, David Miller,
	John Fastabend, Jesper Dangaard Brouer, Wenbo Zhang, KP Singh,
	Andrii Nakryiko, bgregg, Florent Revest, Al Viro

Adding verifier test for attaching tracing program and
calling d_path helper from within and testing that it's
allowed for dentry_open function and denied for 'd_path'
function with appropriate error.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/testing/selftests/bpf/test_verifier.c   | 13 ++++++-
 tools/testing/selftests/bpf/verifier/d_path.c | 37 +++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/verifier/d_path.c

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 21a1ce219c1c..1e38179f0dbf 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -114,6 +114,7 @@ struct bpf_test {
 		bpf_testdata_struct_t retvals[MAX_TEST_RUNS];
 	};
 	enum bpf_attach_type expected_attach_type;
+	const char *kfunc;
 };
 
 /* Note we want this to be 64 bit aligned so that the end of our array is
@@ -961,8 +962,18 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
 		attr.log_level = 4;
 	attr.prog_flags = pflags;
 
+	if (prog_type == BPF_PROG_TYPE_TRACING && test->kfunc) {
+		attr.attach_btf_id = libbpf_find_vmlinux_btf_id(test->kfunc,
+						attr.expected_attach_type);
+	}
+
 	fd_prog = bpf_load_program_xattr(&attr, bpf_vlog, sizeof(bpf_vlog));
-	if (fd_prog < 0 && !bpf_probe_prog_type(prog_type, 0)) {
+
+	/* BPF_PROG_TYPE_TRACING requires more setup and
+	 * bpf_probe_prog_type won't give correct answer
+	 */
+	if (fd_prog < 0 && (prog_type != BPF_PROG_TYPE_TRACING) &&
+	    !bpf_probe_prog_type(prog_type, 0)) {
 		printf("SKIP (unsupported program type %d)\n", prog_type);
 		skips++;
 		goto close_fds;
diff --git a/tools/testing/selftests/bpf/verifier/d_path.c b/tools/testing/selftests/bpf/verifier/d_path.c
new file mode 100644
index 000000000000..b988396379a7
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/d_path.c
@@ -0,0 +1,37 @@
+{
+	"d_path accept",
+	.insns = {
+	BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_1, 0),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+	BPF_MOV64_IMM(BPF_REG_6, 0),
+	BPF_STX_MEM(BPF_DW, BPF_REG_2, BPF_REG_6, 0),
+	BPF_LD_IMM64(BPF_REG_3, 8),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_d_path),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+	.prog_type = BPF_PROG_TYPE_TRACING,
+	.expected_attach_type = BPF_TRACE_FENTRY,
+	.kfunc = "dentry_open",
+},
+{
+	"d_path reject",
+	.insns = {
+	BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_1, 0),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+	BPF_MOV64_IMM(BPF_REG_6, 0),
+	BPF_STX_MEM(BPF_DW, BPF_REG_2, BPF_REG_6, 0),
+	BPF_LD_IMM64(BPF_REG_3, 8),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_d_path),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.errstr = "helper call is not allowed in probe",
+	.result = REJECT,
+	.prog_type = BPF_PROG_TYPE_TRACING,
+	.expected_attach_type = BPF_TRACE_FENTRY,
+	.kfunc = "d_path",
+},
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux
  2020-05-06 13:29 ` [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux Jiri Olsa
@ 2020-05-13 18:29   ` Alexei Starovoitov
  2020-05-14  8:05     ` Jiri Olsa
  0 siblings, 1 reply; 28+ messages in thread
From: Alexei Starovoitov @ 2020-05-13 18:29 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, netdev, bpf, Yonghong Song,
	Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	bgregg, Florent Revest, Al Viro

On Wed, May 06, 2020 at 03:29:44PM +0200, Jiri Olsa wrote:
> Squeezing in the BTF id whitelist data into vmlinux object
> with BTF section compiled in, with following steps:
> 
>   - generate whitelist data with bpfwl
>     $ bpfwl .tmp_vmlinux.btf kernel/bpf/helpers-whitelist > ${whitelist}.c
> 
>   - compile whitelist.c
>     $ gcc -c -o ${whitelist}.o ${whitelist}.c
> 
>   - keep only the whitelist data in ${whitelist}.o using objcopy
> 
>   - link .tmp_vmlinux.btf and ${whitelist}.o into $btf_vmlinux_bin_o}
>     $ ld -r -o ${btf_vmlinux_bin_o} .tmp_vmlinux.btf ${whitelist}.o
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  Makefile                |  3 ++-
>  scripts/link-vmlinux.sh | 20 +++++++++++++++-----
>  2 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index b0537af523dc..3bb995245592 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -437,6 +437,7 @@ OBJSIZE		= $(CROSS_COMPILE)size
>  STRIP		= $(CROSS_COMPILE)strip
>  endif
>  PAHOLE		= pahole
> +BPFWL		= $(srctree)/tools/bpf/bpfwl/bpfwl
>  LEX		= flex
>  YACC		= bison
>  AWK		= awk
> @@ -493,7 +494,7 @@ GCC_PLUGINS_CFLAGS :=
>  CLANG_FLAGS :=
>  
>  export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC
> -export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE LEX YACC AWK INSTALLKERNEL
> +export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE BPFWL LEX YACC AWK INSTALLKERNEL
>  export PERL PYTHON PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
>  export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE
>  
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index d09ab4afbda4..dee91c6bf450 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -130,16 +130,26 @@ gen_btf()
>  	info "BTF" ${2}
>  	LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1}
>  
> -	# Create ${2} which contains just .BTF section but no symbols. Add
> +	# Create object which contains just .BTF section but no symbols. Add
>  	# SHF_ALLOC because .BTF will be part of the vmlinux image. --strip-all
>  	# deletes all symbols including __start_BTF and __stop_BTF, which will
>  	# be redefined in the linker script. Add 2>/dev/null to suppress GNU
>  	# objcopy warnings: "empty loadable segment detected at ..."
>  	${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
> -		--strip-all ${1} ${2} 2>/dev/null
> -	# Change e_type to ET_REL so that it can be used to link final vmlinux.
> -	# Unlike GNU ld, lld does not allow an ET_EXEC input.
> -	printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none
> +		--strip-all ${1} 2>/dev/null
> +
> +	# Create object that contains just .BTF_whitelist_* sections generated
> +	# by bpfwl. Same as BTF section, BTF_whitelist_* data will be part of
> +	# the vmlinux image, hence SHF_ALLOC.
> +	whitelist=.btf.vmlinux.whitelist
> +
> +	${BPFWL} ${1} kernel/bpf/helpers-whitelist > ${whitelist}.c
> +	${CC} -c -o ${whitelist}.o ${whitelist}.c
> +	${OBJCOPY} --only-section=.BTF_whitelist* --set-section-flags .BTF=alloc,readonly \
> +                --strip-all ${whitelist}.o 2>/dev/null
> +
> +	# Link BTF and BTF_whitelist objects together
> +	${LD} -r -o ${2} ${1} ${whitelist}.o

Thank you for working on it!
Looks great to me overall. In the next rev please drop RFC tag.

My only concern is this extra linking step. How many extra seconds does it add?

Also in patch 3:
+               func = func__find(str);
+               if (func)
+                       func->id = id;
which means that if somebody mistyped the name or that kernel function
got renamed there will be no warnings or errors.
I think it needs to fail the build instead.

If additional linking step takes another 20 seconds it could be a reason
to move the search to run-time.
We already have that with struct bpf_func_proto->btf_id[].
Whitelist could be something similar.
I think this mechanism will be reused for unstable helpers and other
func->btf_id mappings, so 'bpfwl' name would change eventually.
It's not white list specific. It generates a mapping of names to btf_ids.
Doing it at build time vs run-time is a trade off and it doesn't have
an obvious answer.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux
  2020-05-13 18:29   ` Alexei Starovoitov
@ 2020-05-14  8:05     ` Jiri Olsa
  2020-05-14 22:46       ` Andrii Nakryiko
  0 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2020-05-14  8:05 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, netdev, bpf,
	Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	bgregg, Florent Revest, Al Viro

On Wed, May 13, 2020 at 11:29:40AM -0700, Alexei Starovoitov wrote:

SNIP

> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index d09ab4afbda4..dee91c6bf450 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -130,16 +130,26 @@ gen_btf()
> >  	info "BTF" ${2}
> >  	LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1}
> >  
> > -	# Create ${2} which contains just .BTF section but no symbols. Add
> > +	# Create object which contains just .BTF section but no symbols. Add
> >  	# SHF_ALLOC because .BTF will be part of the vmlinux image. --strip-all
> >  	# deletes all symbols including __start_BTF and __stop_BTF, which will
> >  	# be redefined in the linker script. Add 2>/dev/null to suppress GNU
> >  	# objcopy warnings: "empty loadable segment detected at ..."
> >  	${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
> > -		--strip-all ${1} ${2} 2>/dev/null
> > -	# Change e_type to ET_REL so that it can be used to link final vmlinux.
> > -	# Unlike GNU ld, lld does not allow an ET_EXEC input.
> > -	printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none
> > +		--strip-all ${1} 2>/dev/null
> > +
> > +	# Create object that contains just .BTF_whitelist_* sections generated
> > +	# by bpfwl. Same as BTF section, BTF_whitelist_* data will be part of
> > +	# the vmlinux image, hence SHF_ALLOC.
> > +	whitelist=.btf.vmlinux.whitelist
> > +
> > +	${BPFWL} ${1} kernel/bpf/helpers-whitelist > ${whitelist}.c
> > +	${CC} -c -o ${whitelist}.o ${whitelist}.c
> > +	${OBJCOPY} --only-section=.BTF_whitelist* --set-section-flags .BTF=alloc,readonly \
> > +                --strip-all ${whitelist}.o 2>/dev/null
> > +
> > +	# Link BTF and BTF_whitelist objects together
> > +	${LD} -r -o ${2} ${1} ${whitelist}.o
> 
> Thank you for working on it!
> Looks great to me overall. In the next rev please drop RFC tag.
> 
> My only concern is this extra linking step. How many extra seconds does it add?

I did not meassure, but I haven't noticed any noticable delay,
I'll add meassurements to the next post

> 
> Also in patch 3:
> +               func = func__find(str);
> +               if (func)
> +                       func->id = id;
> which means that if somebody mistyped the name or that kernel function
> got renamed there will be no warnings or errors.
> I think it needs to fail the build instead.

it fails later on, when generating the array:

     if (!func->id) {
             fprintf(stderr, "FAILED: '%s' function not found in BTF data\n",
                     func->name);
             return -1;
     }

but it can clearly fail before that.. I'll change that

> 
> If additional linking step takes another 20 seconds it could be a reason
> to move the search to run-time.
> We already have that with struct bpf_func_proto->btf_id[].
> Whitelist could be something similar.
> I think this mechanism will be reused for unstable helpers and other
> func->btf_id mappings, so 'bpfwl' name would change eventually.
> It's not white list specific. It generates a mapping of names to btf_ids.
> Doing it at build time vs run-time is a trade off and it doesn't have
> an obvious answer.

I was thinking of putting the names in __init section and generate the BTF
ids on kernel start, but the build time generation seemed more convenient..
let's see the linking times with 'real size' whitelist and we can reconsider

thanks,
jirka


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/9] bpf: Add d_path helper
  2020-05-06 13:29 ` [PATCH 1/9] " Jiri Olsa
@ 2020-05-14 22:06   ` Andrii Nakryiko
  2020-05-15 14:59     ` Jiri Olsa
  0 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2020-05-14 22:06 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
	Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Wed, May 6, 2020 at 6:30 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding d_path helper function that returns full path
> for give 'struct path' object, which needs to be the
> kernel BTF 'path' object.
>
> The helper calls directly d_path function.
>
> Updating also bpf.h tools uapi header and adding
> 'path' to bpf_helpers_doc.py script.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  include/uapi/linux/bpf.h       | 14 +++++++++++++-
>  kernel/trace/bpf_trace.c       | 31 +++++++++++++++++++++++++++++++
>  scripts/bpf_helpers_doc.py     |  2 ++
>  tools/include/uapi/linux/bpf.h | 14 +++++++++++++-
>  4 files changed, 59 insertions(+), 2 deletions(-)
>

[...]

>
> +BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz)
> +{
> +       char *p = d_path(path, buf, sz - 1);
> +       int len;
> +
> +       if (IS_ERR(p)) {
> +               len = PTR_ERR(p);
> +       } else {
> +               len = strlen(p);
> +               if (len && p != buf) {
> +                       memmove(buf, p, len);
> +                       buf[len] = 0;
> +               }
> +       }
> +
> +       return len;
> +}
> +
> +static u32 bpf_d_path_btf_ids[3];

Using shorter than 5 element array is "unconventional", but seems like
btf_distill_func_proto will never access elements that are not
ARG_PTR_TO_BTF_ID, so it's fine. But than again, if we are saving
space, why not just 1-element array? :)


> +static const struct bpf_func_proto bpf_d_path_proto = {
> +       .func           = bpf_d_path,
> +       .gpl_only       = true,
> +       .ret_type       = RET_INTEGER,
> +       .arg1_type      = ARG_PTR_TO_BTF_ID,
> +       .arg2_type      = ARG_PTR_TO_MEM,
> +       .arg3_type      = ARG_CONST_SIZE,
> +       .btf_id         = bpf_d_path_btf_ids,
> +};
> +

[...]

> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index b3643e27e264..bc13cad27872 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -3068,6 +3068,17 @@ union bpf_attr {
>   *             See: clock_gettime(CLOCK_BOOTTIME)
>   *     Return
>   *             Current *ktime*.
> + *
> + * int bpf_d_path(struct path *path, char *buf, u32 sz)
> + *     Description
> + *             Return full path for given 'struct path' object, which
> + *             needs to be the kernel BTF 'path' object. The path is
> + *             returned in buffer provided 'buf' of size 'sz'.
> + *

Please specify if it's always zero-terminated string (especially on truncation).

> + *     Return
> + *             length of returned string on success, or a negative
> + *             error in case of failure
> + *
>   */
>  #define __BPF_FUNC_MAPPER(FN)          \
>         FN(unspec),                     \
> @@ -3195,7 +3206,8 @@ union bpf_attr {
>         FN(get_netns_cookie),           \
>         FN(get_current_ancestor_cgroup_id),     \
>         FN(sk_assign),                  \
> -       FN(ktime_get_boot_ns),
> +       FN(ktime_get_boot_ns),          \
> +       FN(d_path),
>
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> --
> 2.25.4
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/9] bpf: Add bpfwl tool to construct bpf whitelists
  2020-05-06 13:29 ` [PATCH 3/9] bpf: Add bpfwl tool to construct bpf whitelists Jiri Olsa
@ 2020-05-14 22:20   ` Andrii Nakryiko
  2020-05-15 14:58     ` Jiri Olsa
  0 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2020-05-14 22:20 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
	Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Wed, May 6, 2020 at 6:30 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> This tool takes vmlinux object and whitelist directory on input
> and produces C source object with BPF whitelist data.
>
> The vmlinux object needs to have a BTF information compiled in.
>
> The whitelist directory is expected to contain files with helper
> names, where each file contains list of functions/probes that
> helper is allowed to be called from - whitelist.
>
> The bpfwl tool has following output:
>
>   $ bpfwl vmlinux dir
>   unsigned long d_path[] __attribute__((section(".BTF_whitelist_d_path"))) = \
>   { 24507, 24511, 24537, 24539, 24545, 24588, 24602, 24920 };

why long instead of int? btf_id is 4-byte one.

>
> Each array are sorted BTF ids of the functions provided in the
> helper file.
>
> Each array will be compiled into kernel and used during the helper
> check in verifier.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/bpf/bpfwl/Build    |  11 ++
>  tools/bpf/bpfwl/Makefile |  60 +++++++++
>  tools/bpf/bpfwl/bpfwl.c  | 285 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 356 insertions(+)
>  create mode 100644 tools/bpf/bpfwl/Build
>  create mode 100644 tools/bpf/bpfwl/Makefile
>  create mode 100644 tools/bpf/bpfwl/bpfwl.c

Sorry, I didn't want to nitpick on naming, honestly, but I think this
is actually harmful in the long run. bpfwl is incomprehensible name,
anyone reading link script would be like "what the hell is bpfwl?" Why
not bpf_build_whitelist or something with "whitelist" spelled out in
full?

>
> diff --git a/tools/bpf/bpfwl/Build b/tools/bpf/bpfwl/Build
> new file mode 100644
> index 000000000000..667e30d6ce79
> --- /dev/null
> +++ b/tools/bpf/bpfwl/Build
> @@ -0,0 +1,11 @@
> +bpfwl-y += bpfwl.o
> +bpfwl-y += rbtree.o
> +bpfwl-y += zalloc.o
> +

[...]

> +
> +struct func {
> +       char                    *name;
> +       unsigned long            id;

as mentioned above, btf_id is 4 byte

> +       struct rb_node           rb_node;
> +       struct list_head         list[];
> +};
> +

[...]

> +       btf = btf__parse_elf(vmlinux, NULL);
> +       err = libbpf_get_error(btf);
> +       if (err) {
> +               fprintf(stderr, "FAILED: load BTF from %s: %s",
> +                       vmlinux, strerror(err));
> +               return -1;
> +       }
> +
> +       nr = btf__get_nr_types(btf);
> +
> +       /* Iterate all the BTF types and resolve all the function IDs. */
> +       for (id = 0; id < nr; id++) {

It has to be `for (id = 1; id <= nr; id++)`. 0 is VOID type and not
included into nr_types. I know it's confusing, but.. life :)

> +               const struct btf_type *type;
> +               struct func *func;
> +               const char *str;
> +
> +               type = btf__type_by_id(btf, id);
> +               if (!type)
> +                       continue;
> +

[...]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/9] bpf: Allow nested BTF object to be refferenced by BTF object + offset
  2020-05-06 13:29 ` [PATCH 4/9] bpf: Allow nested BTF object to be refferenced by BTF object + offset Jiri Olsa
@ 2020-05-14 22:32   ` Andrii Nakryiko
  0 siblings, 0 replies; 28+ messages in thread
From: Andrii Nakryiko @ 2020-05-14 22:32 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
	Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Wed, May 6, 2020 at 6:31 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding btf_struct_address function that takes 2 BTF objects
> and offset as arguments and checks wether object A is nested
> in object B on given offset.
>
> This function is be used when checking the helper function
> PTR_TO_BTF_ID arguments. If the argument has an offset value,
> the btf_struct_address will check if the final address is
> the expected BTF ID.
>
> This way we can access nested BTF objects under PTR_TO_BTF_ID
> pointer type and pass them to helpers, while they still point
> to valid kernel BTF objects.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  include/linux/bpf.h   |  3 ++
>  kernel/bpf/btf.c      | 69 +++++++++++++++++++++++++++++++++++++++++++
>  kernel/bpf/verifier.c | 32 +++++++++++++-------
>  3 files changed, 94 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 1262ec460ab3..bc589cdd8c34 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1213,6 +1213,9 @@ int btf_struct_access(struct bpf_verifier_log *log,
>                       const struct btf_type *t, int off, int size,
>                       enum bpf_access_type atype,
>                       u32 *next_btf_id);
> +int btf_struct_address(struct bpf_verifier_log *log,
> +                    const struct btf_type *t,
> +                    u32 off, u32 exp_id);
>  int btf_resolve_helper_id(struct bpf_verifier_log *log,
>                           const struct bpf_func_proto *fn, int);
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index a2cfba89a8e1..07f22469acab 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -4004,6 +4004,75 @@ int btf_struct_access(struct bpf_verifier_log *log,
>         return -EINVAL;
>  }
>
> +int btf_struct_address(struct bpf_verifier_log *log,
> +                      const struct btf_type *t,
> +                      u32 off, u32 exp_id)

The logic in this function is quite tricky and overlaps heavily with
btf_struct_access. You are already missing flexible array support that
Yonghong added recently. Let's see if it's possible to extract all
this "find field in a struct by offset" logic into reusable function?

> +{
> +       u32 i, moff, mtrue_end, msize = 0;
> +       const struct btf_member *member;
> +       const struct btf_type *mtype;
> +       const char *tname, *mname;
> +
> +again:
> +       tname = __btf_name_by_offset(btf_vmlinux, t->name_off);
> +       if (!btf_type_is_struct(t)) {
> +               bpf_log(log, "Type '%s' is not a struct\n", tname);
> +               return -EINVAL;
> +       }
> +
> +       if (off > t->size) {

>=, actually?

> +               bpf_log(log, "address beyond struct %s at off %u size %u\n",
> +                       tname, off, t->size);
> +               return -EACCES;
> +       }
> +

[...]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 70ad009577f8..b988df5ada20 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3665,6 +3665,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
>  {
>         struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[regno];
>         enum bpf_reg_type expected_type, type = reg->type;
> +       const struct btf_type *btf_type;
>         int err = 0;
>
>         if (arg_type == ARG_DONTCARE)
> @@ -3743,17 +3744,28 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
>                 expected_type = PTR_TO_BTF_ID;
>                 if (type != expected_type)
>                         goto err_type;
> -               if (reg->btf_id != meta->btf_id) {
> -                       verbose(env, "Helper has type %s got %s in R%d\n",
> -                               kernel_type_name(meta->btf_id),
> -                               kernel_type_name(reg->btf_id), regno);
> +               if (reg->off) {

well, off==0 can still point to (arbitrarily) nested structs that are
first member of outer struct... So I guess it would be better to
search for inner struct if btf_id is unexpected regardless of off
value?

> +                       btf_type = btf_type_by_id(btf_vmlinux, reg->btf_id);
> +                       if (btf_struct_address(&env->log, btf_type, reg->off, meta->btf_id)) {
> +                               verbose(env, "Helper has type %s got %s in R%d, off %d\n",
> +                                       kernel_type_name(meta->btf_id),
> +                                       kernel_type_name(reg->btf_id), regno, reg->off);
>
> -                       return -EACCES;
> -               }
> -               if (!tnum_is_const(reg->var_off) || reg->var_off.value || reg->off) {
> -                       verbose(env, "R%d is a pointer to in-kernel struct with non-zero offset\n",
> -                               regno);
> -                       return -EACCES;
> +                               return -EACCES;
> +                       }
> +               } else {
> +                       if (reg->btf_id != meta->btf_id) {
> +                               verbose(env, "Helper has type %s got %s in R%d\n",
> +                                       kernel_type_name(meta->btf_id),
> +                                       kernel_type_name(reg->btf_id), regno);
> +
> +                               return -EACCES;
> +                       }
> +                       if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
> +                               verbose(env, "R%d is a pointer to in-kernel struct with non-zero offset\n",
> +                                       regno);
> +                               return -EACCES;
> +                       }
>                 }
>         } else if (arg_type == ARG_PTR_TO_SPIN_LOCK) {
>                 if (meta->func_id == BPF_FUNC_spin_lock) {
> --
> 2.25.4
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 6/9] bpf: Compile bpfwl tool at kernel compilation start
  2020-05-06 13:29 ` [PATCH 6/9] bpf: Compile bpfwl tool at kernel compilation start Jiri Olsa
@ 2020-05-14 22:38   ` Andrii Nakryiko
  2020-05-15 14:57     ` Jiri Olsa
  0 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2020-05-14 22:38 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
	Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Wed, May 6, 2020 at 6:31 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> The bpfwl tool will be used during the vmlinux linking,
> so it's necessary it's ready.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  Makefile           | 21 +++++++++++++++++----
>  tools/Makefile     |  3 +++
>  tools/bpf/Makefile |  5 ++++-
>  3 files changed, 24 insertions(+), 5 deletions(-)
>

[...]

>
> +prepare-bpfwl: $(bpfwl_target)
> +ifeq ($(SKIP_BTF_WHITELIST_GENERATION),1)
> +       @echo "warning: Cannot use BTF whitelist checks, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
> +endif

When we added BTF dedup and generation first time, we also made pahole
unavailability or any error during deduplication process an error. It
actually was very confusing to users and they often missed that BTF
generation didn't happen, but they would notice it only at runtime
(after a confusing debugging session).

So I wonder if it's better to make this an error instead? Just guard
whitelist generation on whether CONFIG_DEBUG_INFO_BTF is enabled or
not?

>  # Generate some files
>  # ---------------------------------------------------------------------------
>
> diff --git a/tools/Makefile b/tools/Makefile
> index bd778812e915..85af6ebbce91 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -67,6 +67,9 @@ cpupower: FORCE
>  cgroup firewire hv guest bootconfig spi usb virtio vm bpf iio gpio objtool leds wmi pci firmware debugging: FORCE
>         $(call descend,$@)
>
> +bpf/%: FORCE
> +       $(call descend,$@)
> +
>  liblockdep: FORCE
>         $(call descend,lib/lockdep)
>
> diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
> index f897eeeb0b4f..d4ea2b5a2e58 100644
> --- a/tools/bpf/Makefile
> +++ b/tools/bpf/Makefile
> @@ -124,5 +124,8 @@ runqslower_install:
>  runqslower_clean:
>         $(call descend,runqslower,clean)
>
> +bpfwl:
> +       $(call descend,bpfwl)
> +
>  .PHONY: all install clean bpftool bpftool_install bpftool_clean \
> -       runqslower runqslower_install runqslower_clean
> +       runqslower runqslower_install runqslower_clean bpfwl

what about install/clean subcommands? At least clean seems like a good idea?

> --
> 2.25.4
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux
  2020-05-14  8:05     ` Jiri Olsa
@ 2020-05-14 22:46       ` Andrii Nakryiko
  2020-05-15 14:57         ` Jiri Olsa
  2020-05-28 17:23         ` Jiri Olsa
  0 siblings, 2 replies; 28+ messages in thread
From: Andrii Nakryiko @ 2020-05-14 22:46 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Jiri Olsa, Alexei Starovoitov,
	Daniel Borkmann, Networking, bpf, Yonghong Song,
	Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Thu, May 14, 2020 at 1:05 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Wed, May 13, 2020 at 11:29:40AM -0700, Alexei Starovoitov wrote:
>
> SNIP
>
> > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > > index d09ab4afbda4..dee91c6bf450 100755
> > > --- a/scripts/link-vmlinux.sh
> > > +++ b/scripts/link-vmlinux.sh
> > > @@ -130,16 +130,26 @@ gen_btf()
> > >     info "BTF" ${2}
> > >     LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1}
> > >
> > > -   # Create ${2} which contains just .BTF section but no symbols. Add
> > > +   # Create object which contains just .BTF section but no symbols. Add
> > >     # SHF_ALLOC because .BTF will be part of the vmlinux image. --strip-all
> > >     # deletes all symbols including __start_BTF and __stop_BTF, which will
> > >     # be redefined in the linker script. Add 2>/dev/null to suppress GNU
> > >     # objcopy warnings: "empty loadable segment detected at ..."
> > >     ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
> > > -           --strip-all ${1} ${2} 2>/dev/null
> > > -   # Change e_type to ET_REL so that it can be used to link final vmlinux.
> > > -   # Unlike GNU ld, lld does not allow an ET_EXEC input.
> > > -   printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none
> > > +           --strip-all ${1} 2>/dev/null
> > > +
> > > +   # Create object that contains just .BTF_whitelist_* sections generated
> > > +   # by bpfwl. Same as BTF section, BTF_whitelist_* data will be part of
> > > +   # the vmlinux image, hence SHF_ALLOC.
> > > +   whitelist=.btf.vmlinux.whitelist
> > > +
> > > +   ${BPFWL} ${1} kernel/bpf/helpers-whitelist > ${whitelist}.c
> > > +   ${CC} -c -o ${whitelist}.o ${whitelist}.c
> > > +   ${OBJCOPY} --only-section=.BTF_whitelist* --set-section-flags .BTF=alloc,readonly \
> > > +                --strip-all ${whitelist}.o 2>/dev/null
> > > +
> > > +   # Link BTF and BTF_whitelist objects together
> > > +   ${LD} -r -o ${2} ${1} ${whitelist}.o
> >
> > Thank you for working on it!
> > Looks great to me overall. In the next rev please drop RFC tag.
> >
> > My only concern is this extra linking step. How many extra seconds does it add?
>
> I did not meassure, but I haven't noticed any noticable delay,
> I'll add meassurements to the next post
>
> >
> > Also in patch 3:
> > +               func = func__find(str);
> > +               if (func)
> > +                       func->id = id;
> > which means that if somebody mistyped the name or that kernel function
> > got renamed there will be no warnings or errors.
> > I think it needs to fail the build instead.
>
> it fails later on, when generating the array:
>
>      if (!func->id) {
>              fprintf(stderr, "FAILED: '%s' function not found in BTF data\n",
>                      func->name);
>              return -1;
>      }
>
> but it can clearly fail before that.. I'll change that

I also means that whitelist can't contain functions that can be
conditionally compiled out, right? I guess we can invent some naming
convention to handle that, e.g: ?some_func will mean it's fine if we
didn't find it?

>
> >
> > If additional linking step takes another 20 seconds it could be a reason
> > to move the search to run-time.
> > We already have that with struct bpf_func_proto->btf_id[].
> > Whitelist could be something similar.
> > I think this mechanism will be reused for unstable helpers and other
> > func->btf_id mappings, so 'bpfwl' name would change eventually.
> > It's not white list specific. It generates a mapping of names to btf_ids.
> > Doing it at build time vs run-time is a trade off and it doesn't have
> > an obvious answer.
>
> I was thinking of putting the names in __init section and generate the BTF
> ids on kernel start, but the build time generation seemed more convenient..
> let's see the linking times with 'real size' whitelist and we can reconsider
>

Being able to record such places where to put BTF ID in code would be
really nice, as Alexei mentioned. There are many potential use cases
where it would be good to have BTF IDs just put into arbitrary
variables/arrays. This would trigger compilation error, if someone
screws up the name, or function is renamed, or if function can be
compiled out under some configuration. E.g., assuming some reasonable
implementation of the macro

static const u32 d_path_whitelist[] = {
    BTF_ID_FUNC(vfs_fallocate),
#ifdef CONFIG_WHATEVER
    BTF_ID_FUNC(do_truncate),
#endif
};

Would be nice and very explicit. Given this is not going to be sorted,
you won't be able to use binary search, but if whitelists are
generally small, it should be fine as is. If not, hashmap could be
built in runtime and would be, probably, faster than binary search for
longer sets of BTF IDs.

I wonder if we can do some assembly magic with generating extra
symbols and/or relocations to achieve this? What do you think? Is it
doable/desirable/better?


> thanks,
> jirka
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 8/9] selftests/bpf: Add test for d_path helper
  2020-05-06 13:29 ` [PATCH 8/9] selftests/bpf: Add test for d_path helper Jiri Olsa
@ 2020-05-14 22:48   ` Andrii Nakryiko
  2020-05-15 14:57     ` Jiri Olsa
  0 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2020-05-14 22:48 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Wenbo Zhang, Networking,
	bpf, Yonghong Song, Martin KaFai Lau, David Miller,
	John Fastabend, Jesper Dangaard Brouer, KP Singh,
	Andrii Nakryiko, Brendan Gregg, Florent Revest, Al Viro

On Wed, May 6, 2020 at 6:32 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding test for d_path helper which is pretty much
> copied from Wenbo Zhang's test for bpf_get_fd_path,
> which never made it in.
>
> I've failed so far to compile the test with <linux/fs.h>
> kernel header, so for now adding 'struct file' with f_path
> member that has same offset as kernel's file object.
>

Switch to using vmlinux.h? It would also be nice to use skeletons
instead of bpf_object__xxx API.

> Original-patch-by: Wenbo Zhang <ethercflow@gmail.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  .../testing/selftests/bpf/prog_tests/d_path.c | 196 ++++++++++++++++++
>  .../testing/selftests/bpf/progs/test_d_path.c |  71 +++++++
>  2 files changed, 267 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/d_path.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_d_path.c
>

[...]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 8/9] selftests/bpf: Add test for d_path helper
  2020-05-14 22:48   ` Andrii Nakryiko
@ 2020-05-15 14:57     ` Jiri Olsa
  0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2020-05-15 14:57 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Wenbo Zhang,
	Networking, bpf, Yonghong Song, Martin KaFai Lau, David Miller,
	John Fastabend, Jesper Dangaard Brouer, KP Singh,
	Andrii Nakryiko, Brendan Gregg, Florent Revest, Al Viro

On Thu, May 14, 2020 at 03:48:20PM -0700, Andrii Nakryiko wrote:
> On Wed, May 6, 2020 at 6:32 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding test for d_path helper which is pretty much
> > copied from Wenbo Zhang's test for bpf_get_fd_path,
> > which never made it in.
> >
> > I've failed so far to compile the test with <linux/fs.h>
> > kernel header, so for now adding 'struct file' with f_path
> > member that has same offset as kernel's file object.
> >
> 
> Switch to using vmlinux.h? It would also be nice to use skeletons
> instead of bpf_object__xxx API.

I just spot it this week when checking for something else.. yep, nice ;-)

thanks,
jirka

> 
> > Original-patch-by: Wenbo Zhang <ethercflow@gmail.com>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  .../testing/selftests/bpf/prog_tests/d_path.c | 196 ++++++++++++++++++
> >  .../testing/selftests/bpf/progs/test_d_path.c |  71 +++++++
> >  2 files changed, 267 insertions(+)
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/d_path.c
> >  create mode 100644 tools/testing/selftests/bpf/progs/test_d_path.c
> >
> 
> [...]
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux
  2020-05-14 22:46       ` Andrii Nakryiko
@ 2020-05-15 14:57         ` Jiri Olsa
  2020-05-28 17:23         ` Jiri Olsa
  1 sibling, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2020-05-15 14:57 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Jiri Olsa, Alexei Starovoitov,
	Daniel Borkmann, Networking, bpf, Yonghong Song,
	Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Thu, May 14, 2020 at 03:46:26PM -0700, Andrii Nakryiko wrote:
> On Thu, May 14, 2020 at 1:05 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Wed, May 13, 2020 at 11:29:40AM -0700, Alexei Starovoitov wrote:
> >
> > SNIP
> >
> > > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > > > index d09ab4afbda4..dee91c6bf450 100755
> > > > --- a/scripts/link-vmlinux.sh
> > > > +++ b/scripts/link-vmlinux.sh
> > > > @@ -130,16 +130,26 @@ gen_btf()
> > > >     info "BTF" ${2}
> > > >     LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1}
> > > >
> > > > -   # Create ${2} which contains just .BTF section but no symbols. Add
> > > > +   # Create object which contains just .BTF section but no symbols. Add
> > > >     # SHF_ALLOC because .BTF will be part of the vmlinux image. --strip-all
> > > >     # deletes all symbols including __start_BTF and __stop_BTF, which will
> > > >     # be redefined in the linker script. Add 2>/dev/null to suppress GNU
> > > >     # objcopy warnings: "empty loadable segment detected at ..."
> > > >     ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
> > > > -           --strip-all ${1} ${2} 2>/dev/null
> > > > -   # Change e_type to ET_REL so that it can be used to link final vmlinux.
> > > > -   # Unlike GNU ld, lld does not allow an ET_EXEC input.
> > > > -   printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none
> > > > +           --strip-all ${1} 2>/dev/null
> > > > +
> > > > +   # Create object that contains just .BTF_whitelist_* sections generated
> > > > +   # by bpfwl. Same as BTF section, BTF_whitelist_* data will be part of
> > > > +   # the vmlinux image, hence SHF_ALLOC.
> > > > +   whitelist=.btf.vmlinux.whitelist
> > > > +
> > > > +   ${BPFWL} ${1} kernel/bpf/helpers-whitelist > ${whitelist}.c
> > > > +   ${CC} -c -o ${whitelist}.o ${whitelist}.c
> > > > +   ${OBJCOPY} --only-section=.BTF_whitelist* --set-section-flags .BTF=alloc,readonly \
> > > > +                --strip-all ${whitelist}.o 2>/dev/null
> > > > +
> > > > +   # Link BTF and BTF_whitelist objects together
> > > > +   ${LD} -r -o ${2} ${1} ${whitelist}.o
> > >
> > > Thank you for working on it!
> > > Looks great to me overall. In the next rev please drop RFC tag.
> > >
> > > My only concern is this extra linking step. How many extra seconds does it add?
> >
> > I did not meassure, but I haven't noticed any noticable delay,
> > I'll add meassurements to the next post
> >
> > >
> > > Also in patch 3:
> > > +               func = func__find(str);
> > > +               if (func)
> > > +                       func->id = id;
> > > which means that if somebody mistyped the name or that kernel function
> > > got renamed there will be no warnings or errors.
> > > I think it needs to fail the build instead.
> >
> > it fails later on, when generating the array:
> >
> >      if (!func->id) {
> >              fprintf(stderr, "FAILED: '%s' function not found in BTF data\n",
> >                      func->name);
> >              return -1;
> >      }
> >
> > but it can clearly fail before that.. I'll change that
> 
> I also means that whitelist can't contain functions that can be
> conditionally compiled out, right? I guess we can invent some naming
> convention to handle that, e.g: ?some_func will mean it's fine if we
> didn't find it?

right.. I did not think of functions which won't be compiled in
because of disabled config options, in that case build falsly fails 

> 
> >
> > >
> > > If additional linking step takes another 20 seconds it could be a reason
> > > to move the search to run-time.
> > > We already have that with struct bpf_func_proto->btf_id[].
> > > Whitelist could be something similar.
> > > I think this mechanism will be reused for unstable helpers and other
> > > func->btf_id mappings, so 'bpfwl' name would change eventually.
> > > It's not white list specific. It generates a mapping of names to btf_ids.
> > > Doing it at build time vs run-time is a trade off and it doesn't have
> > > an obvious answer.
> >
> > I was thinking of putting the names in __init section and generate the BTF
> > ids on kernel start, but the build time generation seemed more convenient..
> > let's see the linking times with 'real size' whitelist and we can reconsider
> >
> 
> Being able to record such places where to put BTF ID in code would be
> really nice, as Alexei mentioned. There are many potential use cases
> where it would be good to have BTF IDs just put into arbitrary
> variables/arrays. This would trigger compilation error, if someone
> screws up the name, or function is renamed, or if function can be
> compiled out under some configuration. E.g., assuming some reasonable
> implementation of the macro
> 
> static const u32 d_path_whitelist[] = {
>     BTF_ID_FUNC(vfs_fallocate),
> #ifdef CONFIG_WHATEVER
>     BTF_ID_FUNC(do_truncate),
> #endif
> };
> 
> Would be nice and very explicit. Given this is not going to be sorted,
> you won't be able to use binary search, but if whitelists are
> generally small, it should be fine as is. If not, hashmap could be
> built in runtime and would be, probably, faster than binary search for
> longer sets of BTF IDs.
> 
> I wonder if we can do some assembly magic with generating extra
> symbols and/or relocations to achieve this? What do you think? Is it
> doable/desirable/better?

so assuming this is doable bpfwl could be a generic tool for both
whitelist and bpf_func_proto->btf_id cases

and we would solve the issue with missing function due to disable CONFIG

and the name could change to something event more generic ;-)

sounds like good idea ;-)

I'll check and see if I can find some reasonable way for BTF_ID_FUNC

thanks,
jirka


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 6/9] bpf: Compile bpfwl tool at kernel compilation start
  2020-05-14 22:38   ` Andrii Nakryiko
@ 2020-05-15 14:57     ` Jiri Olsa
  0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2020-05-15 14:57 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
	Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Thu, May 14, 2020 at 03:38:57PM -0700, Andrii Nakryiko wrote:
> On Wed, May 6, 2020 at 6:31 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > The bpfwl tool will be used during the vmlinux linking,
> > so it's necessary it's ready.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  Makefile           | 21 +++++++++++++++++----
> >  tools/Makefile     |  3 +++
> >  tools/bpf/Makefile |  5 ++++-
> >  3 files changed, 24 insertions(+), 5 deletions(-)
> >
> 
> [...]
> 
> >
> > +prepare-bpfwl: $(bpfwl_target)
> > +ifeq ($(SKIP_BTF_WHITELIST_GENERATION),1)
> > +       @echo "warning: Cannot use BTF whitelist checks, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
> > +endif
> 
> When we added BTF dedup and generation first time, we also made pahole
> unavailability or any error during deduplication process an error. It
> actually was very confusing to users and they often missed that BTF
> generation didn't happen, but they would notice it only at runtime
> (after a confusing debugging session).
> 
> So I wonder if it's better to make this an error instead? Just guard
> whitelist generation on whether CONFIG_DEBUG_INFO_BTF is enabled or
> not?

ok, makes sense.. I'll let it fail if there's CONFIG_DEBUG_INFO_BTF
enabled and we'are missing libelf

> 
> >  # Generate some files
> >  # ---------------------------------------------------------------------------
> >
> > diff --git a/tools/Makefile b/tools/Makefile
> > index bd778812e915..85af6ebbce91 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -67,6 +67,9 @@ cpupower: FORCE
> >  cgroup firewire hv guest bootconfig spi usb virtio vm bpf iio gpio objtool leds wmi pci firmware debugging: FORCE
> >         $(call descend,$@)
> >
> > +bpf/%: FORCE
> > +       $(call descend,$@)
> > +
> >  liblockdep: FORCE
> >         $(call descend,lib/lockdep)
> >
> > diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
> > index f897eeeb0b4f..d4ea2b5a2e58 100644
> > --- a/tools/bpf/Makefile
> > +++ b/tools/bpf/Makefile
> > @@ -124,5 +124,8 @@ runqslower_install:
> >  runqslower_clean:
> >         $(call descend,runqslower,clean)
> >
> > +bpfwl:
> > +       $(call descend,bpfwl)
> > +
> >  .PHONY: all install clean bpftool bpftool_install bpftool_clean \
> > -       runqslower runqslower_install runqslower_clean
> > +       runqslower runqslower_install runqslower_clean bpfwl
> 
> what about install/clean subcommands? At least clean seems like a good idea?

not sure about install, does not seem necessary for this tool,
but I'll add propagation of clean (it's defined in bpfwl already)

thanks,
jirka


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/9] bpf: Add bpfwl tool to construct bpf whitelists
  2020-05-14 22:20   ` Andrii Nakryiko
@ 2020-05-15 14:58     ` Jiri Olsa
  0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2020-05-15 14:58 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
	Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Thu, May 14, 2020 at 03:20:19PM -0700, Andrii Nakryiko wrote:
> On Wed, May 6, 2020 at 6:30 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > This tool takes vmlinux object and whitelist directory on input
> > and produces C source object with BPF whitelist data.
> >
> > The vmlinux object needs to have a BTF information compiled in.
> >
> > The whitelist directory is expected to contain files with helper
> > names, where each file contains list of functions/probes that
> > helper is allowed to be called from - whitelist.
> >
> > The bpfwl tool has following output:
> >
> >   $ bpfwl vmlinux dir
> >   unsigned long d_path[] __attribute__((section(".BTF_whitelist_d_path"))) = \
> >   { 24507, 24511, 24537, 24539, 24545, 24588, 24602, 24920 };
> 
> why long instead of int? btf_id is 4-byte one.

ok, int it is

> 
> >
> > Each array are sorted BTF ids of the functions provided in the
> > helper file.
> >
> > Each array will be compiled into kernel and used during the helper
> > check in verifier.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/bpf/bpfwl/Build    |  11 ++
> >  tools/bpf/bpfwl/Makefile |  60 +++++++++
> >  tools/bpf/bpfwl/bpfwl.c  | 285 +++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 356 insertions(+)
> >  create mode 100644 tools/bpf/bpfwl/Build
> >  create mode 100644 tools/bpf/bpfwl/Makefile
> >  create mode 100644 tools/bpf/bpfwl/bpfwl.c
> 
> Sorry, I didn't want to nitpick on naming, honestly, but I think this
> is actually harmful in the long run. bpfwl is incomprehensible name,
> anyone reading link script would be like "what the hell is bpfwl?" Why
> not bpf_build_whitelist or something with "whitelist" spelled out in
> full?

hum, will pick some more generic name

> 
> >
> > diff --git a/tools/bpf/bpfwl/Build b/tools/bpf/bpfwl/Build
> > new file mode 100644
> > index 000000000000..667e30d6ce79
> > --- /dev/null
> > +++ b/tools/bpf/bpfwl/Build
> > @@ -0,0 +1,11 @@
> > +bpfwl-y += bpfwl.o
> > +bpfwl-y += rbtree.o
> > +bpfwl-y += zalloc.o
> > +
> 
> [...]
> 
> > +
> > +struct func {
> > +       char                    *name;
> > +       unsigned long            id;
> 
> as mentioned above, btf_id is 4 byte

ok, changing to int

> 
> > +       struct rb_node           rb_node;
> > +       struct list_head         list[];
> > +};
> > +
> 
> [...]
> 
> > +       btf = btf__parse_elf(vmlinux, NULL);
> > +       err = libbpf_get_error(btf);
> > +       if (err) {
> > +               fprintf(stderr, "FAILED: load BTF from %s: %s",
> > +                       vmlinux, strerror(err));
> > +               return -1;
> > +       }
> > +
> > +       nr = btf__get_nr_types(btf);
> > +
> > +       /* Iterate all the BTF types and resolve all the function IDs. */
> > +       for (id = 0; id < nr; id++) {
> 
> It has to be `for (id = 1; id <= nr; id++)`. 0 is VOID type and not
> included into nr_types. I know it's confusing, but.. life :)

right, will change

thanks,
jirka

> 
> > +               const struct btf_type *type;
> > +               struct func *func;
> > +               const char *str;
> > +
> > +               type = btf__type_by_id(btf, id);
> > +               if (!type)
> > +                       continue;
> > +
> 
> [...]
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/9] bpf: Add d_path helper
  2020-05-14 22:06   ` Andrii Nakryiko
@ 2020-05-15 14:59     ` Jiri Olsa
  0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2020-05-15 14:59 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
	Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Thu, May 14, 2020 at 03:06:01PM -0700, Andrii Nakryiko wrote:
> On Wed, May 6, 2020 at 6:30 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding d_path helper function that returns full path
> > for give 'struct path' object, which needs to be the
> > kernel BTF 'path' object.
> >
> > The helper calls directly d_path function.
> >
> > Updating also bpf.h tools uapi header and adding
> > 'path' to bpf_helpers_doc.py script.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/uapi/linux/bpf.h       | 14 +++++++++++++-
> >  kernel/trace/bpf_trace.c       | 31 +++++++++++++++++++++++++++++++
> >  scripts/bpf_helpers_doc.py     |  2 ++
> >  tools/include/uapi/linux/bpf.h | 14 +++++++++++++-
> >  4 files changed, 59 insertions(+), 2 deletions(-)
> >
> 
> [...]
> 
> >
> > +BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz)
> > +{
> > +       char *p = d_path(path, buf, sz - 1);
> > +       int len;
> > +
> > +       if (IS_ERR(p)) {
> > +               len = PTR_ERR(p);
> > +       } else {
> > +               len = strlen(p);
> > +               if (len && p != buf) {
> > +                       memmove(buf, p, len);
> > +                       buf[len] = 0;
> > +               }
> > +       }
> > +
> > +       return len;
> > +}
> > +
> > +static u32 bpf_d_path_btf_ids[3];
> 
> Using shorter than 5 element array is "unconventional", but seems like
> btf_distill_func_proto will never access elements that are not
> ARG_PTR_TO_BTF_ID, so it's fine. But than again, if we are saving
> space, why not just 1-element array? :)

right, that can be actualy just 1 element array ;-)

> 
> 
> > +static const struct bpf_func_proto bpf_d_path_proto = {
> > +       .func           = bpf_d_path,
> > +       .gpl_only       = true,
> > +       .ret_type       = RET_INTEGER,
> > +       .arg1_type      = ARG_PTR_TO_BTF_ID,
> > +       .arg2_type      = ARG_PTR_TO_MEM,
> > +       .arg3_type      = ARG_CONST_SIZE,
> > +       .btf_id         = bpf_d_path_btf_ids,
> > +};
> > +
> 
> [...]
> 
> > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> > index b3643e27e264..bc13cad27872 100644
> > --- a/tools/include/uapi/linux/bpf.h
> > +++ b/tools/include/uapi/linux/bpf.h
> > @@ -3068,6 +3068,17 @@ union bpf_attr {
> >   *             See: clock_gettime(CLOCK_BOOTTIME)
> >   *     Return
> >   *             Current *ktime*.
> > + *
> > + * int bpf_d_path(struct path *path, char *buf, u32 sz)
> > + *     Description
> > + *             Return full path for given 'struct path' object, which
> > + *             needs to be the kernel BTF 'path' object. The path is
> > + *             returned in buffer provided 'buf' of size 'sz'.
> > + *
> 
> Please specify if it's always zero-terminated string (especially on truncation).

ok, will mention the zero termination in here

thanks,
jirka


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux
  2020-05-14 22:46       ` Andrii Nakryiko
  2020-05-15 14:57         ` Jiri Olsa
@ 2020-05-28 17:23         ` Jiri Olsa
  2020-05-29 20:48           ` Andrii Nakryiko
  1 sibling, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2020-05-28 17:23 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Jiri Olsa, Alexei Starovoitov,
	Daniel Borkmann, Networking, bpf, Yonghong Song,
	Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Thu, May 14, 2020 at 03:46:26PM -0700, Andrii Nakryiko wrote:

SNIP

> > I was thinking of putting the names in __init section and generate the BTF
> > ids on kernel start, but the build time generation seemed more convenient..
> > let's see the linking times with 'real size' whitelist and we can reconsider
> >
> 
> Being able to record such places where to put BTF ID in code would be
> really nice, as Alexei mentioned. There are many potential use cases
> where it would be good to have BTF IDs just put into arbitrary
> variables/arrays. This would trigger compilation error, if someone
> screws up the name, or function is renamed, or if function can be
> compiled out under some configuration. E.g., assuming some reasonable
> implementation of the macro

hi,
I'm struggling with this part.. to get some reasonable reference
to function/name into 32 bits? any idea? ;-)

jirka

> 
> static const u32 d_path_whitelist[] = {
>     BTF_ID_FUNC(vfs_fallocate),
> #ifdef CONFIG_WHATEVER
>     BTF_ID_FUNC(do_truncate),
> #endif
> };
> 
> Would be nice and very explicit. Given this is not going to be sorted,
> you won't be able to use binary search, but if whitelists are
> generally small, it should be fine as is. If not, hashmap could be
> built in runtime and would be, probably, faster than binary search for
> longer sets of BTF IDs.
> 
> I wonder if we can do some assembly magic with generating extra
> symbols and/or relocations to achieve this? What do you think? Is it
> doable/desirable/better?
> 
> 
> > thanks,
> > jirka
> >
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux
  2020-05-28 17:23         ` Jiri Olsa
@ 2020-05-29 20:48           ` Andrii Nakryiko
  2020-05-31 15:10             ` Jiri Olsa
  0 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2020-05-29 20:48 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Jiri Olsa, Alexei Starovoitov,
	Daniel Borkmann, Networking, bpf, Yonghong Song,
	Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Thu, May 28, 2020 at 10:24 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, May 14, 2020 at 03:46:26PM -0700, Andrii Nakryiko wrote:
>
> SNIP
>
> > > I was thinking of putting the names in __init section and generate the BTF
> > > ids on kernel start, but the build time generation seemed more convenient..
> > > let's see the linking times with 'real size' whitelist and we can reconsider
> > >
> >
> > Being able to record such places where to put BTF ID in code would be
> > really nice, as Alexei mentioned. There are many potential use cases
> > where it would be good to have BTF IDs just put into arbitrary
> > variables/arrays. This would trigger compilation error, if someone
> > screws up the name, or function is renamed, or if function can be
> > compiled out under some configuration. E.g., assuming some reasonable
> > implementation of the macro
>
> hi,
> I'm struggling with this part.. to get some reasonable reference
> to function/name into 32 bits? any idea? ;-)
>

Well, you don't have to store actual pointer, right? E.g, emitting
something like this in assembly:

.global __BTF_ID___some_function
.type __BTF_ID___some_function, @object
.size __BTF_ID___some_function, 4
__BTF_ID___some_function:
.zero  4

Would reserve 4 bytes and emit __BTF_ID___some_function symbol. If we
can then post-process vmlinux image and for all symbols starting with
__BTF_ID___ find some_function BTF type id and put it into those 4
bytes, that should work, no?

Maybe generalize it to __BTF_ID__{func,struct,typedef}__some_function,
whatever, not sure. Just an idea.


> jirka
>
> >
> > static const u32 d_path_whitelist[] = {
> >     BTF_ID_FUNC(vfs_fallocate),
> > #ifdef CONFIG_WHATEVER
> >     BTF_ID_FUNC(do_truncate),
> > #endif
> > };
> >
> > Would be nice and very explicit. Given this is not going to be sorted,
> > you won't be able to use binary search, but if whitelists are
> > generally small, it should be fine as is. If not, hashmap could be
> > built in runtime and would be, probably, faster than binary search for
> > longer sets of BTF IDs.
> >
> > I wonder if we can do some assembly magic with generating extra
> > symbols and/or relocations to achieve this? What do you think? Is it
> > doable/desirable/better?
> >
> >
> > > thanks,
> > > jirka
> > >
> >
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux
  2020-05-29 20:48           ` Andrii Nakryiko
@ 2020-05-31 15:10             ` Jiri Olsa
  2020-06-01 19:06               ` Andrii Nakryiko
  0 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2020-05-31 15:10 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Jiri Olsa, Alexei Starovoitov,
	Daniel Borkmann, Networking, bpf, Yonghong Song,
	Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Fri, May 29, 2020 at 01:48:58PM -0700, Andrii Nakryiko wrote:
> On Thu, May 28, 2020 at 10:24 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Thu, May 14, 2020 at 03:46:26PM -0700, Andrii Nakryiko wrote:
> >
> > SNIP
> >
> > > > I was thinking of putting the names in __init section and generate the BTF
> > > > ids on kernel start, but the build time generation seemed more convenient..
> > > > let's see the linking times with 'real size' whitelist and we can reconsider
> > > >
> > >
> > > Being able to record such places where to put BTF ID in code would be
> > > really nice, as Alexei mentioned. There are many potential use cases
> > > where it would be good to have BTF IDs just put into arbitrary
> > > variables/arrays. This would trigger compilation error, if someone
> > > screws up the name, or function is renamed, or if function can be
> > > compiled out under some configuration. E.g., assuming some reasonable
> > > implementation of the macro
> >
> > hi,
> > I'm struggling with this part.. to get some reasonable reference
> > to function/name into 32 bits? any idea? ;-)
> >
> 
> Well, you don't have to store actual pointer, right? E.g, emitting
> something like this in assembly:
> 
> .global __BTF_ID___some_function
> .type __BTF_ID___some_function, @object
> .size __BTF_ID___some_function, 4
> __BTF_ID___some_function:
> .zero  4
> 
> Would reserve 4 bytes and emit __BTF_ID___some_function symbol. If we
> can then post-process vmlinux image and for all symbols starting with
> __BTF_ID___ find some_function BTF type id and put it into those 4
> bytes, that should work, no?
> 
> Maybe generalize it to __BTF_ID__{func,struct,typedef}__some_function,
> whatever, not sure. Just an idea.

nice, so something like below?

it'd be in .S file, or perhaps in inline asm, assuming I'll be
able to pass macro arguments to asm("")

with externs defined in some header file:

  extern const int bpf_skb_output_btf_ids[];
  extern const int btf_whitelist_d_path[];

  $ objdump -x ./kernel/bpf/whitelist.o
  ...
  0000000000000000 l     O .data  0000000000000004 __BTF_ID__func__vfs_truncate
  0000000000000004 l     O .data  0000000000000004 __BTF_ID__func__vfs_fallocate
  0000000000000008 l     O .data  0000000000000004 __BTF_ID__func__krava
  0000000000000010 l     O .data  0000000000000004 __BTF_ID__struct__sk_buff
  0000000000000000 g       .data  0000000000000000 btf_whitelist_d_path
  0000000000000010 g       .data  0000000000000000 bpf_skb_output_btf_ids

also it'd be nice to get rid of BTF_ID__ symbols in the final link

thanks,
jirka


---
#define BTF_ID(prefix, name)                    \
.local __BTF_ID__##prefix##__##name;            \
.type __BTF_ID__##prefix##__##name, @object;    \
.size __BTF_ID__##prefix##__##name, 4;          \
__BTF_ID__##prefix##__##name:                   \
.zero 4

#define BTF_ID_LIST(name)                       \
.global name;                                   \
name:                

#define ZERO .zero 4

.data

BTF_ID_LIST(btf_whitelist_d_path)
BTF_ID(func, vfs_truncate)
BTF_ID(func, vfs_fallocate)
BTF_ID(func, krava)
ZERO

BTF_ID_LIST(bpf_skb_output_btf_ids)
BTF_ID(struct, sk_buff)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux
  2020-05-31 15:10             ` Jiri Olsa
@ 2020-06-01 19:06               ` Andrii Nakryiko
  2020-06-02  8:16                 ` Jiri Olsa
  0 siblings, 1 reply; 28+ messages in thread
From: Andrii Nakryiko @ 2020-06-01 19:06 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Jiri Olsa, Alexei Starovoitov,
	Daniel Borkmann, Networking, bpf, Yonghong Song,
	Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Sun, May 31, 2020 at 8:10 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, May 29, 2020 at 01:48:58PM -0700, Andrii Nakryiko wrote:
> > On Thu, May 28, 2020 at 10:24 AM Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > On Thu, May 14, 2020 at 03:46:26PM -0700, Andrii Nakryiko wrote:
> > >
> > > SNIP
> > >
> > > > > I was thinking of putting the names in __init section and generate the BTF
> > > > > ids on kernel start, but the build time generation seemed more convenient..
> > > > > let's see the linking times with 'real size' whitelist and we can reconsider
> > > > >
> > > >
> > > > Being able to record such places where to put BTF ID in code would be
> > > > really nice, as Alexei mentioned. There are many potential use cases
> > > > where it would be good to have BTF IDs just put into arbitrary
> > > > variables/arrays. This would trigger compilation error, if someone
> > > > screws up the name, or function is renamed, or if function can be
> > > > compiled out under some configuration. E.g., assuming some reasonable
> > > > implementation of the macro
> > >
> > > hi,
> > > I'm struggling with this part.. to get some reasonable reference
> > > to function/name into 32 bits? any idea? ;-)
> > >
> >
> > Well, you don't have to store actual pointer, right? E.g, emitting
> > something like this in assembly:
> >
> > .global __BTF_ID___some_function
> > .type __BTF_ID___some_function, @object
> > .size __BTF_ID___some_function, 4
> > __BTF_ID___some_function:
> > .zero  4
> >
> > Would reserve 4 bytes and emit __BTF_ID___some_function symbol. If we
> > can then post-process vmlinux image and for all symbols starting with
> > __BTF_ID___ find some_function BTF type id and put it into those 4
> > bytes, that should work, no?
> >
> > Maybe generalize it to __BTF_ID__{func,struct,typedef}__some_function,
> > whatever, not sure. Just an idea.
>
> nice, so something like below?
>
> it'd be in .S file, or perhaps in inline asm, assuming I'll be
> able to pass macro arguments to asm("")

I'd do inline asm, there are no arguments you need to pass into
asm("") itself, everything can be done through macro string
interpolation, I think. Having everything in .c file would be way more
convenient and obvious.

>
> with externs defined in some header file:
>
>   extern const int bpf_skb_output_btf_ids[];
>   extern const int btf_whitelist_d_path[];
>
>   $ objdump -x ./kernel/bpf/whitelist.o
>   ...
>   0000000000000000 l     O .data  0000000000000004 __BTF_ID__func__vfs_truncate
>   0000000000000004 l     O .data  0000000000000004 __BTF_ID__func__vfs_fallocate
>   0000000000000008 l     O .data  0000000000000004 __BTF_ID__func__krava
>   0000000000000010 l     O .data  0000000000000004 __BTF_ID__struct__sk_buff
>   0000000000000000 g       .data  0000000000000000 btf_whitelist_d_path
>   0000000000000010 g       .data  0000000000000000 bpf_skb_output_btf_ids
>
> also it'd be nice to get rid of BTF_ID__ symbols in the final link
>
> thanks,
> jirka
>
>
> ---
> #define BTF_ID(prefix, name)                    \
> .local __BTF_ID__##prefix##__##name;            \
> .type __BTF_ID__##prefix##__##name, @object;    \
> .size __BTF_ID__##prefix##__##name, 4;          \
> __BTF_ID__##prefix##__##name:                   \
> .zero 4
>
> #define BTF_ID_LIST(name)                       \
> .global name;                                   \
> name:
>
> #define ZERO .zero 4
>
> .data
>
> BTF_ID_LIST(btf_whitelist_d_path)
> BTF_ID(func, vfs_truncate)
> BTF_ID(func, vfs_fallocate)
> BTF_ID(func, krava)
> ZERO
>
> BTF_ID_LIST(bpf_skb_output_btf_ids)
> BTF_ID(struct, sk_buff)
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux
  2020-06-01 19:06               ` Andrii Nakryiko
@ 2020-06-02  8:16                 ` Jiri Olsa
  0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2020-06-02  8:16 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Jiri Olsa, Alexei Starovoitov,
	Daniel Borkmann, Networking, bpf, Yonghong Song,
	Martin KaFai Lau, David Miller, John Fastabend,
	Jesper Dangaard Brouer, Wenbo Zhang, KP Singh, Andrii Nakryiko,
	Brendan Gregg, Florent Revest, Al Viro

On Mon, Jun 01, 2020 at 12:06:34PM -0700, Andrii Nakryiko wrote:
> On Sun, May 31, 2020 at 8:10 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Fri, May 29, 2020 at 01:48:58PM -0700, Andrii Nakryiko wrote:
> > > On Thu, May 28, 2020 at 10:24 AM Jiri Olsa <jolsa@redhat.com> wrote:
> > > >
> > > > On Thu, May 14, 2020 at 03:46:26PM -0700, Andrii Nakryiko wrote:
> > > >
> > > > SNIP
> > > >
> > > > > > I was thinking of putting the names in __init section and generate the BTF
> > > > > > ids on kernel start, but the build time generation seemed more convenient..
> > > > > > let's see the linking times with 'real size' whitelist and we can reconsider
> > > > > >
> > > > >
> > > > > Being able to record such places where to put BTF ID in code would be
> > > > > really nice, as Alexei mentioned. There are many potential use cases
> > > > > where it would be good to have BTF IDs just put into arbitrary
> > > > > variables/arrays. This would trigger compilation error, if someone
> > > > > screws up the name, or function is renamed, or if function can be
> > > > > compiled out under some configuration. E.g., assuming some reasonable
> > > > > implementation of the macro
> > > >
> > > > hi,
> > > > I'm struggling with this part.. to get some reasonable reference
> > > > to function/name into 32 bits? any idea? ;-)
> > > >
> > >
> > > Well, you don't have to store actual pointer, right? E.g, emitting
> > > something like this in assembly:
> > >
> > > .global __BTF_ID___some_function
> > > .type __BTF_ID___some_function, @object
> > > .size __BTF_ID___some_function, 4
> > > __BTF_ID___some_function:
> > > .zero  4
> > >
> > > Would reserve 4 bytes and emit __BTF_ID___some_function symbol. If we
> > > can then post-process vmlinux image and for all symbols starting with
> > > __BTF_ID___ find some_function BTF type id and put it into those 4
> > > bytes, that should work, no?
> > >
> > > Maybe generalize it to __BTF_ID__{func,struct,typedef}__some_function,
> > > whatever, not sure. Just an idea.
> >
> > nice, so something like below?
> >
> > it'd be in .S file, or perhaps in inline asm, assuming I'll be
> > able to pass macro arguments to asm("")
> 
> I'd do inline asm, there are no arguments you need to pass into
> asm("") itself, everything can be done through macro string
> interpolation, I think. Having everything in .c file would be way more
> convenient and obvious.

wil will do it in inline asm

thanks,
jirka


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2020-06-02  8:16 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-06 13:29 [RFCv2 0/9] bpf: Add d_path helper Jiri Olsa
2020-05-06 13:29 ` [PATCH 1/9] " Jiri Olsa
2020-05-14 22:06   ` Andrii Nakryiko
2020-05-15 14:59     ` Jiri Olsa
2020-05-06 13:29 ` [PATCH 2/9] bpf: Add d_path whitelist Jiri Olsa
2020-05-06 13:29 ` [PATCH 3/9] bpf: Add bpfwl tool to construct bpf whitelists Jiri Olsa
2020-05-14 22:20   ` Andrii Nakryiko
2020-05-15 14:58     ` Jiri Olsa
2020-05-06 13:29 ` [PATCH 4/9] bpf: Allow nested BTF object to be refferenced by BTF object + offset Jiri Olsa
2020-05-14 22:32   ` Andrii Nakryiko
2020-05-06 13:29 ` [PATCH 5/9] bpf: Add support to check on BTF id whitelist for d_path helper Jiri Olsa
2020-05-06 13:29 ` [PATCH 6/9] bpf: Compile bpfwl tool at kernel compilation start Jiri Olsa
2020-05-14 22:38   ` Andrii Nakryiko
2020-05-15 14:57     ` Jiri Olsa
2020-05-06 13:29 ` [PATCH 7/9] bpf: Compile the BTF id whitelist data in vmlinux Jiri Olsa
2020-05-13 18:29   ` Alexei Starovoitov
2020-05-14  8:05     ` Jiri Olsa
2020-05-14 22:46       ` Andrii Nakryiko
2020-05-15 14:57         ` Jiri Olsa
2020-05-28 17:23         ` Jiri Olsa
2020-05-29 20:48           ` Andrii Nakryiko
2020-05-31 15:10             ` Jiri Olsa
2020-06-01 19:06               ` Andrii Nakryiko
2020-06-02  8:16                 ` Jiri Olsa
2020-05-06 13:29 ` [PATCH 8/9] selftests/bpf: Add test for d_path helper Jiri Olsa
2020-05-14 22:48   ` Andrii Nakryiko
2020-05-15 14:57     ` Jiri Olsa
2020-05-06 13:29 ` [PATCH 9/9] selftests/bpf: Add verifier " Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).