* [PATCH v3 0/3] perf hists browser: Avoid crash in some unusal operations
@ 2015-12-07 2:35 Wang Nan
2015-12-07 2:35 ` [PATCH v3 1/3] perf hists browser: Fix segfault if use symbol filter in cmdline Wang Nan
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Wang Nan @ 2015-12-07 2:35 UTC (permalink / raw)
To: acme, acme, namhyung; +Cc: linux-kernel, pi3orama, lizefan, Wang Nan
When using symbol filter in hists browser, some unusal operations
causes segfault. This patchset avoid those crashes.
Compare with v2: Improve coding style, check he_selection in 1/3.
Arnaldo: I can't find them in your repository. If you have already
picked these patches, please ignore this version.
Thank you.
Wang Nan (3):
perf hists browser: Fix segfault if use symbol filter in cmdline
perf hists browser: Add NULL pointer check to prevent crash
perf hists browser: Reset selection when refresh
tools/perf/ui/browsers/hists.c | 8 ++++++++
1 file changed, 8 insertions(+)
--
1.8.3.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/3] perf hists browser: Fix segfault if use symbol filter in cmdline
2015-12-07 2:35 [PATCH v3 0/3] perf hists browser: Avoid crash in some unusal operations Wang Nan
@ 2015-12-07 2:35 ` Wang Nan
2015-12-08 5:13 ` [tip:perf/core] " tip-bot for Wang Nan
2015-12-07 2:35 ` [PATCH v3 2/3] perf hists browser: Add NULL pointer check to prevent crash Wang Nan
2015-12-07 2:35 ` [PATCH v3 3/3] perf hists browser: Reset selection when refresh Wang Nan
2 siblings, 1 reply; 7+ messages in thread
From: Wang Nan @ 2015-12-07 2:35 UTC (permalink / raw)
To: acme, acme, namhyung; +Cc: linux-kernel, pi3orama, lizefan, Wang Nan
If feed perf a symbol filter in cmdline and the result is empty,
pressing 'Enter' in the hist browser causes crash:
# ./perf report perf.data <-- Common mistake for beginners
Then press 'Enter':
perf: Segmentation fault
-------- backtrace --------
/home/wangnan/perf[0x53e578]
/lib64/libc.so.6(+0x3545f)[0x7f76bafe045f]
/home/wangnan/perf[0x539dd4]
/home/wangnan/perf(perf_evlist__tui_browse_hists+0x96)[0x53d216]
/home/wangnan/perf(cmd_report+0x1b9f)[0x442c7f]
/home/wangnan/perf[0x47efa2]
/home/wangnan/perf(main+0x5f5)[0x432fa5]
/lib64/libc.so.6(__libc_start_main+0xf4)[0x7f76bafccbd4]
/home/wangnan/perf[0x4330d4]
This is because 'perf.data' is interpreted as a symbol filter, and the
result is empty, so selection is empty. However,
hist_browser__toggle_fold() forgets to check it.
This patch simply return false when selection is NULL.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/hists.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index dcdcbaf..75ed14b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -340,6 +340,9 @@ static bool hist_browser__toggle_fold(struct hist_browser *browser)
struct callchain_list *cl = container_of(ms, struct callchain_list, ms);
bool has_children;
+ if (!he || !ms)
+ return false;
+
if (ms == &he->ms)
has_children = hist_entry__toggle_fold(he);
else
--
1.8.3.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/3] perf hists browser: Add NULL pointer check to prevent crash
2015-12-07 2:35 [PATCH v3 0/3] perf hists browser: Avoid crash in some unusal operations Wang Nan
2015-12-07 2:35 ` [PATCH v3 1/3] perf hists browser: Fix segfault if use symbol filter in cmdline Wang Nan
@ 2015-12-07 2:35 ` Wang Nan
2015-12-08 5:13 ` [tip:perf/core] " tip-bot for Wang Nan
2015-12-07 2:35 ` [PATCH v3 3/3] perf hists browser: Reset selection when refresh Wang Nan
2 siblings, 1 reply; 7+ messages in thread
From: Wang Nan @ 2015-12-07 2:35 UTC (permalink / raw)
To: acme, acme, namhyung; +Cc: linux-kernel, pi3orama, lizefan, Wang Nan
Before this patch we can trigger a segfault by following steps:
Step 0: Use 'perf record' to generate a perf.data without callchain
Step 1: perf report
Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'
Step 3: Use '/' to filter symbols, use a filter which returns
empty result
Step 4: Press 'ENTER' (notice here that the old selection is still
there. This is another problem)
Step 5: Press 'ENTER' to annotate that symbol
Step 6: Press 'LEFT' to go out.
Result: segfault:
perf: Segmentation fault
-------- backtrace --------
/home/wangnan/perf[0x53e568]
/lib64/libc.so.6(+0x3545f)[0x7fba75d3245f]
/home/wangnan/perf[0x537516]
/home/wangnan/perf[0x533fef]
/home/wangnan/perf[0x53b347]
/home/wangnan/perf(perf_evlist__tui_browse_hists+0x96)[0x53d206]
/home/wangnan/perf(cmd_report+0x1b9f)[0x442c7f]
/home/wangnan/perf[0x47efa2]
/home/wangnan/perf(main+0x5f5)[0x432fa5]
/lib64/libc.so.6(__libc_start_main+0xf4)[0x7fba75d1ebd4]
/home/wangnan/perf[0x4330d4]
This is because in this case 'nd' could be NULL in
ui_browser__hists_seek(), but that function never check it.
This patch adds checker for potential NULL pointer in that function.
After this patch the above steps won't segfault again.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/hists.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 75ed14b..9b8a48d 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1297,6 +1297,9 @@ static void ui_browser__hists_seek(struct ui_browser *browser,
* and stop when we printed enough lines to fill the screen.
*/
do_offset:
+ if (!nd)
+ return;
+
if (offset > 0) {
do {
h = rb_entry(nd, struct hist_entry, rb_node);
--
1.8.3.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] perf hists browser: Reset selection when refresh
2015-12-07 2:35 [PATCH v3 0/3] perf hists browser: Avoid crash in some unusal operations Wang Nan
2015-12-07 2:35 ` [PATCH v3 1/3] perf hists browser: Fix segfault if use symbol filter in cmdline Wang Nan
2015-12-07 2:35 ` [PATCH v3 2/3] perf hists browser: Add NULL pointer check to prevent crash Wang Nan
@ 2015-12-07 2:35 ` Wang Nan
2015-12-08 5:13 ` [tip:perf/core] " tip-bot for Wang Nan
2 siblings, 1 reply; 7+ messages in thread
From: Wang Nan @ 2015-12-07 2:35 UTC (permalink / raw)
To: acme, acme, namhyung; +Cc: linux-kernel, pi3orama, lizefan, Wang Nan
With following steps:
Step 1: perf report
Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'
Step 3: Use '/' to filter symbols, use a filter which returns
empty result
Step 4: Press 'ENTER'
We see that, even if we have filter all symbols (and the main interface
is empty), pressing 'ENTER' still select one symbol. This behavior
surprises user. This patch resets browser->{he_,}selection in
hist_browser__refresh() and let it choose default selection. In this
case browser->{he_,}selection keeps NULL so user won't see annotation item
in menu.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/hists.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 9b8a48d..ec33196 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1192,6 +1192,8 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
}
ui_browser__hists_init_top(browser);
+ hb->he_selection = NULL;
+ hb->selection = NULL;
for (nd = browser->top; nd; nd = rb_next(nd)) {
struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
--
1.8.3.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:perf/core] perf hists browser: Add NULL pointer check to prevent crash
2015-12-07 2:35 ` [PATCH v3 2/3] perf hists browser: Add NULL pointer check to prevent crash Wang Nan
@ 2015-12-08 5:13 ` tip-bot for Wang Nan
0 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Wang Nan @ 2015-12-08 5:13 UTC (permalink / raw)
To: linux-tip-commits
Cc: hpa, namhyung, linux-kernel, mingo, acme, wangnan0, lizefan, tglx
Commit-ID: 837eeb7569bf2b3bd3b1b82e0e61edb19811036e
Gitweb: http://git.kernel.org/tip/837eeb7569bf2b3bd3b1b82e0e61edb19811036e
Author: Wang Nan <wangnan0@huawei.com>
AuthorDate: Mon, 7 Dec 2015 02:35:45 +0000
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 7 Dec 2015 12:02:11 -0300
perf hists browser: Add NULL pointer check to prevent crash
Before this patch we can trigger a segfault by following steps:
Step 0: Use 'perf record' to generate a perf.data without callchain
Step 1: perf report
Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'
Step 3: Use '/' to filter symbols, use a filter which returns
empty result
Step 4: Press 'ENTER' (notice here that the old selection is still
there. This is another problem)
Step 5: Press 'ENTER' to annotate that symbol
Step 6: Press 'LEFT' to go out.
Result: segfault:
perf: Segmentation fault
-------- backtrace --------
/home/wangnan/perf[0x53e568]
/lib64/libc.so.6(+0x3545f)[0x7fba75d3245f]
/home/wangnan/perf[0x537516]
/home/wangnan/perf[0x533fef]
/home/wangnan/perf[0x53b347]
/home/wangnan/perf(perf_evlist__tui_browse_hists+0x96)[0x53d206]
/home/wangnan/perf(cmd_report+0x1b9f)[0x442c7f]
/home/wangnan/perf[0x47efa2]
/home/wangnan/perf(main+0x5f5)[0x432fa5]
/lib64/libc.so.6(__libc_start_main+0xf4)[0x7fba75d1ebd4]
/home/wangnan/perf[0x4330d4]
This is because in this case 'nd' could be NULL in
ui_browser__hists_seek(), but that function never checks it.
This patch adds checker for potential NULL pointer in that function.
After this patch the above steps won't segfault.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1449455746-41952-3-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/hists.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index fa9eb92..932e13d 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1033,6 +1033,9 @@ static void ui_browser__hists_seek(struct ui_browser *browser,
* and stop when we printed enough lines to fill the screen.
*/
do_offset:
+ if (!nd)
+ return;
+
if (offset > 0) {
do {
h = rb_entry(nd, struct hist_entry, rb_node);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:perf/core] perf hists browser: Reset selection when refresh
2015-12-07 2:35 ` [PATCH v3 3/3] perf hists browser: Reset selection when refresh Wang Nan
@ 2015-12-08 5:13 ` tip-bot for Wang Nan
0 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Wang Nan @ 2015-12-08 5:13 UTC (permalink / raw)
To: linux-tip-commits
Cc: hpa, wangnan0, linux-kernel, lizefan, namhyung, tglx, mingo, acme
Commit-ID: 979d2cac1144da6b25334a8572c80cde9662105c
Gitweb: http://git.kernel.org/tip/979d2cac1144da6b25334a8572c80cde9662105c
Author: Wang Nan <wangnan0@huawei.com>
AuthorDate: Mon, 7 Dec 2015 02:35:46 +0000
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 7 Dec 2015 12:02:12 -0300
perf hists browser: Reset selection when refresh
With the following steps:
Step 1: perf report
Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'
Step 3: Use '/' to filter symbols, use a filter which returns
empty result
Step 4: Press 'ENTER'
We see that, even if we have filtered all the symbols (and the main
interface is empty), pressing 'ENTER' still selects one symbol. This
behavior surprises the user.
This patch resets browser->{he_,}selection in hist_browser__refresh()
and lets it choose default selection. In this case
browser->{he_,}selection keeps NULL so user won't see annotation item in
menu.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1449455746-41952-4-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/hists.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 932e13d..84c8251 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -928,6 +928,8 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
}
ui_browser__hists_init_top(browser);
+ hb->he_selection = NULL;
+ hb->selection = NULL;
for (nd = browser->top; nd; nd = rb_next(nd)) {
struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:perf/core] perf hists browser: Fix segfault if use symbol filter in cmdline
2015-12-07 2:35 ` [PATCH v3 1/3] perf hists browser: Fix segfault if use symbol filter in cmdline Wang Nan
@ 2015-12-08 5:13 ` tip-bot for Wang Nan
0 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Wang Nan @ 2015-12-08 5:13 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, linux-kernel, hpa, namhyung, tglx, lizefan, wangnan0, acme
Commit-ID: 4938cf0c7a62025bbfbf3db7bcdcc2c33312bedb
Gitweb: http://git.kernel.org/tip/4938cf0c7a62025bbfbf3db7bcdcc2c33312bedb
Author: Wang Nan <wangnan0@huawei.com>
AuthorDate: Mon, 7 Dec 2015 02:35:44 +0000
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 7 Dec 2015 12:02:35 -0300
perf hists browser: Fix segfault if use symbol filter in cmdline
If feed perf a symbol filter in cmdline and the result is empty,
pressing 'Enter' in the hist browser causes crash:
# ./perf report perf.data <-- Common mistake for beginners
Then press 'Enter':
perf: Segmentation fault
-------- backtrace --------
/home/wangnan/perf[0x53e578]
/lib64/libc.so.6(+0x3545f)[0x7f76bafe045f]
/home/wangnan/perf[0x539dd4]
/home/wangnan/perf(perf_evlist__tui_browse_hists+0x96)[0x53d216]
/home/wangnan/perf(cmd_report+0x1b9f)[0x442c7f]
/home/wangnan/perf[0x47efa2]
/home/wangnan/perf(main+0x5f5)[0x432fa5]
/lib64/libc.so.6(__libc_start_main+0xf4)[0x7f76bafccbd4]
/home/wangnan/perf[0x4330d4]
This is because 'perf.data' is interpreted as a symbol filter, and the
result is empty, so selection is empty. However,
hist_browser__toggle_fold() forgets to check it.
This patch simply return false when selection is NULL.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1449455746-41952-2-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/hists.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 84c8251..81def6c3 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -298,6 +298,9 @@ static bool hist_browser__toggle_fold(struct hist_browser *browser)
struct callchain_list *cl = container_of(ms, struct callchain_list, ms);
bool has_children;
+ if (!he || !ms)
+ return false;
+
if (ms == &he->ms)
has_children = hist_entry__toggle_fold(he);
else
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-12-08 5:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-07 2:35 [PATCH v3 0/3] perf hists browser: Avoid crash in some unusal operations Wang Nan
2015-12-07 2:35 ` [PATCH v3 1/3] perf hists browser: Fix segfault if use symbol filter in cmdline Wang Nan
2015-12-08 5:13 ` [tip:perf/core] " tip-bot for Wang Nan
2015-12-07 2:35 ` [PATCH v3 2/3] perf hists browser: Add NULL pointer check to prevent crash Wang Nan
2015-12-08 5:13 ` [tip:perf/core] " tip-bot for Wang Nan
2015-12-07 2:35 ` [PATCH v3 3/3] perf hists browser: Reset selection when refresh Wang Nan
2015-12-08 5:13 ` [tip:perf/core] " tip-bot for Wang Nan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).