All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools: perf: session: avoid segfault on implicit stdin
@ 2025-08-26  0:16 Stephen Brennan
  2025-09-12 19:19 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 2+ messages in thread
From: Stephen Brennan @ 2025-08-26  0:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Peter Zijlstra, Namhyung Kim,
	Ingo Molnar
  Cc: Alexander Shishkin, Andi Kleen, linux-kernel, Mark Rutland,
	Liang, Kan, linux-perf-users, Ben Gainey, Dmitry Vyukov,
	Blake Jones, Chun-Tse Shao, Ian Rogers, Jiri Olsa,
	Stephen Brennan, Adrian Hunter

A user reported that running a command like:

    perf script flamegraph -ag -F99 -- sleep 10

Resulted in a segmentation fault. The reason is twofold. First, the
"flamegraph-report" script has a shebang line which ends with "--". This
disables option parsing, so that the exec'd "perf script report" command
does not see the option "-i -" which is appended to its command line.
Second, despite the unprocessed "-i -" option, the default behavior of
perf is to use stdin if it is a pipe -- and in this case, it is. Thus, the
report continues running, but segfaults on accessing the filename.

The fix for the second issue is a simple NULL check. Implement this
here.

Fixes: 4553c431e7dd2 ("perf report: Display pregress bar on redirected pipe data")

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
---
 tools/perf/util/session.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 26ae078278cd6..a9624505c0ca3 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1854,9 +1854,11 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
 	/*
 	 * If it's from a file saving pipe data (by redirection), it would have
 	 * a file name other than "-".  Then we can get the total size and show
-	 * the progress.
+	 * the progress. However, be careful because path may be NULL if input
+	 * is coming from stdin.
 	 */
-	if (strcmp(session->data->path, "-") && session->data->file.size) {
+	if (session->data->path && strcmp(session->data->path, "-")
+	    && session->data->file.size) {
 		ui_progress__init_size(&prog, session->data->file.size,
 				       "Processing events...");
 		update_prog = true;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] tools: perf: session: avoid segfault on implicit stdin
  2025-08-26  0:16 [PATCH] tools: perf: session: avoid segfault on implicit stdin Stephen Brennan
@ 2025-09-12 19:19 ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 2+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-09-12 19:19 UTC (permalink / raw)
  To: Stephen Brennan
  Cc: Peter Zijlstra, Namhyung Kim, Ingo Molnar, Alexander Shishkin,
	Andi Kleen, linux-kernel, Mark Rutland, Liang, Kan,
	linux-perf-users, Ben Gainey, Dmitry Vyukov, Blake Jones,
	Chun-Tse Shao, Ian Rogers, Jiri Olsa, Adrian Hunter

On Mon, Aug 25, 2025 at 05:16:48PM -0700, Stephen Brennan wrote:
> A user reported that running a command like:
> 
>     perf script flamegraph -ag -F99 -- sleep 10
> 
> Resulted in a segmentation fault. The reason is twofold. First, the
> "flamegraph-report" script has a shebang line which ends with "--". This
> disables option parsing, so that the exec'd "perf script report" command
> does not see the option "-i -" which is appended to its command line.
> Second, despite the unprocessed "-i -" option, the default behavior of
> perf is to use stdin if it is a pipe -- and in this case, it is. Thus, the
> report continues running, but segfaults on accessing the filename.
> 
> The fix for the second issue is a simple NULL check. Implement this
> here.

I did a test before and after and the problem is indeed present:

root@number:~# perf script flamegraph -ag -F99 -- sleep 1
/home/acme/libexec/perf-core/scripts/python/bin/flamegraph-report: line 3: 362975 Segmentation fault      (core dumped) perf script -s "$PERF_EXEC_PATH"/scripts/python/flamegraph.py "$@"
root@number:~# 

But even after your patch, it still segfaults.

I'll try to look at this after processing lower hanging patches.

Thanks for working on this!

- Arnaldo
 
> Fixes: 4553c431e7dd2 ("perf report: Display pregress bar on redirected pipe data")
> 
> Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
> ---
>  tools/perf/util/session.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 26ae078278cd6..a9624505c0ca3 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1854,9 +1854,11 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
>  	/*
>  	 * If it's from a file saving pipe data (by redirection), it would have
>  	 * a file name other than "-".  Then we can get the total size and show
> -	 * the progress.
> +	 * the progress. However, be careful because path may be NULL if input
> +	 * is coming from stdin.
>  	 */
> -	if (strcmp(session->data->path, "-") && session->data->file.size) {
> +	if (session->data->path && strcmp(session->data->path, "-")
> +	    && session->data->file.size) {
>  		ui_progress__init_size(&prog, session->data->file.size,
>  				       "Processing events...");
>  		update_prog = true;
> -- 
> 2.47.3

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-09-12 19:19 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-26  0:16 [PATCH] tools: perf: session: avoid segfault on implicit stdin Stephen Brennan
2025-09-12 19:19 ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.