* [PATCH v2] perf stat-display: Check if snprintf()'s fmt argument is NULL
@ 2023-08-04 2:09 Kaige Ye
2023-08-14 20:24 ` Ian Rogers
0 siblings, 1 reply; 3+ messages in thread
From: Kaige Ye @ 2023-08-04 2:09 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, irogers, adrian.hunter
Cc: linux-perf-users, linux-kernel, Kaige Ye
It is undefined behavior to pass NULL as snprintf()'s fmt argument.
Here is an example to trigger the problem:
$ perf stat --metric-only -x, -e instructions -- sleep 1
insn per cycle,
Segmentation fault (core dumped)
With this patch:
$ perf stat --metric-only -x, -e instructions -- sleep 1
insn per cycle,
,
Signed-off-by: Kaige Ye <ye@kaige.org>
---
V1 -> V2: Addressed Ian's comments (Ian Rogers)
---
tools/perf/util/stat-display.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 7329b3340..031888545 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -578,7 +578,7 @@ static void print_metric_only_csv(struct perf_stat_config *config __maybe_unused
if (!valid_only_metric(unit))
return;
unit = fixunit(tbuf, os->evsel, unit);
- snprintf(buf, sizeof buf, fmt, val);
+ snprintf(buf, sizeof(buf), fmt ?: "", val);
ends = vals = skip_spaces(buf);
while (isdigit(*ends) || *ends == '.')
ends++;
@@ -600,7 +600,7 @@ static void print_metric_only_json(struct perf_stat_config *config __maybe_unuse
if (!valid_only_metric(unit))
return;
unit = fixunit(tbuf, os->evsel, unit);
- snprintf(buf, sizeof(buf), fmt, val);
+ snprintf(buf, sizeof(buf), fmt ?: "", val);
ends = vals = skip_spaces(buf);
while (isdigit(*ends) || *ends == '.')
ends++;
base-commit: f6b8436bede3e80226e8b2100279c4450c73806a
--
2.41.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] perf stat-display: Check if snprintf()'s fmt argument is NULL
2023-08-04 2:09 [PATCH v2] perf stat-display: Check if snprintf()'s fmt argument is NULL Kaige Ye
@ 2023-08-14 20:24 ` Ian Rogers
2023-08-21 13:54 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2023-08-14 20:24 UTC (permalink / raw)
To: Kaige Ye
Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, adrian.hunter, linux-perf-users, linux-kernel
On Thu, Aug 3, 2023 at 7:10 PM Kaige Ye <ye@kaige.org> wrote:
>
> It is undefined behavior to pass NULL as snprintf()'s fmt argument.
> Here is an example to trigger the problem:
>
> $ perf stat --metric-only -x, -e instructions -- sleep 1
> insn per cycle,
> Segmentation fault (core dumped)
>
> With this patch:
>
> $ perf stat --metric-only -x, -e instructions -- sleep 1
> insn per cycle,
> ,
>
> Signed-off-by: Kaige Ye <ye@kaige.org>
Thanks Kaige!
Reviewed-by: Ian Rogers <irogers@google.com>
> ---
> V1 -> V2: Addressed Ian's comments (Ian Rogers)
> ---
> tools/perf/util/stat-display.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 7329b3340..031888545 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -578,7 +578,7 @@ static void print_metric_only_csv(struct perf_stat_config *config __maybe_unused
> if (!valid_only_metric(unit))
> return;
> unit = fixunit(tbuf, os->evsel, unit);
> - snprintf(buf, sizeof buf, fmt, val);
> + snprintf(buf, sizeof(buf), fmt ?: "", val);
> ends = vals = skip_spaces(buf);
> while (isdigit(*ends) || *ends == '.')
> ends++;
> @@ -600,7 +600,7 @@ static void print_metric_only_json(struct perf_stat_config *config __maybe_unuse
> if (!valid_only_metric(unit))
> return;
> unit = fixunit(tbuf, os->evsel, unit);
> - snprintf(buf, sizeof(buf), fmt, val);
> + snprintf(buf, sizeof(buf), fmt ?: "", val);
> ends = vals = skip_spaces(buf);
> while (isdigit(*ends) || *ends == '.')
> ends++;
>
> base-commit: f6b8436bede3e80226e8b2100279c4450c73806a
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] perf stat-display: Check if snprintf()'s fmt argument is NULL
2023-08-14 20:24 ` Ian Rogers
@ 2023-08-21 13:54 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-08-21 13:54 UTC (permalink / raw)
To: Ian Rogers
Cc: Kaige Ye, peterz, mingo, mark.rutland, alexander.shishkin, jolsa,
namhyung, adrian.hunter, linux-perf-users, linux-kernel
Em Mon, Aug 14, 2023 at 01:24:06PM -0700, Ian Rogers escreveu:
> On Thu, Aug 3, 2023 at 7:10 PM Kaige Ye <ye@kaige.org> wrote:
> >
> > It is undefined behavior to pass NULL as snprintf()'s fmt argument.
> > Here is an example to trigger the problem:
> >
> > $ perf stat --metric-only -x, -e instructions -- sleep 1
> > insn per cycle,
> > Segmentation fault (core dumped)
> >
> > With this patch:
> >
> > $ perf stat --metric-only -x, -e instructions -- sleep 1
> > insn per cycle,
> > ,
> >
> > Signed-off-by: Kaige Ye <ye@kaige.org>
>
> Thanks Kaige!
>
> Reviewed-by: Ian Rogers <irogers@google.com>
Thanks, applied.
- Arnaldo
> > ---
> > V1 -> V2: Addressed Ian's comments (Ian Rogers)
> > ---
> > tools/perf/util/stat-display.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index 7329b3340..031888545 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
> > @@ -578,7 +578,7 @@ static void print_metric_only_csv(struct perf_stat_config *config __maybe_unused
> > if (!valid_only_metric(unit))
> > return;
> > unit = fixunit(tbuf, os->evsel, unit);
> > - snprintf(buf, sizeof buf, fmt, val);
> > + snprintf(buf, sizeof(buf), fmt ?: "", val);
> > ends = vals = skip_spaces(buf);
> > while (isdigit(*ends) || *ends == '.')
> > ends++;
> > @@ -600,7 +600,7 @@ static void print_metric_only_json(struct perf_stat_config *config __maybe_unuse
> > if (!valid_only_metric(unit))
> > return;
> > unit = fixunit(tbuf, os->evsel, unit);
> > - snprintf(buf, sizeof(buf), fmt, val);
> > + snprintf(buf, sizeof(buf), fmt ?: "", val);
> > ends = vals = skip_spaces(buf);
> > while (isdigit(*ends) || *ends == '.')
> > ends++;
> >
> > base-commit: f6b8436bede3e80226e8b2100279c4450c73806a
> > --
> > 2.41.0
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-21 13:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-04 2:09 [PATCH v2] perf stat-display: Check if snprintf()'s fmt argument is NULL Kaige Ye
2023-08-14 20:24 ` Ian Rogers
2023-08-21 13:54 ` 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.