From 9def249dc8409ffc1f5a1d7195f1c462f2b49c07 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Thu, 15 Aug 2019 15:32:15 +0100 Subject: [PATCH 1/6] tools: bpftool: fix arguments for p_err() in do_event_pipe() The last argument passed to some calls to the p_err() functions is not correct, it should be "*argv" instead of "**argv". This may lead to a segmentation fault error if CPU IDs or indices from the command line cannot be parsed correctly. Let's fix this. Fixes: f412eed9dfde ("tools: bpftool: add simple perf event output reader") Signed-off-by: Quentin Monnet Reviewed-by: Jakub Kicinski Signed-off-by: Alexei Starovoitov --- tools/bpf/bpftool/map_perf_ring.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/bpf/bpftool/map_perf_ring.c b/tools/bpf/bpftool/map_perf_ring.c index 3f108ab177973..4c5531d1a4500 100644 --- a/tools/bpf/bpftool/map_perf_ring.c +++ b/tools/bpf/bpftool/map_perf_ring.c @@ -157,7 +157,7 @@ int do_event_pipe(int argc, char **argv) NEXT_ARG(); ctx.cpu = strtoul(*argv, &endptr, 0); if (*endptr) { - p_err("can't parse %s as CPU ID", **argv); + p_err("can't parse %s as CPU ID", *argv); goto err_close_map; } @@ -168,7 +168,7 @@ int do_event_pipe(int argc, char **argv) NEXT_ARG(); ctx.idx = strtoul(*argv, &endptr, 0); if (*endptr) { - p_err("can't parse %s as index", **argv); + p_err("can't parse %s as index", *argv); goto err_close_map; } From 22c349e8db89df86804d3ba23cef037ccd44a8bf Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Thu, 15 Aug 2019 15:32:16 +0100 Subject: [PATCH 2/6] tools: bpftool: fix format strings and arguments for jsonw_printf() There are some mismatches between format strings and arguments passed to jsonw_printf() in the BTF dumper for bpftool, which seems harmless but may result in warnings if the "__printf()" attribute is used correctly for jsonw_printf(). Let's fix relevant format strings and type cast. Fixes: b12d6ec09730 ("bpf: btf: add btf print functionality") Signed-off-by: Quentin Monnet Reviewed-by: Jakub Kicinski Signed-off-by: Alexei Starovoitov --- tools/bpf/bpftool/btf_dumper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c index 8cafb9b314672..d66131f696892 100644 --- a/tools/bpf/bpftool/btf_dumper.c +++ b/tools/bpf/bpftool/btf_dumper.c @@ -26,9 +26,9 @@ static void btf_dumper_ptr(const void *data, json_writer_t *jw, bool is_plain_text) { if (is_plain_text) - jsonw_printf(jw, "%p", *(unsigned long *)data); + jsonw_printf(jw, "%p", data); else - jsonw_printf(jw, "%u", *(unsigned long *)data); + jsonw_printf(jw, "%lu", *(unsigned long *)data); } static int btf_dumper_modifier(const struct btf_dumper *d, __u32 type_id, @@ -216,7 +216,7 @@ static int btf_dumper_int(const struct btf_type *t, __u8 bit_offset, switch (BTF_INT_ENCODING(*int_type)) { case 0: if (BTF_INT_BITS(*int_type) == 64) - jsonw_printf(jw, "%lu", *(__u64 *)data); + jsonw_printf(jw, "%llu", *(__u64 *)data); else if (BTF_INT_BITS(*int_type) == 32) jsonw_printf(jw, "%u", *(__u32 *)data); else if (BTF_INT_BITS(*int_type) == 16) @@ -229,7 +229,7 @@ static int btf_dumper_int(const struct btf_type *t, __u8 bit_offset, break; case BTF_INT_SIGNED: if (BTF_INT_BITS(*int_type) == 64) - jsonw_printf(jw, "%ld", *(long long *)data); + jsonw_printf(jw, "%lld", *(long long *)data); else if (BTF_INT_BITS(*int_type) == 32) jsonw_printf(jw, "%d", *(int *)data); else if (BTF_INT_BITS(*int_type) == 16) From ed4a3983cd3eb93aaf80de8d8a36efed808acff2 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Thu, 15 Aug 2019 15:32:17 +0100 Subject: [PATCH 3/6] tools: bpftool: fix argument for p_err() in BTF do_dump() The last argument passed to one call to the p_err() function is not correct, it should be "*argv" instead of "**argv". This may lead to a segmentation fault error if BTF id cannot be parsed correctly. Let's fix this. Fixes: c93cc69004dt ("bpftool: add ability to dump BTF types") Signed-off-by: Quentin Monnet Reviewed-by: Jakub Kicinski Signed-off-by: Alexei Starovoitov --- tools/bpf/bpftool/btf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c index 1b8ec91899e6a..8805637f1a7e3 100644 --- a/tools/bpf/bpftool/btf.c +++ b/tools/bpf/bpftool/btf.c @@ -449,7 +449,7 @@ static int do_dump(int argc, char **argv) btf_id = strtoul(*argv, &endptr, 0); if (*endptr) { - p_err("can't parse %s as ID", **argv); + p_err("can't parse %s as ID", *argv); return -1; } NEXT_ARG(); From 8a15d5ced8c626c0331974c7281c1d651f7b0d83 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Thu, 15 Aug 2019 15:32:18 +0100 Subject: [PATCH 4/6] tools: bpftool: fix format string for p_err() in query_flow_dissector() The format string passed to one call to the p_err() function in query_flow_dissector() does not match the value that should be printed, resulting in some garbage integer being printed instead of strerror(errno) if /proc/self/ns/net cannot be open. Let's fix the format string. Fixes: 7f0c57fec80f ("bpftool: show flow_dissector attachment status") Signed-off-by: Quentin Monnet Reviewed-by: Jakub Kicinski Signed-off-by: Alexei Starovoitov --- tools/bpf/bpftool/net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c index a213a9b7f69c7..4f52d31516166 100644 --- a/tools/bpf/bpftool/net.c +++ b/tools/bpf/bpftool/net.c @@ -226,7 +226,7 @@ static int query_flow_dissector(struct bpf_attach_info *attach_info) fd = open("/proc/self/ns/net", O_RDONLY); if (fd < 0) { - p_err("can't open /proc/self/ns/net: %d", + p_err("can't open /proc/self/ns/net: %s", strerror(errno)); return -1; } From b0ead6d75a5b335287337e602e6b815e1115481c Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Thu, 15 Aug 2019 15:32:19 +0100 Subject: [PATCH 5/6] tools: bpftool: fix format string for p_err() in detect_common_prefix() There is one call to the p_err() function in detect_common_prefix() where the message to print is passed directly as the first argument, without using a format string. This is harmless, but may trigger warnings if the "__printf()" attribute is used correctly for the p_err() function. Let's fix it by using a "%s" format string. Fixes: ba95c7452439 ("tools: bpftool: add "prog run" subcommand to test-run programs") Signed-off-by: Quentin Monnet Reviewed-by: Jakub Kicinski Signed-off-by: Alexei Starovoitov --- tools/bpf/bpftool/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index e916ff25697f1..93d008687020c 100644 --- a/tools/bpf/bpftool/main.c +++ b/tools/bpf/bpftool/main.c @@ -139,7 +139,7 @@ int detect_common_prefix(const char *arg, ...) strncat(msg, "'", sizeof(msg) - strlen(msg) - 1); if (count >= 2) { - p_err(msg); + p_err("%s", msg); return -1; } From 8918dc42dc85ba6981028f65a989c478eb80bc02 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Thu, 15 Aug 2019 15:32:20 +0100 Subject: [PATCH 6/6] tools: bpftool: move "__printf()" attributes to header file Some functions in bpftool have a "__printf()" format attributes to tell the compiler they should expect printf()-like arguments. But because these attributes are not used for the function prototypes in the header files, the compiler does not run the checks everywhere the functions are used, and some mistakes on format string and corresponding arguments slipped in over time. Let's move the __printf() attributes to the correct places. Note: We add guards around the definition of GCC_VERSION in tools/include/linux/compiler-gcc.h to prevent a conflict in jit_disasm.c on GCC_VERSION from headers pulled via libbfd. Fixes: c101189bc968 ("tools: bpftool: fix -Wmissing declaration warnings") Reported-by: Jakub Kicinski Signed-off-by: Quentin Monnet Reviewed-by: Jakub Kicinski Signed-off-by: Alexei Starovoitov --- tools/bpf/bpftool/common.c | 4 ++-- tools/bpf/bpftool/json_writer.c | 6 ++---- tools/bpf/bpftool/json_writer.h | 6 ++++-- tools/bpf/bpftool/main.h | 4 ++-- tools/include/linux/compiler-gcc.h | 2 ++ 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c index 5215e0870bcba..a7036c70db485 100644 --- a/tools/bpf/bpftool/common.c +++ b/tools/bpf/bpftool/common.c @@ -29,7 +29,7 @@ #define BPF_FS_MAGIC 0xcafe4a11 #endif -void __printf(1, 2) p_err(const char *fmt, ...) +void p_err(const char *fmt, ...) { va_list ap; @@ -47,7 +47,7 @@ void __printf(1, 2) p_err(const char *fmt, ...) va_end(ap); } -void __printf(1, 2) p_info(const char *fmt, ...) +void p_info(const char *fmt, ...) { va_list ap; diff --git a/tools/bpf/bpftool/json_writer.c b/tools/bpf/bpftool/json_writer.c index 6046dcab51cc5..86501cd3c763e 100644 --- a/tools/bpf/bpftool/json_writer.c +++ b/tools/bpf/bpftool/json_writer.c @@ -15,7 +15,6 @@ #include #include #include -#include #include "json_writer.h" @@ -153,8 +152,7 @@ void jsonw_name(json_writer_t *self, const char *name) putc(' ', self->out); } -void __printf(2, 0) -jsonw_vprintf_enquote(json_writer_t *self, const char *fmt, va_list ap) +void jsonw_vprintf_enquote(json_writer_t *self, const char *fmt, va_list ap) { jsonw_eor(self); putc('"', self->out); @@ -162,7 +160,7 @@ jsonw_vprintf_enquote(json_writer_t *self, const char *fmt, va_list ap) putc('"', self->out); } -void __printf(2, 3) jsonw_printf(json_writer_t *self, const char *fmt, ...) +void jsonw_printf(json_writer_t *self, const char *fmt, ...) { va_list ap; diff --git a/tools/bpf/bpftool/json_writer.h b/tools/bpf/bpftool/json_writer.h index cb9a1993681c6..35cf1f00f96ca 100644 --- a/tools/bpf/bpftool/json_writer.h +++ b/tools/bpf/bpftool/json_writer.h @@ -14,6 +14,7 @@ #include #include #include +#include /* Opaque class structure */ typedef struct json_writer json_writer_t; @@ -30,8 +31,9 @@ void jsonw_pretty(json_writer_t *self, bool on); void jsonw_name(json_writer_t *self, const char *name); /* Add value */ -void jsonw_vprintf_enquote(json_writer_t *self, const char *fmt, va_list ap); -void jsonw_printf(json_writer_t *self, const char *fmt, ...); +void __printf(2, 0) jsonw_vprintf_enquote(json_writer_t *self, const char *fmt, + va_list ap); +void __printf(2, 3) jsonw_printf(json_writer_t *self, const char *fmt, ...); void jsonw_string(json_writer_t *self, const char *value); void jsonw_bool(json_writer_t *self, bool value); void jsonw_float(json_writer_t *self, double number); diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h index 7031a4bf87a02..af9ad56c303a1 100644 --- a/tools/bpf/bpftool/main.h +++ b/tools/bpf/bpftool/main.h @@ -98,8 +98,8 @@ extern int bpf_flags; extern struct pinned_obj_table prog_table; extern struct pinned_obj_table map_table; -void p_err(const char *fmt, ...); -void p_info(const char *fmt, ...); +void __printf(1, 2) p_err(const char *fmt, ...); +void __printf(1, 2) p_info(const char *fmt, ...); bool is_prefix(const char *pfx, const char *str); int detect_common_prefix(const char *arg, ...); diff --git a/tools/include/linux/compiler-gcc.h b/tools/include/linux/compiler-gcc.h index 0d35f18006a13..95c072b70d0e8 100644 --- a/tools/include/linux/compiler-gcc.h +++ b/tools/include/linux/compiler-gcc.h @@ -6,9 +6,11 @@ /* * Common definitions for all gcc versions go here. */ +#ifndef GCC_VERSION #define GCC_VERSION (__GNUC__ * 10000 \ + __GNUC_MINOR__ * 100 \ + __GNUC_PATCHLEVEL__) +#endif #if GCC_VERSION >= 70000 && !defined(__CHECKER__) # define __fallthrough __attribute__ ((fallthrough))