mirror of
https://github.com/torvalds/linux.git
synced 2026-07-27 17:47:41 +02:00
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 <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude:claude-opus-4.6-1m
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This commit is contained in:
parent
1e389b7639
commit
1a646a28a0
|
|
@ -8,7 +8,14 @@
|
|||
#include <linux/bpf.h>
|
||||
#include <sys/types.h> /* 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;
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user