linux-kernel - Re: [PATCH 16/35] perf annotate: Add samples into struct annotation

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]

Message-ID: <20171114093114.GA25679@krava>
Date:   Tue, 14 Nov 2017 10:31:14 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
Cc:     Jiri Olsa <jolsa@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        David Ahern <dsahern@...il.com>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH 16/35] perf annotate: Add samples into struct
 annotation_line

On Mon, Nov 13, 2017 at 09:14:38PM +0100, Jiri Olsa wrote:
> On Mon, Nov 13, 2017 at 09:16:20PM +0530, Ravi Bangoria wrote:
> > Hi Jiri,
> > 
> > This patch seems to be causing segfault with "perf top --stdio".
> > 
> > Steps to reproduce:
> > 1. start "perf top --stdio" in one terminal
> > 2. run some simple workload in another terminal, let it get finished.
> > 3. annotate function from previous workload in perf top (press 'a' followed
> > by 's')
> > 
> > Perf will crash with:
> > 
> >   perf: Segmentation fault
> >   Obtained 8 stack frames.
> >   ./perf(sighandler_dump_stack+0x3e) [0x4f1b6e]
> >   /lib64/libc.so.6(+0x36a7f) [0x7ff3aa7e4a7f]
> >   ./perf() [0x4a27fd]
> >   ./perf(symbol__annotate+0x199) [0x4a4439]
> >   ./perf() [0x44e32d]
> >   ./perf() [0x44f098]
> >   /lib64/libpthread.so.0(+0x736c) [0x7ff3acee836c]
> >   /lib64/libc.so.6(clone+0x3e) [0x7ff3aa8bee1e]
> > 
> > Can you please check.
> 
> hum, I'm getting following crash after resizing the terminal window:
> 
> perf: Floating point exception
> Obtained 8 stack frames.
> ./perf(dump_stack+0x2e) [0x510c89]
> ./perf(sighandler_dump_stack+0x2e) [0x510d69]
> /lib64/libc.so.6(+0x36a80) [0x7f9419588a80]
> ./perf(perf_top__header_snprintf+0x208) [0x4f42c1]
> ./perf() [0x453c09]
> ./perf() [0x454ddb]
> /lib64/libpthread.so.0(+0x736d) [0x7f941bc8c36d]
> /lib64/libc.so.6(clone+0x3f) [0x7f9419662e1f]
> Floating point exception (core dumped)
> 
> working on fix

so my crash is caused by bogus resize code, I have it working with fix for
memory corruption happening in SIGWINCH signal handler (attached)
could you please check if that fixes the code for you?

I'll check if that does not break the TUI

jirka


---
 tools/perf/builtin-top.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index adfeeb488f1a..7a2a7d7931c6 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -77,6 +77,7 @@
 #include "sane_ctype.h"
 
 static volatile int done;
+static volatile int resize;
 
 #define HEADER_LINE_NR  5
 
@@ -86,10 +87,13 @@ static void perf_top__update_print_entries(struct perf_top *top)
 }
 
 static void perf_top__sig_winch(int sig __maybe_unused,
-				siginfo_t *info __maybe_unused, void *arg)
+				siginfo_t *info __maybe_unused, void *arg __maybe_unused)
 {
-	struct perf_top *top = arg;
+	resize = 1;
+}
 
+static void perf_top__resize(struct perf_top *top)
+{
 	get_term_dimensions(&top->winsize);
 	perf_top__update_print_entries(top);
 }
@@ -475,9 +479,8 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
 			if (top->print_entries == 0) {
 				struct sigaction act = {
 					.sa_sigaction = perf_top__sig_winch,
-					.sa_flags     = SA_SIGINFO,
 				};
-				perf_top__sig_winch(SIGWINCH, NULL, top);
+				perf_top__resize(top);
 				sigaction(SIGWINCH, &act, NULL);
 			} else {
 				signal(SIGWINCH, SIG_DFL);
@@ -1030,6 +1033,11 @@ static int __cmd_top(struct perf_top *top)
 
 		if (hits == top->samples)
 			ret = perf_evlist__poll(top->evlist, 100);
+
+		if (resize) {
+			perf_top__resize(top);
+			resize = 0;
+		}
 	}
 
 	ret = 0;
@@ -1354,9 +1362,8 @@ int cmd_top(int argc, const char **argv)
 	if (top.print_entries == 0) {
 		struct sigaction act = {
 			.sa_sigaction = perf_top__sig_winch,
-			.sa_flags     = SA_SIGINFO,
 		};
-		perf_top__update_print_entries(&top);
+		perf_top__resize(&top);
 		sigaction(SIGWINCH, &act, NULL);
 	}
 
-- 
2.9.5