Messages in this thread | | | From | Ian Rogers <> | Date | Wed, 2 Aug 2023 10:45:47 -0700 | Subject | Re: [PATCH] perf stat-display: Check if snprintf()'s fmt argument is NULL |
| |
On Tue, Aug 1, 2023 at 9:04 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> > --- > tools/perf/util/stat-display.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c > index 7329b3340..e8936cffd 100644 > --- a/tools/perf/util/stat-display.c > +++ b/tools/perf/util/stat-display.c > @@ -438,7 +438,7 @@ static void print_metric_csv(struct perf_stat_config *config __maybe_unused, > fprintf(out, "%s%s", config->csv_sep, config->csv_sep); > return; > } > - snprintf(buf, sizeof(buf), fmt, val); > + snprintf(buf, sizeof(buf), fmt ?: "", val);
Hi,
I couldn't reproduce this error and the code immediately above here does null check fmt: https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/stat-display.c?h=perf-tools-next#n437 Perhaps this is an older version of the perf tool? You can run "perf version" to determine this.
Thanks, Ian
> ends = vals = skip_spaces(buf); > while (isdigit(*ends) || *ends == '.') > ends++; > @@ -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++; > -- > 2.41.0 >
| |