From fc96ec4d5d4155c61cbafd49fb2dd403c899a9f4 Mon Sep 17 00:00:00 2001 From: John Garry Date: Thu, 10 Jun 2021 22:32:59 +0800 Subject: perf metricgroup: Fix find_evsel_group() event selector The following command segfaults on my x86 broadwell: $ ./perf stat -M frontend_bound,retiring,backend_bound,bad_speculation sleep 1 WARNING: grouped events cpus do not match, disabling group: anon group { raw 0x10e } anon group { raw 0x10e } perf: util/evsel.c:1596: get_group_fd: Assertion `!(!leader->core.fd)' failed. Aborted (core dumped) The issue shows itself as a use-after-free in evlist__check_cpu_maps(), whereby the leader of an event selector (evsel) has been deleted (yet we still attempt to verify for an evsel). Fundamentally the problem comes from metricgroup__setup_events() -> find_evsel_group(), and has developed from the previous fix attempt in commit 9c880c24cb0d ("perf metricgroup: Fix for metrics containing duration_time"). The problem now is that the logic in checking if an evsel is in the same group is subtly broken for the "cycles" event. For the "cycles" event, the pmu_name is NULL; however the logic in find_evsel_group() may set an event matched against "cycles" as used, when it should not be. This leads to a condition where an evsel is set, yet its leader is not. Fix the check for evsel pmu_name by not matching evsels when either has a NULL pmu_name. There is still a pre-existing metric issue whereby the ordering of the metrics may break the 'stat' function, as discussed at: https://lore.kernel.org/lkml/49c6fccb-b716-1bf0-18a6-cace1cdb66b9@huawei.com/ Fixes: 9c880c24cb0d ("perf metricgroup: Fix for metrics containing duration_time") Signed-off-by: John Garry Tested-by: Arnaldo Carvalho de Melo # On a Thinkpad T450S Cc: Alexander Shishkin Cc: Ian Rogers Cc: Jiri Olsa Cc: Kajol Jain Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lore.kernel.org/lkml/1623335580-187317-2-git-send-email-john.garry@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/metricgroup.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c index 8336dd8e8098..c456fdeae06a 100644 --- a/tools/perf/util/metricgroup.c +++ b/tools/perf/util/metricgroup.c @@ -162,10 +162,10 @@ static bool contains_event(struct evsel **metric_events, int num_events, return false; } -static bool evsel_same_pmu(struct evsel *ev1, struct evsel *ev2) +static bool evsel_same_pmu_or_none(struct evsel *ev1, struct evsel *ev2) { if (!ev1->pmu_name || !ev2->pmu_name) - return false; + return true; return !strcmp(ev1->pmu_name, ev2->pmu_name); } @@ -288,7 +288,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist, */ if (!has_constraint && ev->leader != metric_events[i]->leader && - evsel_same_pmu(ev->leader, metric_events[i]->leader)) + evsel_same_pmu_or_none(ev->leader, metric_events[i]->leader)) break; if (!strcmp(metric_events[i]->name, ev->name)) { set_bit(ev->idx, evlist_used); -- cgit v1.2.3 From fe7a98b9d9b36e5c8a22d76b67d29721f153f66e Mon Sep 17 00:00:00 2001 From: John Garry Date: Thu, 10 Jun 2021 22:33:00 +0800 Subject: perf metricgroup: Return error code from metricgroup__add_metric_sys_event_iter() The error code is not set at all in the sys event iter function. This may lead to an uninitialized value of "ret" in metricgroup__add_metric() when no CPU metric is added. Fix by properly setting the error code. It is not necessary to init "ret" to 0 in metricgroup__add_metric(), as if we have no CPU or sys event metric matching, then "has_match" should be 0 and "ret" is set to -EINVAL. However gcc cannot detect that it may not have been set after the map_for_each_metric() loop for CPU metrics, which is strange. Fixes: be335ec28efa8 ("perf metricgroup: Support adding metrics for system PMUs") Signed-off-by: John Garry Acked-by: Ian Rogers Cc: Alexander Shishkin Cc: Jiri Olsa Cc: Kajol Jain Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lore.kernel.org/lkml/1623335580-187317-3-git-send-email-john.garry@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/metricgroup.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c index c456fdeae06a..d3cf2dee36c8 100644 --- a/tools/perf/util/metricgroup.c +++ b/tools/perf/util/metricgroup.c @@ -1073,16 +1073,18 @@ static int metricgroup__add_metric_sys_event_iter(struct pmu_event *pe, ret = add_metric(d->metric_list, pe, d->metric_no_group, &m, NULL, d->ids); if (ret) - return ret; + goto out; ret = resolve_metric(d->metric_no_group, d->metric_list, NULL, d->ids); if (ret) - return ret; + goto out; *(d->has_match) = true; - return *d->ret; +out: + *(d->ret) = ret; + return ret; } static int metricgroup__add_metric(const char *metric, bool metric_no_group, -- cgit v1.2.3 From c087e9480cf33672ef2c6cce4348d754988b8437 Mon Sep 17 00:00:00 2001 From: Riccardo Mancini Date: Sat, 12 Jun 2021 19:37:48 +0200 Subject: perf machine: Fix refcount usage when processing PERF_RECORD_KSYMBOL ASan reported a memory leak of BPF-related ksymbols map and dso. The leak is caused by refount never reaching 0, due to missing __put calls in the function machine__process_ksymbol_register. Once the dso is inserted in the map, dso__put() should be called (map__new2() increases the refcount to 2). The same thing applies for the map when it's inserted into maps (maps__insert() increases the refcount to 2). $ sudo ./perf record -- sleep 5 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.025 MB perf.data (8 samples) ] ================================================================= ==297735==ERROR: LeakSanitizer: detected memory leaks Direct leak of 6992 byte(s) in 19 object(s) allocated from: #0 0x4f43c7 in calloc (/home/user/linux/tools/perf/perf+0x4f43c7) #1 0x8e4e53 in map__new2 /home/user/linux/tools/perf/util/map.c:216:20 #2 0x8cf68c in machine__process_ksymbol_register /home/user/linux/tools/perf/util/machine.c:778:10 [...] Indirect leak of 8702 byte(s) in 19 object(s) allocated from: #0 0x4f43c7 in calloc (/home/user/linux/tools/perf/perf+0x4f43c7) #1 0x8728d7 in dso__new_id /home/user/linux/tools/perf/util/dso.c:1256:20 #2 0x872015 in dso__new /home/user/linux/tools/perf/util/dso.c:1295:9 #3 0x8cf623 in machine__process_ksymbol_register /home/user/linux/tools/perf/util/machine.c:774:21 [...] Indirect leak of 1520 byte(s) in 19 object(s) allocated from: #0 0x4f43c7 in calloc (/home/user/linux/tools/perf/perf+0x4f43c7) #1 0x87b3da in symbol__new /home/user/linux/tools/perf/util/symbol.c:269:23 #2 0x888954 in map__process_kallsym_symbol /home/user/linux/tools/perf/util/symbol.c:710:8 [...] Indirect leak of 1406 byte(s) in 19 object(s) allocated from: #0 0x4f43c7 in calloc (/home/user/linux/tools/perf/perf+0x4f43c7) #1 0x87b3da in symbol__new /home/user/linux/tools/perf/util/symbol.c:269:23 #2 0x8cfbd8 in machine__process_ksymbol_register /home/user/linux/tools/perf/util/machine.c:803:8 [...] Signed-off-by: Riccardo Mancini Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Andi Kleen Cc: Ian Rogers Cc: Jiapeng Chong Cc: Jiri Olsa Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Tommi Rantala Link: http://lore.kernel.org/lkml/20210612173751.188582-1-rickyman7@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/machine.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 3ff4936a15a4..da19be7da284 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -776,10 +776,10 @@ static int machine__process_ksymbol_register(struct machine *machine, if (dso) { dso->kernel = DSO_SPACE__KERNEL; map = map__new2(0, dso); + dso__put(dso); } if (!dso || !map) { - dso__put(dso); return -ENOMEM; } @@ -792,6 +792,7 @@ static int machine__process_ksymbol_register(struct machine *machine, map->start = event->ksymbol.addr; map->end = map->start + event->ksymbol.len; maps__insert(&machine->kmaps, map); + map__put(map); dso__set_loaded(dso); if (is_bpf_image(event->ksymbol.name)) { -- cgit v1.2.3 From 482698c2f848f9dee1a5bd949793c2fe6a71adc5 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 17 Jun 2021 11:42:13 -0700 Subject: perf test: Fix non-bash issue with stat bpf counters $(( .. )) is a bash feature but the test's interpreter is !/bin/sh, switch the code to use expr. Signed-off-by: Ian Rogers Cc: Alexander Shishkin Cc: Jiri Olsa Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Song Liu Cc: bpf@vger.kernel.org Link: http://lore.kernel.org/lkml/20210617184216.2075588-1-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/shell/stat_bpf_counters.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/tests/shell/stat_bpf_counters.sh b/tools/perf/tests/shell/stat_bpf_counters.sh index 22eb31e48ca7..2f9948b3d943 100755 --- a/tools/perf/tests/shell/stat_bpf_counters.sh +++ b/tools/perf/tests/shell/stat_bpf_counters.sh @@ -11,9 +11,9 @@ compare_number() second_num=$2 # upper bound is first_num * 110% - upper=$(( $first_num + $first_num / 10 )) + upper=$(expr $first_num + $first_num / 10 ) # lower bound is first_num * 90% - lower=$(( $first_num - $first_num / 10 )) + lower=$(expr $first_num - $first_num / 10 ) if [ $second_num -gt $upper ] || [ $second_num -lt $lower ]; then echo "The difference between $first_num and $second_num are greater than 10%." -- cgit v1.2.3 From ef83f9efe8461b8fd71eb60b53dbb6a5dd7b39e9 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Sat, 19 Jun 2021 10:09:08 -0300 Subject: perf beauty: Update copy of linux/socket.h with the kernel sources To pick the changes in: ea6932d70e223e02 ("net: make get_net_ns return error if NET_NS is disabled") That don't result in any changes in the tables generated from that header. This silences this perf build warning: Warning: Kernel ABI header at 'tools/perf/trace/beauty/include/linux/socket.h' differs from latest version at 'include/linux/socket.h' diff -u tools/perf/trace/beauty/include/linux/socket.h include/linux/socket.h Cc: Changbin Du Cc: David S. Miller Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/trace/beauty/include/linux/socket.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/perf/trace/beauty/include/linux/socket.h b/tools/perf/trace/beauty/include/linux/socket.h index b8fc5c53ba6f..0d8e3dcb7f88 100644 --- a/tools/perf/trace/beauty/include/linux/socket.h +++ b/tools/perf/trace/beauty/include/linux/socket.h @@ -438,6 +438,4 @@ extern int __sys_socketpair(int family, int type, int protocol, int __user *usockvec); extern int __sys_shutdown_sock(struct socket *sock, int how); extern int __sys_shutdown(int fd, int how); - -extern struct ns_common *get_net_ns(struct ns_common *ns); #endif /* _LINUX_SOCKET_H */ -- cgit v1.2.3 From 17d27fc314cba0205eec8966735a7a241cc8a5e0 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Sat, 19 Jun 2021 10:11:46 -0300 Subject: tools headers UAPI: Sync asm-generic/unistd.h with the kernel original To pick the changes in: 8b1462b67f23da54 ("quota: finish disable quotactl_path syscall") Those headers are used in some arches to generate the syscall table used in 'perf trace' to translate syscall numbers into strings. This addresses this perf build warning: Warning: Kernel ABI header at 'tools/include/uapi/asm-generic/unistd.h' differs from latest version at 'include/uapi/asm-generic/unistd.h' diff -u tools/include/uapi/asm-generic/unistd.h include/uapi/asm-generic/unistd.h Cc: Jan Kara Cc: Marcin Juszkiewicz Signed-off-by: Arnaldo Carvalho de Melo --- tools/include/uapi/asm-generic/unistd.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/include/uapi/asm-generic/unistd.h b/tools/include/uapi/asm-generic/unistd.h index 6de5a7fc066b..d2a942086fcb 100644 --- a/tools/include/uapi/asm-generic/unistd.h +++ b/tools/include/uapi/asm-generic/unistd.h @@ -863,8 +863,7 @@ __SYSCALL(__NR_process_madvise, sys_process_madvise) __SC_COMP(__NR_epoll_pwait2, sys_epoll_pwait2, compat_sys_epoll_pwait2) #define __NR_mount_setattr 442 __SYSCALL(__NR_mount_setattr, sys_mount_setattr) -#define __NR_quotactl_path 443 -__SYSCALL(__NR_quotactl_path, sys_quotactl_path) +/* 443 is reserved for quotactl_path */ #define __NR_landlock_create_ruleset 444 __SYSCALL(__NR_landlock_create_ruleset, sys_landlock_create_ruleset) -- cgit v1.2.3 From 1792a59eab9593de2eae36c40c5a22d70f52c026 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Sat, 19 Jun 2021 10:15:22 -0300 Subject: tools headers UAPI: Sync linux/in.h copy with the kernel sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To pick the changes in: 321827477360934d ("icmp: don't send out ICMP messages with a source address of 0.0.0.0") That don't result in any change in tooling, as INADDR_ are not used to generate id->string tables used by 'perf trace'. This addresses this build warning: Warning: Kernel ABI header at 'tools/include/uapi/linux/in.h' differs from latest version at 'include/uapi/linux/in.h' diff -u tools/include/uapi/linux/in.h include/uapi/linux/in.h Cc: David S. Miller Cc: Toke Høiland-Jørgensen Signed-off-by: Arnaldo Carvalho de Melo --- tools/include/uapi/linux/in.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/include/uapi/linux/in.h b/tools/include/uapi/linux/in.h index 7d6687618d80..d1b327036ae4 100644 --- a/tools/include/uapi/linux/in.h +++ b/tools/include/uapi/linux/in.h @@ -289,6 +289,9 @@ struct sockaddr_in { /* Address indicating an error return. */ #define INADDR_NONE ((unsigned long int) 0xffffffff) +/* Dummy address for src of ICMP replies if no real address is set (RFC7600). */ +#define INADDR_DUMMY ((unsigned long int) 0xc0000008) + /* Network number for local host loopback. */ #define IN_LOOPBACKNET 127 -- cgit v1.2.3