]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
perf tools: Bounds check perf_event_attr fields against attr.size before printing
authorArnaldo Carvalho de Melo <acme@redhat.com>
Sat, 2 May 2026 17:20:14 +0000 (14:20 -0300)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Fri, 29 May 2026 14:44:33 +0000 (11:44 -0300)
perf_event_attr__fprintf() accessed all struct fields unconditionally,
but attrs from older perf.data files or BPF-captured syscall payloads
may have a smaller size than the current struct.  Fields beyond the
recorded size contain uninitialized or zero-filled data.

Add size-guarded macros (PRINT_ATTRn, PRINT_ATTRn_bf) that compare
each field's offset against attr->size before accessing it.

Guard the bitfield block (disabled, inherit, ... defer_output) with
attr_size >= 48.  These bitfields share a single __u64 at offset 40,
which is within PERF_ATTR_SIZE_VER0 for validated perf.data attrs,
but BPF-captured attrs from perf trace can have a smaller size when
the tracee passes a minimal struct to sys_perf_event_open.

Also fix the BPF trace path: when perf trace intercepts
sys_perf_event_open via BPF, the program copies PERF_ATTR_SIZE_VER0
bytes when the tracee passes size=0, but leaves the size field as 0.
Set attr->size to PERF_ATTR_SIZE_VER0 in the augmented syscall
handler so the bounds checks match the actual copied size.

Reported-by: sashiko-bot@kernel.org # Running on a local machine
Reviewed-by: Ian Rogers <irogers@google.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/trace/beauty/perf_event_open.c
tools/perf/util/perf_event_attr_fprintf.c

index c1c7445dcff994cbebfed462beae5d4c7c494547..6315b46bcdf02b8ce5cd4ce2b56098c3973e446f 100644 (file)
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: LGPL-2.1
+#include <string.h>
 #include "trace/beauty/beauty.h"
 #include "util/evsel_fprintf.h"
 #include <linux/perf_event.h>
@@ -80,7 +81,27 @@ static size_t perf_event_attr___scnprintf(struct perf_event_attr *attr, char *bf
 
 static size_t syscall_arg__scnprintf_augmented_perf_event_attr(struct syscall_arg *arg, char *bf, size_t size)
 {
-       return perf_event_attr___scnprintf((void *)arg->augmented.args->value, bf, size,
+       struct perf_event_attr *attr = (void *)arg->augmented.args->value;
+       struct perf_event_attr local_attr;
+
+       /*
+        * augmented_raw_syscalls.bpf.c (shipped with perf) copies
+        * PERF_ATTR_SIZE_VER0 bytes when the tracee passes size=0,
+        * but leaves the size field as 0.  The payload size is
+        * guaranteed by perf's own BPF program, not externally
+        * controllable.  Copy to a local so we can fix up size
+        * without writing to the potentially read-only augmented
+        * args buffer.
+        */
+       if (!attr->size) {
+               memcpy(&local_attr, attr, PERF_ATTR_SIZE_VER0);
+               memset((void *)&local_attr + PERF_ATTR_SIZE_VER0, 0,
+                      sizeof(local_attr) - PERF_ATTR_SIZE_VER0);
+               local_attr.size = PERF_ATTR_SIZE_VER0;
+               attr = &local_attr;
+       }
+
+       return perf_event_attr___scnprintf(attr, bf, size,
                                           trace__show_zeros(arg->trace));
 }
 
index 741c3d657a8b6ae7d455e19e73fee367413f7ed5..3933639d76c54bb3d4b9b95177395f7fb77fdf42 100644 (file)
@@ -275,24 +275,56 @@ static void __p_config_id(struct perf_pmu *pmu, char *buf, size_t size, u32 type
 #define p_type_id(val)         __p_type_id(buf, BUF_SIZE, pmu, val)
 #define p_config_id(val)       __p_config_id(pmu, buf, BUF_SIZE, attr->type, val)
 
-#define PRINT_ATTRn(_n, _f, _p, _a)                    \
-do {                                                   \
-       if (_a || attr->_f) {                           \
-               _p(attr->_f);                           \
-               ret += attr__fprintf(fp, _n, buf, priv);\
-       }                                               \
+#define PRINT_ATTRn(_n, _f, _p, _a)                                    \
+do {                                                                   \
+       if (attr_size >= offsetof(struct perf_event_attr, _f) +         \
+                        sizeof(attr->_f) &&                            \
+           (_a || attr->_f)) {                                         \
+               _p(attr->_f);                                           \
+               ret += attr__fprintf(fp, _n, buf, priv);                \
+       }                                                               \
+} while (0)
+
+/* bitfield members share an offset; most are within PERF_ATTR_SIZE_VER0 */
+#define PRINT_ATTRn_bf(_n, _f, _p, _a)                                 \
+do {                                                                   \
+       if (_a || attr->_f) {                                           \
+               _p(attr->_f);                                           \
+               ret += attr__fprintf(fp, _n, buf, priv);                \
+       }                                                               \
 } while (0)
 
 #define PRINT_ATTRf(_f, _p)    PRINT_ATTRn(#_f, _f, _p, false)
+#define PRINT_ATTRf_bf(_f, _p) PRINT_ATTRn_bf(#_f, _f, _p, false)
 
 int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
                             attr__fprintf_f attr__fprintf, void *priv)
 {
        struct perf_pmu *pmu = perf_pmus__find_by_type(attr->type);
+       /*
+        * size == 0 means ABI0 — the producer didn't set attr.size.
+        * perf_event__fprintf_attr() may pass the raw mmap'd event
+        * before the local copy, so default to PERF_ATTR_SIZE_VER0
+        * (the ABI0 footprint) to avoid reading past the attr into
+        * the ID array that follows it in HEADER_ATTR events.
+        */
+       u32 attr_size = attr->size ?: PERF_ATTR_SIZE_VER0;
        char buf[BUF_SIZE];
        int ret = 0;
 
-       if (!pmu && (attr->type == PERF_TYPE_HARDWARE || attr->type == PERF_TYPE_HW_CACHE)) {
+       /*
+        * Cap to what we understand: all callers store the attr in a
+        * buffer of sizeof(*attr) bytes (perf.data read path copies
+        * min(attr.size, sizeof), BPF augmented path copies into a
+        * fixed-size value[] array).  A spoofed attr->size larger
+        * than sizeof would cause PRINT_ATTRn to read past the
+        * actual buffer.
+        */
+       if (attr_size > sizeof(*attr))
+               attr_size = sizeof(*attr);
+
+       if (!pmu && attr_size >= offsetof(struct perf_event_attr, config) + sizeof(attr->config) &&
+           (attr->type == PERF_TYPE_HARDWARE || attr->type == PERF_TYPE_HW_CACHE)) {
                u32 extended_type = attr->config >> PERF_PMU_TYPE_SHIFT;
 
                if (extended_type)
@@ -306,45 +338,53 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
        PRINT_ATTRf(sample_type, p_sample_type);
        PRINT_ATTRf(read_format, p_read_format);
 
-       PRINT_ATTRf(disabled, p_unsigned);
-       PRINT_ATTRf(inherit, p_unsigned);
-       PRINT_ATTRf(pinned, p_unsigned);
-       PRINT_ATTRf(exclusive, p_unsigned);
-       PRINT_ATTRf(exclude_user, p_unsigned);
-       PRINT_ATTRf(exclude_kernel, p_unsigned);
-       PRINT_ATTRf(exclude_hv, p_unsigned);
-       PRINT_ATTRf(exclude_idle, p_unsigned);
-       PRINT_ATTRf(mmap, p_unsigned);
-       PRINT_ATTRf(comm, p_unsigned);
-       PRINT_ATTRf(freq, p_unsigned);
-       PRINT_ATTRf(inherit_stat, p_unsigned);
-       PRINT_ATTRf(enable_on_exec, p_unsigned);
-       PRINT_ATTRf(task, p_unsigned);
-       PRINT_ATTRf(watermark, p_unsigned);
-       PRINT_ATTRf(precise_ip, p_unsigned);
-       PRINT_ATTRf(mmap_data, p_unsigned);
-       PRINT_ATTRf(sample_id_all, p_unsigned);
-       PRINT_ATTRf(exclude_host, p_unsigned);
-       PRINT_ATTRf(exclude_guest, p_unsigned);
-       PRINT_ATTRf(exclude_callchain_kernel, p_unsigned);
-       PRINT_ATTRf(exclude_callchain_user, p_unsigned);
-       PRINT_ATTRf(mmap2, p_unsigned);
-       PRINT_ATTRf(comm_exec, p_unsigned);
-       PRINT_ATTRf(use_clockid, p_unsigned);
-       PRINT_ATTRf(context_switch, p_unsigned);
-       PRINT_ATTRf(write_backward, p_unsigned);
-       PRINT_ATTRf(namespaces, p_unsigned);
-       PRINT_ATTRf(ksymbol, p_unsigned);
-       PRINT_ATTRf(bpf_event, p_unsigned);
-       PRINT_ATTRf(aux_output, p_unsigned);
-       PRINT_ATTRf(cgroup, p_unsigned);
-       PRINT_ATTRf(text_poke, p_unsigned);
-       PRINT_ATTRf(build_id, p_unsigned);
-       PRINT_ATTRf(inherit_thread, p_unsigned);
-       PRINT_ATTRf(remove_on_exec, p_unsigned);
-       PRINT_ATTRf(sigtrap, p_unsigned);
-       PRINT_ATTRf(defer_callchain, p_unsigned);
-       PRINT_ATTRf(defer_output, p_unsigned);
+       /*
+        * All bitfields share a single __u64 right after read_format.
+        * BPF-captured attrs from perf trace may have a small size
+        * when the tracee passes a minimal struct, so skip the
+        * entire block when it's not covered.
+        */
+       if (attr_size >= offsetof(struct perf_event_attr, wakeup_events)) {
+               PRINT_ATTRf_bf(disabled, p_unsigned);
+               PRINT_ATTRf_bf(inherit, p_unsigned);
+               PRINT_ATTRf_bf(pinned, p_unsigned);
+               PRINT_ATTRf_bf(exclusive, p_unsigned);
+               PRINT_ATTRf_bf(exclude_user, p_unsigned);
+               PRINT_ATTRf_bf(exclude_kernel, p_unsigned);
+               PRINT_ATTRf_bf(exclude_hv, p_unsigned);
+               PRINT_ATTRf_bf(exclude_idle, p_unsigned);
+               PRINT_ATTRf_bf(mmap, p_unsigned);
+               PRINT_ATTRf_bf(comm, p_unsigned);
+               PRINT_ATTRf_bf(freq, p_unsigned);
+               PRINT_ATTRf_bf(inherit_stat, p_unsigned);
+               PRINT_ATTRf_bf(enable_on_exec, p_unsigned);
+               PRINT_ATTRf_bf(task, p_unsigned);
+               PRINT_ATTRf_bf(watermark, p_unsigned);
+               PRINT_ATTRf_bf(precise_ip, p_unsigned);
+               PRINT_ATTRf_bf(mmap_data, p_unsigned);
+               PRINT_ATTRf_bf(sample_id_all, p_unsigned);
+               PRINT_ATTRf_bf(exclude_host, p_unsigned);
+               PRINT_ATTRf_bf(exclude_guest, p_unsigned);
+               PRINT_ATTRf_bf(exclude_callchain_kernel, p_unsigned);
+               PRINT_ATTRf_bf(exclude_callchain_user, p_unsigned);
+               PRINT_ATTRf_bf(mmap2, p_unsigned);
+               PRINT_ATTRf_bf(comm_exec, p_unsigned);
+               PRINT_ATTRf_bf(use_clockid, p_unsigned);
+               PRINT_ATTRf_bf(context_switch, p_unsigned);
+               PRINT_ATTRf_bf(write_backward, p_unsigned);
+               PRINT_ATTRf_bf(namespaces, p_unsigned);
+               PRINT_ATTRf_bf(ksymbol, p_unsigned);
+               PRINT_ATTRf_bf(bpf_event, p_unsigned);
+               PRINT_ATTRf_bf(aux_output, p_unsigned);
+               PRINT_ATTRf_bf(cgroup, p_unsigned);
+               PRINT_ATTRf_bf(text_poke, p_unsigned);
+               PRINT_ATTRf_bf(build_id, p_unsigned);
+               PRINT_ATTRf_bf(inherit_thread, p_unsigned);
+               PRINT_ATTRf_bf(remove_on_exec, p_unsigned);
+               PRINT_ATTRf_bf(sigtrap, p_unsigned);
+               PRINT_ATTRf_bf(defer_callchain, p_unsigned);
+               PRINT_ATTRf_bf(defer_output, p_unsigned);
+       }
 
        PRINT_ATTRn("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned, false);
        PRINT_ATTRf(bp_type, p_unsigned);
@@ -359,9 +399,12 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
        PRINT_ATTRf(sample_max_stack, p_unsigned);
        PRINT_ATTRf(aux_sample_size, p_unsigned);
        PRINT_ATTRf(sig_data, p_unsigned);
-       PRINT_ATTRf(aux_start_paused, p_unsigned);
-       PRINT_ATTRf(aux_pause, p_unsigned);
-       PRINT_ATTRf(aux_resume, p_unsigned);
+       /* aux_{start_paused,pause,resume} are at byte 116, past VER0 */
+       if (attr_size >= offsetof(struct perf_event_attr, sig_data)) {
+               PRINT_ATTRf_bf(aux_start_paused, p_unsigned);
+               PRINT_ATTRf_bf(aux_pause, p_unsigned);
+               PRINT_ATTRf_bf(aux_resume, p_unsigned);
+       }
 
        return ret;
 }