Merge branch 'bpf-get-rid-of-global-verifier-state-and-reuse-instruction-printer'

Jakub Kicinski says:

====================
bpf: get rid of global verifier state and reuse instruction printer

This set started off as simple extraction of eBPF verifier's instruction
printer into a separate file but evolved into removal of global state.
The purpose of moving instruction printing code is to be able to reuse it
from the bpftool.

As far as the global verifier lock goes, this set removes the global
variables relating to the log buffer, makes the one-time init done
by bpf_get_skb_set_tunnel_proto() not depend on any external locking,
and performs verifier log writeback as data is produced removing the need
for allocating a potentially large temporary buffer.

The final step of actually removing the verifier lock is left to someone
more competent and self-confident :)

Note that struct bpf_verifier_env is just 40B under two pages now,
we should probably switch to vzalloc() when it's expanded again...

v2:
 - add a selftest;
 - use env buffer and flush on every print (Alexei);
 - handle kernel log allocation failures (Daniel);
 - put the env log members into a struct (Daniel).
====================

Signed-off-by: David S. Miller <davem@davemloft.net>

6 files changed, 223 insertions, 22 deletions

diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 57fc4b9924ea..04d12f768f06 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -11,7 +11,7 @@ SYNOPSIS
 ========
 
 |	**bpftool** prog show [*PROG*]
-|	**bpftool** prog dump xlated *PROG*  file *FILE*
+|	**bpftool** prog dump xlated *PROG* [file *FILE*] [opcodes]
 |	**bpftool** prog dump jited  *PROG* [file *FILE*] [opcodes]
 |	**bpftool** prog pin *PROG* *FILE*
 |	**bpftool** prog help
@@ -28,9 +28,12 @@ DESCRIPTION
 		  Output will start with program ID followed by program type and
 		  zero or more named attributes (depending on kernel version).
 
-	**bpftool prog dump xlated** *PROG*  **file** *FILE*
-		  Dump eBPF instructions of the program from the kernel to a
-		  file.
+	**bpftool prog dump xlated** *PROG* [**file** *FILE*] [**opcodes**]
+		  Dump eBPF instructions of the program from the kernel.
+		  If *FILE* is specified image will be written to a file,
+		  otherwise it will be disassembled and printed to stdout.
+
+		  **opcodes** controls if raw opcodes will be printed.
 
 	**bpftool prog dump jited**  *PROG* [**file** *FILE*] [**opcodes**]
 		  Dump jited image (host machine code) of the program.
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 8705ee44664d..4f339824ca57 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -51,7 +51,7 @@ CC = gcc
 
 CFLAGS += -O2
 CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wshadow
-CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf
+CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf -I$(srctree)/kernel/bpf/
 LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
 
 include $(wildcard *.d)
@@ -59,7 +59,10 @@ include $(wildcard *.d)
 all: $(OUTPUT)bpftool
 
 SRCS=$(wildcard *.c)
-OBJS=$(patsubst %.c,$(OUTPUT)%.o,$(SRCS))
+OBJS=$(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
+
+$(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
+	$(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
 
 $(OUTPUT)bpftool: $(OBJS) $(LIBBPF)
 	$(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 85d2d7870a58..8e809b2bb311 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -36,11 +36,12 @@
 #ifndef __BPF_TOOL_H
 #define __BPF_TOOL_H
 
+/* BFD and kernel.h both define GCC_VERSION, differently */
+#undef GCC_VERSION
 #include <stdbool.h>
 #include <stdio.h>
 #include <linux/bpf.h>
-
-#define ARRAY_SIZE(a)	(sizeof(a) / sizeof(a[0]))
+#include <linux/kernel.h>
 
 #define err(msg...)	fprintf(stderr, "Error: " msg)
 #define warn(msg...)	fprintf(stderr, "Warning: " msg)
@@ -48,11 +49,6 @@
 
 #define ptr_to_u64(ptr)	((__u64)(unsigned long)(ptr))
 
-#define min(a, b)							\
-	({ typeof(a) _a = (a); typeof(b) _b = (b); _a > _b ? _b : _a; })
-#define max(a, b)							\
-	({ typeof(a) _a = (a); typeof(b) _b = (b); _a < _b ? _b : _a; })
-
 #define NEXT_ARG()	({ argc--; argv++; if (argc < 0) usage(); })
 #define NEXT_ARGP()	({ (*argc)--; (*argv)++; if (*argc < 0) usage(); })
 #define BAD_ARG()	({ err("what is '%s'?\n", *argv); -1; })
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 421ba89ce86a..9e2681c83717 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -35,6 +35,7 @@
 
 #include <errno.h>
 #include <fcntl.h>
+#include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -46,6 +47,7 @@
 #include <bpf.h>
 
 #include "main.h"
+#include "disasm.h"
 
 static const char * const prog_type_name[] = {
 	[BPF_PROG_TYPE_UNSPEC]		= "unspec",
@@ -297,11 +299,39 @@ static int do_show(int argc, char **argv)
 	return 0;
 }
 
+static void print_insn(struct bpf_verifier_env *env, const char *fmt, ...)
+{
+	va_list args;
+
+	va_start(args, fmt);
+	vprintf(fmt, args);
+	va_end(args);
+}
+
+static void dump_xlated(void *buf, unsigned int len, bool opcodes)
+{
+	struct bpf_insn *insn = buf;
+	unsigned int i;
+
+	for (i = 0; i < len / sizeof(*insn); i++) {
+		printf("% 4d: ", i);
+		print_bpf_insn(print_insn, NULL, insn + i, true);
+
+		if (opcodes) {
+			printf("       ");
+			print_hex(insn + i, 8, " ");
+			printf("\n");
+		}
+
+		if (insn[i].code == (BPF_LD | BPF_IMM | BPF_DW))
+			i++;
+	}
+}
+
 static int do_dump(int argc, char **argv)
 {
 	struct bpf_prog_info info = {};
 	__u32 len = sizeof(info);
-	bool can_disasm = false;
 	unsigned int buf_size;
 	char *filepath = NULL;
 	bool opcodes = false;
@@ -315,7 +345,6 @@ static int do_dump(int argc, char **argv)
 	if (is_prefix(*argv, "jited")) {
 		member_len = &info.jited_prog_len;
 		member_ptr = &info.jited_prog_insns;
-		can_disasm = true;
 	} else if (is_prefix(*argv, "xlated")) {
 		member_len = &info.xlated_prog_len;
 		member_ptr = &info.xlated_prog_insns;
@@ -346,10 +375,6 @@ static int do_dump(int argc, char **argv)
 		NEXT_ARG();
 	}
 
-	if (!filepath && !can_disasm) {
-		err("expected 'file' got %s\n", *argv);
-		return -1;
-	}
 	if (argc) {
 		usage();
 		return -1;
@@ -409,7 +434,10 @@ static int do_dump(int argc, char **argv)
 			goto err_free;
 		}
 	} else {
-		disasm_print_insn(buf, *member_len, opcodes);
+		if (member_len == &info.jited_prog_len)
+			disasm_print_insn(buf, *member_len, opcodes);
+		else
+			dump_xlated(buf, *member_len, opcodes);
 	}
 
 	free(buf);
@@ -430,7 +458,7 @@ static int do_help(int argc, char **argv)
 {
 	fprintf(stderr,
 		"Usage: %s %s show [PROG]\n"
-		"       %s %s dump xlated PROG  file FILE\n"
+		"       %s %s dump xlated PROG [file FILE] [opcodes]\n"
 		"       %s %s dump jited  PROG [file FILE] [opcodes]\n"
 		"       %s %s pin   PROG FILE\n"
 		"       %s %s help\n"
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 924af8d79bde..2e7880ea0add 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -12,7 +12,7 @@ CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../i
 LDLIBS += -lcap -lelf
 
 TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
-	test_align
+	test_align test_verifier_log
 
 TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test_obj_id.o \
 	test_pkt_md_access.o test_xdp_redirect.o test_xdp_meta.o sockmap_parse_prog.o     \
diff --git a/tools/testing/selftests/bpf/test_verifier_log.c b/tools/testing/selftests/bpf/test_verifier_log.c
new file mode 100644
index 000000000000..3cc0b561489e
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_verifier_log.c
@@ -0,0 +1,171 @@
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <linux/bpf.h>
+#include <linux/filter.h>
+#include <linux/unistd.h>
+
+#include <bpf/bpf.h>
+
+#define LOG_SIZE (1 << 20)
+
+#define err(str...)	printf("ERROR: " str)
+
+static const struct bpf_insn code_sample[] = {
+	/* We need a few instructions to pass the min log length */
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
+		     BPF_FUNC_map_lookup_elem),
+	BPF_EXIT_INSN(),
+};
+
+static inline __u64 ptr_to_u64(const void *ptr)
+{
+	return (__u64) (unsigned long) ptr;
+}
+
+static int load(char *log, size_t log_len, int log_level)
+{
+	union bpf_attr attr;
+
+	bzero(&attr, sizeof(attr));
+	attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
+	attr.insn_cnt = (__u32)(sizeof(code_sample) / sizeof(struct bpf_insn));
+	attr.insns = ptr_to_u64(code_sample);
+	attr.license = ptr_to_u64("GPL");
+	attr.log_buf = ptr_to_u64(log);
+	attr.log_size = log_len;
+	attr.log_level = log_level;
+
+	return syscall(__NR_bpf, BPF_PROG_LOAD, &attr, sizeof(attr));
+}
+
+static void check_ret(int ret, int exp_errno)
+{
+	if (ret > 0) {
+		close(ret);
+		err("broken sample loaded successfully!?\n");
+		exit(1);
+	}
+
+	if (!ret || errno != exp_errno) {
+		err("Program load returned: ret:%d/errno:%d, expected ret:%d/errno:%d\n",
+		    ret, errno, -1, exp_errno);
+		exit(1);
+	}
+}
+
+static void check_ones(const char *buf, size_t len, const char *msg)
+{
+	while (len--)
+		if (buf[len] != 1) {
+			err("%s", msg);
+			exit(1);
+		}
+}
+
+static void test_log_good(char *log, size_t buf_len, size_t log_len,
+			  size_t exp_len, int exp_errno, const char *full_log)
+{
+	size_t len;
+	int ret;
+
+	memset(log, 1, buf_len);
+
+	ret = load(log, log_len, 1);
+	check_ret(ret, exp_errno);
+
+	len = strnlen(log, buf_len);
+	if (len == buf_len) {
+		err("verifier did not NULL terminate the log\n");
+		exit(1);
+	}
+	if (exp_len && len != exp_len) {
+		err("incorrect log length expected:%zd have:%zd\n",
+		    exp_len, len);
+		exit(1);
+	}
+
+	if (strchr(log, 1)) {
+		err("verifier leaked a byte through\n");
+		exit(1);
+	}
+
+	check_ones(log + len + 1, buf_len - len - 1,
+		   "verifier wrote bytes past NULL termination\n");
+
+	if (memcmp(full_log, log, LOG_SIZE)) {
+		err("log did not match expected output\n");
+		exit(1);
+	}
+}
+
+static void test_log_bad(char *log, size_t log_len, int log_level)
+{
+	int ret;
+
+	ret = load(log, log_len, log_level);
+	check_ret(ret, EINVAL);
+	if (log)
+		check_ones(log, LOG_SIZE,
+			   "verifier touched log with bad parameters\n");
+}
+
+int main(int argc, char **argv)
+{
+	char full_log[LOG_SIZE];
+	char log[LOG_SIZE];
+	size_t want_len;
+	int i;
+
+	memset(log, 1, LOG_SIZE);
+
+	/* Test incorrect attr */
+	printf("Test log_level 0...\n");
+	test_log_bad(log, LOG_SIZE, 0);
+
+	printf("Test log_size < 128...\n");
+	test_log_bad(log, 15, 1);
+
+	printf("Test log_buff = NULL...\n");
+	test_log_bad(NULL, LOG_SIZE, 1);
+
+	/* Test with log big enough */
+	printf("Test oversized buffer...\n");
+	test_log_good(full_log, LOG_SIZE, LOG_SIZE, 0, EACCES, full_log);
+
+	want_len = strlen(full_log);
+
+	printf("Test exact buffer...\n");
+	test_log_good(log, LOG_SIZE, want_len + 2, want_len, EACCES, full_log);
+
+	printf("Test undersized buffers...\n");
+	for (i = 0; i < 64; i++) {
+		full_log[want_len - i + 1] = 1;
+		full_log[want_len - i] = 0;
+
+		test_log_good(log, LOG_SIZE, want_len + 1 - i, want_len - i,
+			      ENOSPC, full_log);
+	}
+
+	printf("test_verifier_log: OK\n");
+	return 0;
+}