On Thu, Dec 03, 2015 at 03:08:14AM +0000, Wang Nan wrote: > 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 <[email protected]> > Cc: Arnaldo Carvalho de Melo <[email protected]>
Acked-by: Namhyung Kim <[email protected]> A nitpick below.. > --- > 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 601a585..7447515 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; Just a style comment, not serious. I prefer the blank line is under the if statement like below.. do_offset: + if (!nd) + return; + Thanks, Namhyung > if (offset > 0) { > do { > h = rb_entry(nd, struct hist_entry, rb_node); > -- > 1.8.3.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
