From: Arnaldo Carvalho de Melo Date: Mon, 4 May 2026 10:37:22 +0000 (-0300) Subject: perf tools: Fix event_contains() macro to verify full field extent X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1a646a28a0063f1fb31589f7190f2b4fb613e413;p=thirdparty%2Fkernel%2Flinux.git perf tools: Fix event_contains() macro to verify full field extent event_contains() checked whether a field's start offset was within the event (header.size > offsetof), but not whether the full field fit. A crafted event with header.size = offsetof(field) + 1 would pass the check, but an 8-byte access (bswap_64, direct read) would overrun the event boundary by up to 7 bytes. Fix the macro to verify the complete field: header.size >= offsetof(field) + sizeof(field) Also update all callers that check event_contains(time_cycles) but access later fields (time_mask, cap_user_time_zero, cap_user_time_short) to check for cap_user_time_short — the last field accessed — so the entire extended block is verified: tsc.c, arm-spe.c, cs-etm.c, jitdump.c. Note: session.c's perf_event__time_conv_swap() also guards on time_cycles but accesses time_mask — a pre-existing issue not introduced by this macro change. It is fixed by a later patch in this series ("perf session: Add validated swap infrastructure with null-termination checks"), which decouples time_cycles and time_mask into independent per-field event_contains() checks. The struct assignment overread (session->time_conv = event->time_conv copies sizeof on a potentially shorter event) is separately fixed by "perf session: Use bounded copy for PERF_RECORD_TIME_CONV". Reported-by: sashiko-bot@kernel.org # Running on a local machine Reviewed-by: Ian Rogers Cc: Adrian Hunter Cc: Jiri Olsa Cc: Namhyung Kim Assisted-by: Claude:claude-opus-4.6-1m Signed-off-by: Arnaldo Carvalho de Melo --- diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h index 9043dc72b5d68..fdced574c889e 100644 --- a/tools/lib/perf/include/perf/event.h +++ b/tools/lib/perf/include/perf/event.h @@ -8,7 +8,14 @@ #include #include /* pid_t */ -#define event_contains(obj, mem) ((obj).header.size > offsetof(typeof(obj), mem)) +/* + * Verify the full field fits within the event, not just its start offset. + * Only valid for fixed-size scalar fields — for trailing arrays like + * filename[PATH_MAX], sizeof() evaluates to the declared maximum, not + * the actual string length, so this would spuriously return false. + */ +#define event_contains(obj, mem) \ + ((obj).header.size >= offsetof(typeof(obj), mem) + sizeof((obj).mem)) struct perf_record_mmap { struct perf_event_header header; diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c index 31f05f4678109..552f063f126e6 100644 --- a/tools/perf/util/arm-spe.c +++ b/tools/perf/util/arm-spe.c @@ -2002,7 +2002,7 @@ int arm_spe_process_auxtrace_info(union perf_event *event, spe->tc.time_mult = tc->time_mult; spe->tc.time_zero = tc->time_zero; - if (event_contains(*tc, time_cycles)) { + if (event_contains(*tc, cap_user_time_short)) { spe->tc.time_cycles = tc->time_cycles; spe->tc.time_mask = tc->time_mask; spe->tc.cap_user_time_zero = tc->cap_user_time_zero; diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 6ec48de294410..40c6ddfa8c8d9 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -3514,7 +3514,7 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, etm->tc.time_shift = tc->time_shift; etm->tc.time_mult = tc->time_mult; etm->tc.time_zero = tc->time_zero; - if (event_contains(*tc, time_cycles)) { + if (event_contains(*tc, cap_user_time_short)) { etm->tc.time_cycles = tc->time_cycles; etm->tc.time_mask = tc->time_mask; etm->tc.cap_user_time_zero = tc->cap_user_time_zero; diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c index 52e6ffac2b3e1..18fd84a82153c 100644 --- a/tools/perf/util/jitdump.c +++ b/tools/perf/util/jitdump.c @@ -409,7 +409,7 @@ static uint64_t convert_timestamp(struct jit_buf_desc *jd, uint64_t timestamp) * checks the event size and assigns these extended fields if these * fields are contained in the event. */ - if (event_contains(*time_conv, time_cycles)) { + if (event_contains(*time_conv, cap_user_time_short)) { tc.time_cycles = time_conv->time_cycles; tc.time_mask = time_conv->time_mask; tc.cap_user_time_zero = time_conv->cap_user_time_zero; diff --git a/tools/perf/util/tsc.c b/tools/perf/util/tsc.c index 511a517ce613d..ebf289bf6b9d9 100644 --- a/tools/perf/util/tsc.c +++ b/tools/perf/util/tsc.c @@ -127,7 +127,7 @@ size_t perf_event__fprintf_time_conv(union perf_event *event, FILE *fp) * when supported cap_user_time_short, for backward compatibility, * prints the extended fields only if they are contained in the event. */ - if (event_contains(*tc, time_cycles)) { + if (event_contains(*tc, cap_user_time_short)) { ret += fprintf(fp, "... Time Cycles %" PRI_lu64 "\n", tc->time_cycles); ret += fprintf(fp, "... Time Mask %#" PRI_lx64 "\n",