From 6c1785cd75ef55a308701813330a162002ffe192 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Mon, 10 Jun 2024 22:06:26 -0700 Subject: [PATCH] perf record: Ensure space for lost samples Previous allocation didn't account for sample ID written after the lost samples event. Switch from malloc/free to a stack allocation. Reported-by: Milian Wolff Closes: https://lore.kernel.org/linux-perf-users/23879991.0LEYPuXRzz@milian-workstation/ Signed-off-by: Ian Rogers Acked-by: Namhyung Kim Signed-off-by: Namhyung Kim Link: https://lore.kernel.org/r/20240611050626.1223155-1-irogers@google.com --- tools/lib/perf/include/perf/event.h | 6 +++++ tools/perf/builtin-record.c | 34 ++++++++--------------------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h index ae64090184d3..37bb7771d914 100644 --- a/tools/lib/perf/include/perf/event.h +++ b/tools/lib/perf/include/perf/event.h @@ -77,6 +77,12 @@ struct perf_record_lost_samples { __u64 lost; }; +#define MAX_ID_HDR_ENTRIES 6 +struct perf_record_lost_samples_and_ids { + struct perf_record_lost_samples lost; + __u64 sample_ids[MAX_ID_HDR_ENTRIES]; +}; + /* * PERF_FORMAT_ENABLED | PERF_FORMAT_RUNNING | PERF_FORMAT_ID | PERF_FORMAT_LOST */ diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 66a3de8ac661..019305b94e5f 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -1926,7 +1926,7 @@ static void __record__save_lost_samples(struct record *rec, struct evsel *evsel, static void record__read_lost_samples(struct record *rec) { struct perf_session *session = rec->session; - struct perf_record_lost_samples *lost = NULL; + struct perf_record_lost_samples_and_ids lost; struct evsel *evsel; /* there was an error during record__open */ @@ -1951,20 +1951,13 @@ static void record__read_lost_samples(struct record *rec) if (perf_evsel__read(&evsel->core, x, y, &count) < 0) { pr_debug("read LOST count failed\n"); - goto out; + return; } if (count.lost) { - if (!lost) { - lost = zalloc(sizeof(*lost) + - session->machines.host.id_hdr_size); - if (!lost) { - pr_debug("Memory allocation failed\n"); - return; - } - lost->header.type = PERF_RECORD_LOST_SAMPLES; - } - __record__save_lost_samples(rec, evsel, lost, + memset(&lost.lost, 0, sizeof(lost)); + lost.lost.header.type = PERF_RECORD_LOST_SAMPLES; + __record__save_lost_samples(rec, evsel, &lost.lost, x, y, count.lost, 0); } } @@ -1972,21 +1965,12 @@ static void record__read_lost_samples(struct record *rec) lost_count = perf_bpf_filter__lost_count(evsel); if (lost_count) { - if (!lost) { - lost = zalloc(sizeof(*lost) + - session->machines.host.id_hdr_size); - if (!lost) { - pr_debug("Memory allocation failed\n"); - return; - } - lost->header.type = PERF_RECORD_LOST_SAMPLES; - } - __record__save_lost_samples(rec, evsel, lost, 0, 0, lost_count, + memset(&lost.lost, 0, sizeof(lost)); + lost.lost.header.type = PERF_RECORD_LOST_SAMPLES; + __record__save_lost_samples(rec, evsel, &lost.lost, 0, 0, lost_count, PERF_RECORD_MISC_LOST_SAMPLES_BPF); } } -out: - free(lost); } static volatile sig_atomic_t workload_exec_errno; @@ -3198,7 +3182,7 @@ static int switch_output_setup(struct record *rec) unsigned long val; /* - * If we're using --switch-output-events, then we imply its + * If we're using --switch-output-events, then we imply its * --switch-output=signal, as we'll send a SIGUSR2 from the side band * thread to its parent. */