]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
perf auxtrace: Harden auxtrace_error event handling
authorArnaldo Carvalho de Melo <acme@redhat.com>
Sat, 2 May 2026 16:18:45 +0000 (13:18 -0300)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Fri, 29 May 2026 14:44:33 +0000 (11:44 -0300)
Fix four issues in PERF_RECORD_AUXTRACE_ERROR handling:

1. auxtrace_error_name() takes a signed int parameter, but e->type
   is __u32.  A crafted value like 0xFFFFFFFF converts to -1, passes
   the bounds check, and causes a negative array index.  Fix by
   changing the parameter to unsigned int.

2. The msg field is printed via %s without a length bound.  The
   min_size table only guarantees fields up to msg (offset 48), so
   a truncated event has zero msg bytes within the event boundary.
   Compute the available msg length from header.size, cap at
   sizeof(e->msg), and use %.*s.

3. fmt >= 2 adds machine_pid and vcpu fields after msg[64].  Older
   files may have fmt >= 2 but an event size that doesn't include
   these fields.  Add a size check in the swap handler to downgrade
   fmt before the conditional field access, and a matching size
   guard in the fprintf path for native-endian events (which are
   mmap'd read-only and can't be modified in place).

4. python_process_auxtrace_error() had the same issues: msg was
   passed to tuple_set_string() unbounded, and machine_pid/vcpu
   were accessed unconditionally without checking fmt or event
   size.  Apply the same bounds checks.

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>
tools/perf/util/auxtrace.c
tools/perf/util/scripting-engines/trace-event-python.c
tools/perf/util/session.c

index a9f007d47c0b9cb3ab6211707a90bccd4dfbc02e..fcf564e0d777f8f910168cdef503c26337958137 100644 (file)
@@ -1774,7 +1774,7 @@ static const char * const auxtrace_error_type_name[] = {
        [PERF_AUXTRACE_ERROR_ITRACE] = "instruction trace",
 };
 
-static const char *auxtrace_error_name(int type)
+static const char *auxtrace_error_name(unsigned int type)
 {
        const char *error_type_name = NULL;
 
@@ -1790,6 +1790,7 @@ size_t perf_event__fprintf_auxtrace_error(union perf_event *event, FILE *fp)
        struct perf_record_auxtrace_error *e = &event->auxtrace_error;
        unsigned long long nsecs = e->time;
        const char *msg = e->msg;
+       int msg_max;
        int ret;
 
        ret = fprintf(fp, " %s error type %u",
@@ -1807,11 +1808,26 @@ size_t perf_event__fprintf_auxtrace_error(union perf_event *event, FILE *fp)
        if (!e->fmt)
                msg = (const char *)&e->time;
 
-       if (e->fmt >= 2 && e->machine_pid)
+       /* Bound msg to the bytes actually within the event, capped at the array size */
+       msg_max = (int)((void *)event + event->header.size - (void *)msg);
+       if (msg_max < 0)
+               msg_max = 0;
+       if (msg_max > (int)sizeof(e->msg))
+               msg_max = sizeof(e->msg);
+
+       /*
+        * Unlike the swap path which downgrades fmt in place,
+        * native-endian events are mmap'd read-only — check size
+        * instead to avoid accessing machine_pid/vcpu OOB.
+        */
+       if (e->fmt >= 2 &&
+           event->header.size >= offsetof(typeof(event->auxtrace_error), vcpu) +
+                                 sizeof(event->auxtrace_error.vcpu) &&
+           e->machine_pid)
                ret += fprintf(fp, " machine_pid %d vcpu %d", e->machine_pid, e->vcpu);
 
-       ret += fprintf(fp, " cpu %d pid %d tid %d ip %#"PRI_lx64" code %u: %s\n",
-                      e->cpu, e->pid, e->tid, e->ip, e->code, msg);
+       ret += fprintf(fp, " cpu %d pid %d tid %d ip %#"PRI_lx64" code %u: %.*s\n",
+                      e->cpu, e->pid, e->tid, e->ip, e->code, msg_max, msg);
        return ret;
 }
 
index 8edd2f36e5a95829dee41637938c33efa1949793..cee1f32d70225cc73bd9e0e1c3a0fd18bf852bd0 100644 (file)
@@ -1607,6 +1607,9 @@ static void python_process_auxtrace_error(struct perf_session *session __maybe_u
        const char *handler_name = "auxtrace_error";
        unsigned long long tm = e->time;
        const char *msg = e->msg;
+       s32 machine_pid = 0, vcpu = 0;
+       char msg_buf[MAX_AUXTRACE_ERROR_MSG + 1];
+       int msg_max;
        PyObject *handler, *t;
 
        handler = get_handler(handler_name);
@@ -1618,6 +1621,25 @@ static void python_process_auxtrace_error(struct perf_session *session __maybe_u
                msg = (const char *)&e->time;
        }
 
+       /* Bound msg to the bytes within the event, ensure NUL-termination */
+       msg_max = (int)((void *)event + event->header.size - (void *)msg);
+       if (msg_max <= 0) {
+               msg_buf[0] = '\0';
+       } else {
+               if (msg_max > (int)sizeof(msg_buf) - 1)
+                       msg_max = sizeof(msg_buf) - 1;
+               memcpy(msg_buf, msg, msg_max);
+               msg_buf[msg_max] = '\0';
+       }
+
+       /* Only access fmt >= 2 fields if the event is large enough */
+       if (e->fmt >= 2 &&
+           event->header.size >= offsetof(typeof(event->auxtrace_error), vcpu) +
+                                 sizeof(event->auxtrace_error.vcpu)) {
+               machine_pid = e->machine_pid;
+               vcpu = e->vcpu;
+       }
+
        t = tuple_new(11);
 
        tuple_set_u32(t, 0, e->type);
@@ -1627,10 +1649,10 @@ static void python_process_auxtrace_error(struct perf_session *session __maybe_u
        tuple_set_s32(t, 4, e->tid);
        tuple_set_u64(t, 5, e->ip);
        tuple_set_u64(t, 6, tm);
-       tuple_set_string(t, 7, msg);
+       tuple_set_string(t, 7, msg_buf);
        tuple_set_u32(t, 8, cpumode);
-       tuple_set_s32(t, 9, e->machine_pid);
-       tuple_set_s32(t, 10, e->vcpu);
+       tuple_set_s32(t, 9, machine_pid);
+       tuple_set_s32(t, 10, vcpu);
 
        call_object(handler, t, handler_name);
 
index 0fac8f4e0e22310fab428084a280734fa2fb5b4d..092fccbea8f8017e56fbfd44220bea81d4c08688 100644 (file)
@@ -766,8 +766,22 @@ static int perf_event__auxtrace_error_swap(union perf_event *event,
        if (event->auxtrace_error.fmt)
                event->auxtrace_error.time = bswap_64(event->auxtrace_error.time);
        if (event->auxtrace_error.fmt >= 2) {
-               event->auxtrace_error.machine_pid = bswap_32(event->auxtrace_error.machine_pid);
-               event->auxtrace_error.vcpu = bswap_32(event->auxtrace_error.vcpu);
+               /*
+                * fmt >= 2 adds machine_pid and vcpu after msg[64].
+                * Older files may have fmt >= 2 but an event size
+                * that doesn't include these fields — downgrade to
+                * avoid swapping out of bounds.
+                */
+               if (event->header.size < offsetof(typeof(event->auxtrace_error), vcpu) +
+                                        sizeof(event->auxtrace_error.vcpu)) {
+                       pr_warning("WARNING: PERF_RECORD_AUXTRACE_ERROR: fmt %u but event too small for machine_pid/vcpu (%u bytes), downgrading fmt\n",
+                                  event->auxtrace_error.fmt,
+                                  event->header.size);
+                       event->auxtrace_error.fmt = 1;
+               } else {
+                       event->auxtrace_error.machine_pid = bswap_32(event->auxtrace_error.machine_pid);
+                       event->auxtrace_error.vcpu = bswap_32(event->auxtrace_error.vcpu);
+               }
        }
        return 0;
 }