]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
perf header: Byte-swap build ID event pid and bounds check section entries
authorArnaldo Carvalho de Melo <acme@redhat.com>
Sat, 2 May 2026 15:53:52 +0000 (12:53 -0300)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Fri, 29 May 2026 14:44:32 +0000 (11:44 -0300)
perf_header__read_build_ids() swaps the event header fields for cross-endian
perf.data files but not bev.pid. This causes perf_session__findnew_machine()
to look up the wrong machine for guest VM build IDs, misattributing them.
Swap bev.pid alongside the header fields.

Also add a build_id_swap callback for stream-mode build ID events,
and validate NUL-termination of build_id.filename on the native-endian
delivery path (perf_session__process_user_event) — events with
unterminated filenames are skipped.

Harden perf_header__read_build_ids() against crafted perf.data files:

- Add overflow check on offset + size to prevent wrap past ULLONG_MAX.
- Reject bev.header.size == 0 which would loop forever.
- Reject bev.header.size > remaining section to prevent reading past
  the section boundary.
- Guard memcmp(filename, "nel.kallsyms]", 13) with len >= 13 to avoid
  reading uninitialized stack memory on short filenames.
- Force NUL-termination of filename before passing it to functions
  like machine__findnew_dso() that use strlen/strcmp.

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/header.c
tools/perf/util/session.c

index 967c3d8ff12c8676d38a16d2897ee27fcf95a8aa..c0b5c99f462ad925322bd39b780d06fe736d99a2 100644 (file)
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <errno.h>
 #include <inttypes.h>
+#include <limits.h>
 #include "string2.h"
 #include <sys/param.h>
 #include <sys/types.h>
@@ -2578,7 +2579,13 @@ static int perf_header__read_build_ids_abi_quirk(struct perf_header *header,
        } old_bev;
        struct perf_record_header_build_id bev;
        char filename[PATH_MAX];
-       u64 limit = offset + size;
+       u64 limit;
+
+       /* Prevent offset + size from wrapping past ULLONG_MAX */
+       if (size > ULLONG_MAX - offset)
+               return -1;
+
+       limit = offset + size;
 
        while (offset < limit) {
                ssize_t len;
@@ -2589,6 +2596,10 @@ static int perf_header__read_build_ids_abi_quirk(struct perf_header *header,
                if (header->needs_swap)
                        perf_event_header__bswap(&old_bev.header);
 
+               /* size == 0 loops forever; size > remaining reads past section */
+               if (old_bev.header.size == 0 || old_bev.header.size > limit - offset)
+                       return -1;
+
                len = old_bev.header.size - sizeof(old_bev);
                if (len < 0 || len >= PATH_MAX) {
                        pr_warning("invalid build_id filename length %zd\n", len);
@@ -2597,6 +2608,13 @@ static int perf_header__read_build_ids_abi_quirk(struct perf_header *header,
 
                if (readn(input, filename, len) != len)
                        return -1;
+               /*
+                * The file data may lack a null terminator, which could
+                * indicate a corrupt or crafted perf.data file.  Ensure
+                * filename is always a valid C string before passing it
+                * to functions like machine__findnew_dso().
+                */
+               filename[len] = '\0';
 
                bev.header = old_bev.header;
 
@@ -2624,17 +2642,32 @@ static int perf_header__read_build_ids(struct perf_header *header,
        struct perf_session *session = container_of(header, struct perf_session, header);
        struct perf_record_header_build_id bev;
        char filename[PATH_MAX];
-       u64 limit = offset + size, orig_offset = offset;
+       u64 limit, orig_offset = offset;
        int err = -1;
 
+       /* Prevent offset + size from wrapping past ULLONG_MAX */
+       if (size > ULLONG_MAX - offset)
+               return -1;
+
+       limit = offset + size;
+
        while (offset < limit) {
                ssize_t len;
 
                if (readn(input, &bev, sizeof(bev)) != sizeof(bev))
                        goto out;
 
-               if (header->needs_swap)
+               if (header->needs_swap) {
                        perf_event_header__bswap(&bev.header);
+                       bev.pid = bswap_32(bev.pid);
+               }
+
+               /*
+                * size == 0 would loop forever (offset never advances);
+                * size > remaining would read past the section boundary.
+                */
+               if (bev.header.size == 0 || bev.header.size > limit - offset)
+                       goto out;
 
                len = bev.header.size - sizeof(bev);
                if (len < 0 || len >= PATH_MAX) {
@@ -2644,6 +2677,13 @@ static int perf_header__read_build_ids(struct perf_header *header,
 
                if (readn(input, filename, len) != len)
                        goto out;
+               /*
+                * The file data may lack a null terminator, which could
+                * indicate a corrupt or crafted perf.data file.  Ensure
+                * filename is always a valid C string before passing it
+                * to functions like machine__findnew_dso().
+                */
+               filename[len] = '\0';
                /*
                 * The a1645ce1 changeset:
                 *
@@ -2657,7 +2697,9 @@ static int perf_header__read_build_ids(struct perf_header *header,
                 * '[kernel.kallsyms]' string for the kernel build-id has the
                 * first 4 characters chopped off (where the pid_t sits).
                 */
-               if (memcmp(filename, "nel.kallsyms]", 13) == 0) {
+               /* Guard short filenames against memcmp reading past the buffer */
+               if (len >= (ssize_t)sizeof("nel.kallsyms]") - 1 &&
+                   memcmp(filename, "nel.kallsyms]", sizeof("nel.kallsyms]") - 1) == 0) {
                        if (lseek(input, orig_offset, SEEK_SET) == (off_t)-1)
                                return -1;
                        return perf_header__read_build_ids_abi_quirk(header, input, offset, size);
index 8588e12f110fca70c7008aa5ec223affcc661822..0fac8f4e0e22310fab428084a280734fa2fb5b4d 100644 (file)
@@ -686,6 +686,25 @@ static int perf_event__hdr_attr_swap(union perf_event *event,
        return 0;
 }
 
+static int perf_event__build_id_swap(union perf_event *event,
+                                    bool sample_id_all)
+{
+       event->build_id.pid = bswap_32(event->build_id.pid);
+
+       if (sample_id_all) {
+               void *data = &event->build_id.filename;
+               void *end = (void *)event + event->header.size;
+               size_t len = strnlen(data, end - data);
+
+               /* See comment in perf_event__comm_swap() */
+               if (len == (size_t)(end - data))
+                       return -1;
+               data += PERF_ALIGN(len + 1, sizeof(u64));
+               swap_sample_id_all(event, data);
+       }
+       return 0;
+}
+
 static int perf_event__event_update_swap(union perf_event *event,
                                         bool sample_id_all __maybe_unused)
 {
@@ -1014,7 +1033,7 @@ static perf_event__swap_op perf_event__swap_ops[] = {
        [PERF_RECORD_HEADER_ATTR]         = perf_event__hdr_attr_swap,
        [PERF_RECORD_HEADER_EVENT_TYPE]   = perf_event__event_type_swap,
        [PERF_RECORD_HEADER_TRACING_DATA] = perf_event__tracing_data_swap,
-       [PERF_RECORD_HEADER_BUILD_ID]     = NULL,
+       [PERF_RECORD_HEADER_BUILD_ID]     = perf_event__build_id_swap,
        [PERF_RECORD_HEADER_FEATURE]      = perf_event__header_feature_swap,
        [PERF_RECORD_ID_INDEX]            = perf_event__all64_swap,
        [PERF_RECORD_AUXTRACE_INFO]       = perf_event__auxtrace_info_swap,
@@ -2004,6 +2023,12 @@ static s64 perf_session__process_user_event(struct perf_session *session,
                err = tool->tracing_data(tool, session, event);
                break;
        case PERF_RECORD_HEADER_BUILD_ID:
+               if (!perf_event__check_nul(event->build_id.filename,
+                                          (void *)event + event->header.size,
+                                          "HEADER_BUILD_ID")) {
+                       err = 0;
+                       break;
+               }
                err = tool->build_id(tool, session, event);
                break;
        case PERF_RECORD_FINISHED_ROUND: