lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 6 Nov 2015 13:49:40 +0000
From:	Wang Nan <wangnan0@...wei.com>
To:	<acme@...nel.org>, <namhyung@...nel.org>
CC:	<lizefan@...wei.com>, <pi3orama@....com>,
	<linux-kernel@...r.kernel.org>, <jolsa@...nel.org>,
	Wang Nan <wangnan0@...wei.com>,
	Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: [PATCH v2 4/7] perf tools: Improve BPF related error messages output

A series of bpf loader related error code is introduced to help error
delivering. Functions are improved to return those new error code.
Functions which return pointers are adjusted to encode error code into
return value using "ERR_PTR".

bpf_loader_strerror() is improved to convert those error message to
string. It detected the value of error code and calls libbpf_strerror()
and strerror_r() accordingly, so caller don't need to consider checking
the range of error code.

In bpf__strerror_load(), print kernel version of running kernel and
the object's 'version' section to notify user how to fix his/her
program.

v1 -> v2:
 Use macro for error code.

 Fetch error message based on array index, eliminate for-loop.

 Print version strings.

Before:

# perf record -e ./test_kversion_nomatch_program.o sleep 1
event syntax error: './test_kversion_nomatch_program.o'
                     \___ Failed to load program: Validate your program and check 'license'/'version' sections in your object
[SKIP]

After:

# perf record -e ./test_kversion_nomatch_program.o ls
event syntax error: './test_kversion_nomatch_program.o'
                     \___ 'version' (4.4.0) doesn't match running kernel (4.3.0)
[SKIP]

Signed-off-by: Wang Nan <wangnan0@...wei.com>
Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
Cc: Namhyung Kim <namhyung@...nel.org>
---
 tools/perf/util/bpf-loader.c   | 92 +++++++++++++++++++++++++++++++++++-------
 tools/perf/util/bpf-loader.h   | 22 ++++++++++
 tools/perf/util/parse-events.c |  7 ++--
 tools/perf/util/util.h         |  5 +++
 4 files changed, 109 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index c46256b..19b496b 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -53,7 +53,7 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
 
 		err = llvm__compile_bpf(filename, &obj_buf, &obj_buf_sz);
 		if (err)
-			return ERR_PTR(err);
+			return ERR_PTR(-BPF_LOADER_ERRNO__COMPILE);
 		obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, filename);
 		free(obj_buf);
 	} else
@@ -113,14 +113,14 @@ config_bpf_program(struct bpf_program *prog)
 	if (err < 0) {
 		pr_debug("bpf: '%s' is not a valid config string\n",
 			 config_str);
-		err = -EINVAL;
+		err = -BPF_LOADER_ERRNO__CONFIG;
 		goto errout;
 	}
 
 	if (pev->group && strcmp(pev->group, PERF_BPF_PROBE_GROUP)) {
 		pr_debug("bpf: '%s': group for event is set and not '%s'.\n",
 			 config_str, PERF_BPF_PROBE_GROUP);
-		err = -EINVAL;
+		err = -BPF_LOADER_ERRNO__GROUP;
 		goto errout;
 	} else if (!pev->group)
 		pev->group = strdup(PERF_BPF_PROBE_GROUP);
@@ -132,9 +132,9 @@ config_bpf_program(struct bpf_program *prog)
 	}
 
 	if (!pev->event) {
-		pr_debug("bpf: '%s': event name is missing\n",
+		pr_debug("bpf: '%s': event name is missing. Section name should be 'key=value'\n",
 			 config_str);
-		err = -EINVAL;
+		err = -BPF_LOADER_ERRNO__EVENTNAME;
 		goto errout;
 	}
 	pr_debug("bpf: config '%s' is ok\n", config_str);
@@ -285,7 +285,7 @@ int bpf__foreach_tev(struct bpf_object *obj,
 				(void **)&priv);
 		if (err || !priv) {
 			pr_debug("bpf: failed to get private field\n");
-			return -EINVAL;
+			return -BPF_LOADER_ERRNO__INTERNAL;
 		}
 
 		pev = &priv->pev;
@@ -308,6 +308,18 @@ int bpf__foreach_tev(struct bpf_object *obj,
 	return 0;
 }
 
+#define ERRNO_OFFSET(e)		((e) - __BPF_LOADER_ERRNO__START)
+#define ERRCODE_OFFSET(c)	ERRNO_OFFSET(BPF_LOADER_ERRNO__##c)
+#define NR_ERRNO	(__BPF_LOADER_ERRNO__END - __BPF_LOADER_ERRNO__START)
+
+static const char *bpf_loader_strerror_table[NR_ERRNO] = {
+	[ERRCODE_OFFSET(CONFIG)]	= "Invalid config string",
+	[ERRCODE_OFFSET(GROUP)]		= "Invalid group name",
+	[ERRCODE_OFFSET(EVENTNAME)]	= "No event name found in config string",
+	[ERRCODE_OFFSET(INTERNAL)]	= "BPF loader internal error",
+	[ERRCODE_OFFSET(COMPILE)]	= "Error when compiling BPF scriptlet",
+};
+
 static int
 bpf_loader_strerror(int err, char *buf, size_t size)
 {
@@ -322,16 +334,27 @@ bpf_loader_strerror(int err, char *buf, size_t size)
 	if (err >= __LIBBPF_ERRNO__START)
 		return libbpf_strerror(err, buf, size);
 
-	msg = strerror_r(err, sbuf, sizeof(sbuf));
-	snprintf(buf, size, "%s", msg);
+	if (err >= __BPF_LOADER_ERRNO__START && err < __BPF_LOADER_ERRNO__END) {
+		msg = bpf_loader_strerror_table[ERRNO_OFFSET(err)];
+		snprintf(buf, size, "%s", msg);
+		buf[size - 1] = '\0';
+		return 0;
+	}
+
+	if (err >= __BPF_LOADER_ERRNO__END)
+		snprintf(buf, size, "Unknown bpf loader error %d", err);
+	else
+		snprintf(buf, size, "%s",
+			 strerror_r(err, sbuf, sizeof(sbuf)));
+
 	buf[size - 1] = '\0';
-	return 0;
+	return -1;
 }
 
 #define bpf__strerror_head(err, buf, size) \
 	char sbuf[STRERR_BUFSIZE], *emsg;\
-	if (!size)\
-		return 0;\
+if (!size)\
+return 0;\
 	if (err < 0)\
 		err = -err;\
 	bpf_loader_strerror(err, sbuf, sizeof(sbuf));\
@@ -351,21 +374,62 @@ bpf_loader_strerror(int err, char *buf, size_t size)
 	}\
 	buf[size - 1] = '\0';
 
+int bpf__strerror_prepare_load(const char *filename, bool source,
+			       int err, char *buf, size_t size)
+{
+	size_t n;
+	int ret;
+
+	n = snprintf(buf, size, "Failed to load %s%s: ",
+			 filename, source ? " from source" : "");
+	if (n >= size) {
+		buf[size - 1] = '\0';
+		return 0;
+	}
+	buf += n;
+	size -= n;
+
+	ret = bpf_loader_strerror(err, buf, size);
+	buf[size - 1] = '\0';
+	return ret;
+}
+
 int bpf__strerror_probe(struct bpf_object *obj __maybe_unused,
 			int err, char *buf, size_t size)
 {
 	bpf__strerror_head(err, buf, size);
 	bpf__strerror_entry(EEXIST, "Probe point exist. Try use 'perf probe -d \"*\"'");
-	bpf__strerror_entry(EPERM, "You need to be root, and /proc/sys/kernel/kptr_restrict should be 0\n");
-	bpf__strerror_entry(ENOENT, "You need to check probing points in BPF file\n");
+	bpf__strerror_entry(EACCES, "You need to be root");
+	bpf__strerror_entry(EPERM, "You need to be root, and /proc/sys/kernel/kptr_restrict should be 0");
+	bpf__strerror_entry(ENOENT, "You need to check probing points in BPF file");
 	bpf__strerror_end(buf, size);
 	return 0;
 }
 
-int bpf__strerror_load(struct bpf_object *obj __maybe_unused,
+int bpf__strerror_load(struct bpf_object *obj,
 		       int err, char *buf, size_t size)
 {
 	bpf__strerror_head(err, buf, size);
+	case LIBBPF_ERRNO__KVER: {
+		unsigned int obj_kver = bpf_object__get_kversion(obj);
+		unsigned int real_kver;
+
+		if (fetch_kernel_version(&real_kver, NULL, 0)) {
+			scnprintf(buf, size, "Unable to fetch kernel version");
+			break;
+		}
+
+		if (obj_kver != real_kver) {
+			scnprintf(buf, size,
+				  "'version' ("KVER_FMT") doesn't match running kernel ("KVER_FMT")",
+				  KVER_PARAM(obj_kver),
+				  KVER_PARAM(real_kver));
+			break;
+		}
+
+		scnprintf(buf, size, "Failed to load program for unknown reason");
+		break;
+	}
 	bpf__strerror_end(buf, size);
 	return 0;
 }
diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
index ccd8d7f..2c39710 100644
--- a/tools/perf/util/bpf-loader.h
+++ b/tools/perf/util/bpf-loader.h
@@ -8,9 +8,21 @@
 #include <linux/compiler.h>
 #include <linux/err.h>
 #include <string.h>
+#include <bpf/libbpf.h>
 #include "probe-event.h"
 #include "debug.h"
 
+enum bpf_loader_errno {
+	__BPF_LOADER_ERRNO__START = __LIBBPF_ERRNO__START - 100,
+	/* Invalid config string */
+	BPF_LOADER_ERRNO__CONFIG = __BPF_LOADER_ERRNO__START,
+	BPF_LOADER_ERRNO__GROUP,	/* Invalid group name */
+	BPF_LOADER_ERRNO__EVENTNAME,	/* Event name is missing */
+	BPF_LOADER_ERRNO__INTERNAL,	/* BPF loader internal error */
+	BPF_LOADER_ERRNO__COMPILE,	/* Error when compiling BPF scriptlet */
+	__BPF_LOADER_ERRNO__END,
+};
+
 struct bpf_object;
 #define PERF_BPF_PROBE_GROUP "perf_bpf_probe"
 
@@ -19,6 +31,8 @@ typedef int (*bpf_prog_iter_callback_t)(struct probe_trace_event *tev,
 
 #ifdef HAVE_LIBBPF_SUPPORT
 struct bpf_object *bpf__prepare_load(const char *filename, bool source);
+int bpf__strerror_prepare_load(const char *filename, bool source,
+			       int err, char *buf, size_t size);
 
 void bpf__clear(void);
 
@@ -67,6 +81,14 @@ __bpf_strerror(char *buf, size_t size)
 	return 0;
 }
 
+int bpf__strerror_prepare_load(const char *filename __maybe_unused,
+			       bool source __maybe_unused,
+			       int err __maybe_unused,
+			       char *buf, size_t size)
+{
+	return __bpf_strerror(buf, size);
+}
+
 static inline int
 bpf__strerror_probe(struct bpf_object *obj __maybe_unused,
 		    int err __maybe_unused,
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index c75b25d..e48d9da 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -642,9 +642,10 @@ int parse_events_load_bpf(struct parse_events_evlist *data,
 			snprintf(errbuf, sizeof(errbuf),
 				 "BPF support is not compiled");
 		else
-			snprintf(errbuf, sizeof(errbuf),
-				 "BPF object file '%s' is invalid",
-				 bpf_file_name);
+			bpf__strerror_prepare_load(bpf_file_name,
+						   source,
+						   -err, errbuf,
+						   sizeof(errbuf));
 
 		data->error->help = strdup("(add -v to see detail)");
 		data->error->str = strdup(errbuf);
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 2665126..dcc6590 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -352,5 +352,10 @@ int get_stack_size(const char *str, unsigned long *_size);
 
 int fetch_kernel_version(unsigned int *puint,
 			 char *str, size_t str_sz);
+#define KVER_VERSION(x)		(((x) >> 16) & 0xff)
+#define KVER_PATCHLEVEL(x)	(((x) >> 8) & 0xff)
+#define KVER_SUBLEVEL(x)	((x) & 0xff)
+#define KVER_FMT	"%d.%d.%d"
+#define KVER_PARAM(x)	KVER_VERSION(x), KVER_PATCHLEVEL(x), KVER_SUBLEVEL(x)
 
 #endif /* GIT_COMPAT_UTIL_H */
-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists