[PATCH v1 1/7] perf session: Fix minimum event size for PERF_RECORD_CALLCHAIN_DEFERRED

[PATCH v1 1/7] perf session: Fix minimum event size for PERF_RECORD_CALLCHAIN_DEFERRED

[Ian Rogers]

struct perf_record_callchain_deferred contains header, cookie, and nr
(24 bytes), but perf_event__min_size[PERF_RECORD_CALLCHAIN_DEFERRED]
only checked for header + one u64 (16 bytes). Use
sizeof(struct perf_record_callchain_deferred).

Fixes: c69993ecdd4d ("perf: Support deferred user unwind")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/session.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 56a5e57ec512..d9d06337933f 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2773,7 +2773,7 @@ static const u32 perf_event__min_size[PERF_RECORD_HEADER_MAX] = {
 	[PERF_RECORD_HEADER_FEATURE]	  = sizeof(struct perf_record_header_feature),
 	[PERF_RECORD_COMPRESSED2]	  = sizeof(struct perf_record_compressed2),
 	[PERF_RECORD_BPF_METADATA]	  = sizeof(struct perf_record_bpf_metadata),
-	[PERF_RECORD_CALLCHAIN_DEFERRED]  = sizeof(struct perf_event_header) + sizeof(__u64),
+	[PERF_RECORD_CALLCHAIN_DEFERRED]  = sizeof(struct perf_record_callchain_deferred),
 	/*
 	 * SCHEDSTAT events have a version-dependent union after the
 	 * fixed header fields; the minimum is the base (pre-union)
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v1 2/7] perf evsel: Add bounds check for PERF_RECORD_CALLCHAIN_DEFERRED ips array

[Ian Rogers]

In __evsel__parse_sample(), verify that event->callchain_deferred.nr and
the trailing ips[] array fit within event->header.size using
OVERFLOW_CHECK_u64() and OVERFLOW_CHECK() before exposing data->callchain
to callers.

Fixes: da27c4379ce6 ("perf session: Add minimum event size and alignment validation")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/evsel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d060fca8090f..64f090ebaa5a 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -3474,9 +3474,12 @@ int __evsel__parse_sample(struct evsel *evsel, union perf_event *event,
 	if (event->header.type == PERF_RECORD_CALLCHAIN_DEFERRED) {
 		const u64 max_callchain_nr = UINT64_MAX / sizeof(u64);
 
+		OVERFLOW_CHECK_u64(&event->callchain_deferred.nr);
 		data->callchain = (struct ip_callchain *)&event->callchain_deferred.nr;
 		if (data->callchain->nr > max_callchain_nr)
 			goto out_efault;
+		OVERFLOW_CHECK(event->callchain_deferred.ips,
+			       data->callchain->nr * sizeof(u64), max_size);
 
 		data->deferred_cookie = event->callchain_deferred.cookie;
 
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v1 3/7] perf dso: Handle trailing zeros symmetrically in dso__build_id_equal()

[Ian Rogers]

dso__build_id_equal() allows a 20-byte dso_bid with trailing zeros to
match a shorter bid (such as an 8-byte or 16-byte build ID), but did not
handle the symmetric case where the lookup bid is 20 bytes padded with
trailing zeros (e.g. from BPF/perf kernel build ID extraction) and
dso_bid has the exact shorter size. Handle both directions symmetrically.

Fixes: f4e3381648be ("perf tools: Minimal DEFERRED_CALLCHAIN support")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/dso.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index ce98613868e7..e46cd614eb21 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1949,7 +1949,8 @@ bool dso__build_id_equal(const struct dso *dso, const struct build_id *bid)
 {
 	const struct build_id *dso_bid = dso__bid(dso);
 
-	if (dso_bid->size > bid->size && dso_bid->size == BUILD_ID_SIZE) {
+	if (dso_bid->size > bid->size && bid->size > 0 &&
+	    dso_bid->size == BUILD_ID_SIZE) {
 		/*
 		 * For the backward compatibility, it allows a build-id has
 		 * trailing zeros.
@@ -1959,6 +1960,13 @@ bool dso__build_id_equal(const struct dso *dso, const struct build_id *bid)
 				    dso_bid->size - bid->size);
 	}
 
+	if (bid->size > dso_bid->size && dso_bid->size > 0 &&
+	    bid->size == BUILD_ID_SIZE) {
+		return !memcmp(dso_bid->data, bid->data, dso_bid->size) &&
+			!memchr_inv(&bid->data[dso_bid->size], 0,
+				    bid->size - dso_bid->size);
+	}
+
 	return dso_bid->size == bid->size &&
 	       memcmp(dso_bid->data, bid->data, dso_bid->size) == 0;
 }
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v1 4/7] perf maps: Check map__start_cmp() in __maps__insert()

[Ian Rogers]

In __maps__insert(), maps_by_address_sorted is kept true if the previous
map's end is less than or equal to the new map's start. However, when a
map has an end of 0 (for example, an unmapped [0, 0) map or a map whose
end has not been set yet) and a [0, 0) map is inserted after it,
map__end(prev) <= map__start(new) evaluates to 0 <= 0 (true) even when
map__start_cmp(&prev, &new) > 0 (either because map__start(prev) > 0 or
because prev's pointer address is greater than new's pointer address).
This leaves maps_by_address_sorted set to true on an array that is not
ordered according to map__start_cmp(), causing bsearch() in
maps__by_address_index() to fail and return -1 during __maps__remove().

Also check map__start_cmp(&maps_by_address[nr_maps - 2], &new) <= 0 when
updating maps_by_address_sorted in __maps__insert().

Fixes: 659ad3492b91 ("perf maps: Switch from rbtree to lazily sorted array for addresses")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/maps.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index f808df2fe77b..675fe0223faa 100644
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -519,7 +519,8 @@ static int __maps__insert(struct maps *maps, struct map *new)
 		/* Sorted if maps were already sorted and this map starts after the last one. */
 		maps__set_maps_by_address_sorted(maps,
 			maps__maps_by_address_sorted(maps) &&
-			map__end(maps_by_address[nr_maps - 2]) <= map__start(new));
+			map__end(maps_by_address[nr_maps - 2]) <= map__start(new) &&
+			map__start_cmp(&maps_by_address[nr_maps - 2], &new) <= 0);
 		maps__set_maps_by_name_sorted(maps, false);
 	}
 	if (map__end(new) < map__start(new))
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v1 5/7] perf inject: Use perf_tool__init() for --vm-time-correlation

[Ian Rogers]

When cmd_inject() was converted to use perf_tool__init() in commit
a37c0436f379 ("perf inject: Use perf_tool__init()"), the
--vm-time-correlation branch in __cmd_inject() still called
memset(&inject->tool, 0, sizeof(inject->tool)) to clear the default
repipe callbacks. Zeroing out the struct overwrites all default stub
handlers set by perf_tool__init() with NULL, causing a NULL function
pointer dereference if any other event type is encountered.

Use perf_tool__init(&inject->tool, /*ordered_events=*/true) instead of
memset() so that un-overridden callbacks retain their default stub
handlers.

Fixes: a37c0436f379 ("perf inject: Use perf_tool__init()")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-inject.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index ce7562fc788c..c30f5aaabf4c 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -2464,12 +2464,11 @@ static int __cmd_inject(struct perf_inject *inject)
 		}
 	} else if (inject->itrace_synth_opts.vm_time_correlation) {
 		session->itrace_synth_opts = &inject->itrace_synth_opts;
-		memset(&inject->tool, 0, sizeof(inject->tool));
+		perf_tool__init(&inject->tool, /*ordered_events=*/true);
 		inject->tool.id_index	    = perf_event__process_id_index;
 		inject->tool.auxtrace_info  = perf_event__process_auxtrace_info;
 		inject->tool.auxtrace	    = perf_event__process_auxtrace;
 		inject->tool.auxtrace_error = perf_event__process_auxtrace_error;
-		inject->tool.ordered_events = true;
 		inject->tool.ordering_requires_timestamps = true;
 	} else if (inject->itrace_synth_opts.set) {
 		session->itrace_synth_opts = &inject->itrace_synth_opts;
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v1 6/7] perf inject: Set ordering_requires_timestamps before creating session

[Ian Rogers]

In cmd_inject(), inject.tool.ordering_requires_timestamps was initialized
after delegating tools (such as aslr_tool__new()) and __perf_session__new()
had already been called, leaving delegate tools with the uninitialized
value. Set ordering_requires_timestamps on inject.tool before creating
delegate tools and the session.

Fixes: febea9ec382f ("perf inject/aslr: Add ASLR tool infrastructure and MMAP tracking")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-inject.c | 49 ++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index c30f5aaabf4c..3f3108fd3981 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -2463,13 +2463,18 @@ static int __cmd_inject(struct perf_inject *inject)
 #endif
 		}
 	} else if (inject->itrace_synth_opts.vm_time_correlation) {
+		bool ordered_events = inject->tool.ordered_events;
+		bool ordering_requires_timestamps =
+			inject->tool.ordering_requires_timestamps;
+
 		session->itrace_synth_opts = &inject->itrace_synth_opts;
-		perf_tool__init(&inject->tool, /*ordered_events=*/true);
+		perf_tool__init(&inject->tool, ordered_events);
 		inject->tool.id_index	    = perf_event__process_id_index;
 		inject->tool.auxtrace_info  = perf_event__process_auxtrace_info;
 		inject->tool.auxtrace	    = perf_event__process_auxtrace;
 		inject->tool.auxtrace_error = perf_event__process_auxtrace_error;
-		inject->tool.ordering_requires_timestamps = true;
+		inject->tool.ordering_requires_timestamps =
+			ordering_requires_timestamps;
 	} else if (inject->itrace_synth_opts.set) {
 		session->itrace_synth_opts = &inject->itrace_synth_opts;
 		inject->itrace_synth_opts.inject = true;
@@ -2482,8 +2487,6 @@ static int __cmd_inject(struct perf_inject *inject)
 		inject->tool.aux	    = perf_event__drop_aux;
 		inject->tool.itrace_start   = perf_event__drop_aux;
 		inject->tool.aux_output_hw_id = perf_event__drop_aux;
-		inject->tool.ordered_events = true;
-		inject->tool.ordering_requires_timestamps = true;
 		/* Allow space in the header for new attributes */
 		output_data_offset = roundup(8192 + session->header.data_offset, 4096);
 		if (inject->strip)
@@ -2514,12 +2517,6 @@ static int __cmd_inject(struct perf_inject *inject)
 		inject->tool.finished_round	= host__finished_round;
 		/* Keep track of which CPU a VCPU is runnng on */
 		inject->tool.context_switch	= host__context_switch;
-		/*
-		 * Must order events to be able to obey finished round
-		 * ordering.
-		 */
-		inject->tool.ordered_events	= true;
-		inject->tool.ordering_requires_timestamps = true;
 		/* Set up a separate session to process guest perf.data file */
 		ret = guest_session__start(gs, name, session->data->force);
 		if (ret) {
@@ -2536,8 +2533,6 @@ static int __cmd_inject(struct perf_inject *inject)
 		inject->tool.exit	= perf_event__repipe_exit;
 		inject->tool.mmap	= perf_event__repipe_mmap;
 		inject->tool.mmap2	= perf_event__repipe_mmap2;
-		inject->tool.ordered_events = true;
-		inject->tool.ordering_requires_timestamps = true;
 	}
 
 	if (!inject->itrace_synth_opts.set)
@@ -2816,6 +2811,10 @@ int cmd_inject(int argc, const char **argv)
 	data.path = inject.input_name;
 
 	ordered_events = inject.jit_mode || inject.sched_stat ||
+		inject.itrace_synth_opts.vm_time_correlation ||
+		inject.itrace_synth_opts.set ||
+		inject.guest_session.perf_data_file ||
+		inject.convert_callchain ||
 		inject.build_id_style == BID_RWS__INJECT_HEADER_LAZY ||
 		inject.build_id_style == BID_RWS__MMAP2_BUILDID_LAZY;
 	perf_tool__init(&inject.tool, ordered_events);
@@ -2862,6 +2861,21 @@ int cmd_inject(int argc, const char **argv)
 	inject.tool.schedstat_domain	= perf_event__repipe_op2_synth;
 	inject.tool.dont_split_sample_group = true;
 	inject.tool.merge_deferred_callchains = false;
+	if (inject.jit_mode ||
+	    inject.itrace_synth_opts.vm_time_correlation ||
+	    inject.itrace_synth_opts.set ||
+	    inject.guest_session.perf_data_file ||
+	    inject.convert_callchain ||
+	    inject.build_id_style == BID_RWS__INJECT_HEADER_LAZY ||
+	    inject.build_id_style == BID_RWS__MMAP2_BUILDID_LAZY) {
+		/*
+		 * to make sure the mmap records are ordered correctly
+		 * and so that the correct especially due to jitted code
+		 * mmaps. We cannot generate the buildid hit list and
+		 * inject the jit mmaps at the same time for now.
+		 */
+		inject.tool.ordering_requires_timestamps = true;
+	}
 	if (inject.aslr) {
 		tool = aslr_tool__new(&inject.tool);
 		if (!tool) {
@@ -2932,16 +2946,6 @@ int cmd_inject(int argc, const char **argv)
 		}
 	}
 
-	if (inject.build_id_style == BID_RWS__INJECT_HEADER_LAZY ||
-	    inject.build_id_style == BID_RWS__MMAP2_BUILDID_LAZY) {
-		/*
-		 * to make sure the mmap records are ordered correctly
-		 * and so that the correct especially due to jitted code
-		 * mmaps. We cannot generate the buildid hit list and
-		 * inject the jit mmaps at the same time for now.
-		 */
-		inject.tool.ordering_requires_timestamps = true;
-	}
 	if (inject.build_id_style != BID_RWS__NONE && known_build_ids != NULL) {
 		inject.known_build_ids =
 			perf_inject__parse_known_build_ids(known_build_ids);
@@ -2978,7 +2982,6 @@ int cmd_inject(int argc, const char **argv)
 	if (inject.jit_mode) {
 		inject.tool.mmap2	   = perf_event__repipe_mmap2;
 		inject.tool.mmap	   = perf_event__repipe_mmap;
-		inject.tool.ordering_requires_timestamps = true;
 		/*
 		 * JIT MMAP injection injects all MMAP events in one go, so it
 		 * does not obey finished_round semantics.
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v1 7/7] perf session: Check lbr_stack for NULL in callchain__lbr_callstack_printf()

[Ian Rogers]

callchain__lbr_callstack_printf() can be called on an evsel with branch
callstack enabled even if a particular sample has no branch_stack payload
(sample->branch_stack == NULL). Check that lbr_stack is non-NULL before
dereferencing lbr_stack->nr.

Fixes: febea9ec382f ("perf inject/aslr: Add ASLR tool infrastructure and MMAP tracking")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/session.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index d9d06337933f..a58b4fe44b62 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1302,7 +1302,7 @@ static void callchain__lbr_callstack_printf(struct perf_sample *sample)
 			break;
 	}
 
-	if ((i != kernel_callchain_nr) && lbr_stack->nr) {
+	if ((i != kernel_callchain_nr) && lbr_stack && lbr_stack->nr) {
 		u64 total_nr;
 		/*
 		 * LBR callstack can only get user call chain,
-- 
2.56.0.307.g5fe7bd680c-goog