[PATCH 03/13] perf header: Sanity check HEADER_NRCPUS and HEADER_CPU_DOMAIN_INFO

[PATCH 03/13] perf header: Sanity check HEADER_NRCPUS and HEADER_CPU_DOMAIN_INFO

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

While working on some cleanups sashiko questioned about pre-existing
issues, namely lacking sanity checks for perf.data headers, add some
with the help of Claude.

Cc: Ian Rogers <irogers@google.com>
Cc: Swapnil Sapkal <swapnil.sapkal@amd.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 44 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 22c44b6f0b098f95..2d23dbc666b676be 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2722,6 +2722,13 @@ static int process_nrcpus(struct feat_fd *ff, void *data __maybe_unused)
 	ret = do_read_u32(ff, &nr_cpus_online);
 	if (ret)
 		return ret;
+
+	if (nr_cpus_online > nr_cpus_avail) {
+		pr_err("Invalid HEADER_NRCPUS: nr_cpus_online (%u) > nr_cpus_avail (%u)\n",
+		       nr_cpus_online, nr_cpus_avail);
+		return -1;
+	}
+
 	env->nr_cpus_avail = (int)nr_cpus_avail;
 	env->nr_cpus_online = (int)nr_cpus_online;
 	return 0;
@@ -3698,6 +3705,17 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
 	nra = env->nr_cpus_avail;
 	nr = env->nr_cpus_online;
 
+	if (nra == 0 || nr == 0) {
+		pr_err("Invalid HEADER_CPU_DOMAIN_INFO: missing HEADER_NRCPUS\n");
+		return -1;
+	}
+
+	if (ff->size < 2 * sizeof(u32) + nr * 2 * sizeof(u32)) {
+		pr_err("Invalid HEADER_CPU_DOMAIN_INFO: section too small (%zu) for %u CPUs\n",
+		       (size_t)ff->size, nr);
+		return -1;
+	}
+
 	cd_map = calloc(nra, sizeof(*cd_map));
 	if (!cd_map)
 		return -1;
@@ -3714,6 +3732,19 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
 	if (ret)
 		return ret;
 
+	/*
+	 * Sanity check: real systems have at most ~10 sched domain levels
+	 * (SMT, CLS, MC, PKG + NUMA hops). Reject obviously bogus values
+	 * from malformed perf.data files before they cause excessive
+	 * allocation in the per-CPU loop.
+	 */
+#define MAX_SCHED_DOMAINS 64
+	if (max_sched_domains > MAX_SCHED_DOMAINS) {
+		pr_err("Invalid HEADER_CPU_DOMAIN_INFO: max_sched_domains %u > %u\n",
+		       max_sched_domains, MAX_SCHED_DOMAINS);
+		return -1;
+	}
+
 	env->max_sched_domains = max_sched_domains;
 
 	for (i = 0; i < nr; i++) {
@@ -3725,6 +3756,11 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
 			return -1;
 		}
 
+		if (cd_map[cpu]) {
+			pr_err("Invalid HEADER_CPU_DOMAIN_INFO: duplicate cpu %u\n", cpu);
+			return -1;
+		}
+
 		cd_map[cpu] = zalloc(sizeof(*cd_map[cpu]));
 		if (!cd_map[cpu])
 			return -1;
@@ -3760,7 +3796,13 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
 			if (!d_info)
 				return -1;
 
-			assert(cd_map[cpu]->domains[domain] == NULL);
+			if (cd_map[cpu]->domains[domain]) {
+				pr_err("Invalid HEADER_CPU_DOMAIN_INFO: duplicate domain %u for cpu %u\n",
+				       domain, cpu);
+				free(d_info);
+				return -1;
+			}
+
 			cd_map[cpu]->domains[domain] = d_info;
 			d_info->domain = domain;
 
-- 
2.53.0

[PATCHES perf-tools-next v2 00/13] Sanity check perf.data headers

[Arnaldo Carvalho de Melo]

Hi,

        Sashiko recently mentioned the lack of sanity checking headers
in perf.data files, that with a fuzzy or maliciously crafted file could
make processing a perf.data file cause deleterious results.

        Add sanity checks and some arbitrarily generous upper limits to
headers, if some are found to be questionable, lets tweak them in
upcoming versions of this patchset.

        As registered in the Assisted-by tags in the patches, this was
done using Claude code to speed up development, hopefully no
hallucinations are present.

        I'm also trying to get some of these checks into review-prompts
skills, some of which were already merged, for instance:

  https://github.com/masoncl/review-prompts/commit/2bb2159893ea926e120105416e95629b9ef1508c

- Arnaldo

v2: Addressed some Sashiko comments for v1, some are valid but not
introduced by this patchset and will be addressed in a follow up series.

Arnaldo Carvalho de Melo (13):
  perf header: Validate nr_domains when reading HEADER_CPU_DOMAIN_INFO
  perf header: Bump up the max number of command line args allowed
  perf header: Sanity check HEADER_NRCPUS and HEADER_CPU_DOMAIN_INFO
  perf header: Sanity check HEADER_CPU_TOPOLOGY
  perf header: Sanity check HEADER_NUMA_TOPOLOGY
  perf header: Sanity check HEADER_MEM_TOPOLOGY
  perf header: Sanity check HEADER_PMU_MAPPINGS
  perf header: Sanity check HEADER_GROUP_DESC
  perf header: Sanity check HEADER_CACHE
  perf header: Sanity check HEADER_HYBRID_TOPOLOGY
  perf header: Sanity check HEADER_PMU_CAPS
  perf header: Sanity check HEADER_BPF_PROG_INFO
  perf header: Add sanity checks to HEADER_BPF_BTF processing

 tools/perf/util/header.c | 220 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 214 insertions(+), 6 deletions(-)

-- 
2.53.0

[PATCH 01/13] perf header: Validate nr_domains when reading HEADER_CPU_DOMAIN_INFO

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Further validate the HEADER_CPU_DOMAIN_INFO fields, this time checking
the nr_domains field.

Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index c6efddb70aee2904..a2796b72adc4d908 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3731,6 +3731,12 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
 		if (do_read_u32(ff, &nr_domains))
 			return -1;
 
+		if (nr_domains > max_sched_domains) {
+			pr_err("Invalid HEADER_CPU_DOMAIN_INFO: nr_domains %u > max_sched_domains (%u)\n",
+			       nr_domains, max_sched_domains);
+			return -1;
+		}
+
 		cd_map[cpu]->nr_domains = nr_domains;
 
 		cd_map[cpu]->domains = calloc(max_sched_domains, sizeof(*d_info));
-- 
2.53.0

[PATCH 02/13] perf header: Bump up the max number of command line args allowed

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

We need to do some upper limit validation, bump up the arbitrary limit
as per suggestion of Sashiko about command line wildcard expansion
ending up with more than 32768 args.

Link: https://sashiko.dev/#/patchset/20260408172846.96360-1-acme%40kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index a2796b72adc4d908..22c44b6f0b098f95 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2795,8 +2795,11 @@ process_event_desc(struct feat_fd *ff, void *data __maybe_unused)
 	return 0;
 }
 
-// Some reasonable arbitrary max for the number of command line arguments
-#define MAX_CMDLINE_NR 32768
+/*
+ * Some arbitrary max for the number of command line arguments,
+ * Wildcards can expand and end up with tons of command line args.
+ */
+#define MAX_CMDLINE_NR 1048576
 
 static int process_cmdline(struct feat_fd *ff, void *data __maybe_unused)
 {
-- 
2.53.0

[PATCH 03/13] perf header: Sanity check HEADER_NRCPUS and HEADER_CPU_DOMAIN_INFO

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

While working on some cleanups sashiko questioned about pre-existing
issues, namely lacking sanity checks for perf.data headers, add some
with the help of Claude.

Cc: Ian Rogers <irogers@google.com>
Cc: Swapnil Sapkal <swapnil.sapkal@amd.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 45 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 22c44b6f0b098f95..4cb748763c8a0741 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -63,6 +63,8 @@
 #include <event-parse.h>
 #endif
 
+#define MAX_SCHED_DOMAINS	64
+
 /*
  * magic2 = "PERFILE2"
  * must be a numerical value to let the endianness
@@ -2722,6 +2724,13 @@ static int process_nrcpus(struct feat_fd *ff, void *data __maybe_unused)
 	ret = do_read_u32(ff, &nr_cpus_online);
 	if (ret)
 		return ret;
+
+	if (nr_cpus_online > nr_cpus_avail) {
+		pr_err("Invalid HEADER_NRCPUS: nr_cpus_online (%u) > nr_cpus_avail (%u)\n",
+		       nr_cpus_online, nr_cpus_avail);
+		return -1;
+	}
+
 	env->nr_cpus_avail = (int)nr_cpus_avail;
 	env->nr_cpus_online = (int)nr_cpus_online;
 	return 0;
@@ -3698,6 +3707,17 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
 	nra = env->nr_cpus_avail;
 	nr = env->nr_cpus_online;
 
+	if (nra == 0 || nr == 0) {
+		pr_err("Invalid HEADER_CPU_DOMAIN_INFO: missing HEADER_NRCPUS\n");
+		return -1;
+	}
+
+	if (ff->size < 2 * sizeof(u32) + nr * 2 * sizeof(u32)) {
+		pr_err("Invalid HEADER_CPU_DOMAIN_INFO: section too small (%zu) for %u CPUs\n",
+		       (size_t)ff->size, nr);
+		return -1;
+	}
+
 	cd_map = calloc(nra, sizeof(*cd_map));
 	if (!cd_map)
 		return -1;
@@ -3714,6 +3734,18 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
 	if (ret)
 		return ret;
 
+	/*
+	 * Sanity check: real systems have at most ~10 sched domain levels
+	 * (SMT, CLS, MC, PKG + NUMA hops). Reject obviously bogus values
+	 * from malformed perf.data files before they cause excessive
+	 * allocation in the per-CPU loop.
+	 */
+	if (max_sched_domains > MAX_SCHED_DOMAINS) {
+		pr_err("Invalid HEADER_CPU_DOMAIN_INFO: max_sched_domains %u > %u\n",
+		       max_sched_domains, MAX_SCHED_DOMAINS);
+		return -1;
+	}
+
 	env->max_sched_domains = max_sched_domains;
 
 	for (i = 0; i < nr; i++) {
@@ -3725,6 +3757,11 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
 			return -1;
 		}
 
+		if (cd_map[cpu]) {
+			pr_err("Invalid HEADER_CPU_DOMAIN_INFO: duplicate cpu %u\n", cpu);
+			return -1;
+		}
+
 		cd_map[cpu] = zalloc(sizeof(*cd_map[cpu]));
 		if (!cd_map[cpu])
 			return -1;
@@ -3760,7 +3797,13 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
 			if (!d_info)
 				return -1;
 
-			assert(cd_map[cpu]->domains[domain] == NULL);
+			if (cd_map[cpu]->domains[domain]) {
+				pr_err("Invalid HEADER_CPU_DOMAIN_INFO: duplicate domain %u for cpu %u\n",
+				       domain, cpu);
+				free(d_info);
+				return -1;
+			}
+
 			cd_map[cpu]->domains[domain] = d_info;
 			d_info->domain = domain;
 
-- 
2.53.0

[PATCH 04/13] perf header: Sanity check HEADER_CPU_TOPOLOGY

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Add validation to process_cpu_topology() to harden against malformed
perf.data files:

- Verify nr_cpus_avail was initialized (HEADER_NRCPUS processed first)
- Bounds check sibling counts (cores, threads, dies) against nr_cpus_avail
- Fix two bare 'return -1' that leaked env->cpu by using 'goto free_cpu'

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 4cb748763c8a0741..acd6b07528e013a4 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2861,6 +2861,11 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
 	int cpu_nr = env->nr_cpus_avail;
 	u64 size = 0;
 
+	if (cpu_nr == 0) {
+		pr_err("Invalid HEADER_CPU_TOPOLOGY: missing HEADER_NRCPUS\n");
+		return -1;
+	}
+
 	env->cpu = calloc(cpu_nr, sizeof(*env->cpu));
 	if (!env->cpu)
 		return -1;
@@ -2868,6 +2873,12 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
 	if (do_read_u32(ff, &nr))
 		goto free_cpu;
 
+	if (nr > (u32)cpu_nr) {
+		pr_err("Invalid HEADER_CPU_TOPOLOGY: nr_sibling_cores (%u) > nr_cpus_avail (%d)\n",
+		       nr, cpu_nr);
+		goto free_cpu;
+	}
+
 	env->nr_sibling_cores = nr;
 	size += sizeof(u32);
 	if (strbuf_init(&sb, 128) < 0)
@@ -2887,7 +2898,13 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
 	env->sibling_cores = strbuf_detach(&sb, NULL);
 
 	if (do_read_u32(ff, &nr))
-		return -1;
+		goto free_cpu;
+
+	if (nr > (u32)cpu_nr) {
+		pr_err("Invalid HEADER_CPU_TOPOLOGY: nr_sibling_threads (%u) > nr_cpus_avail (%d)\n",
+		       nr, cpu_nr);
+		goto free_cpu;
+	}
 
 	env->nr_sibling_threads = nr;
 	size += sizeof(u32);
@@ -2936,7 +2953,13 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
 		return 0;
 
 	if (do_read_u32(ff, &nr))
-		return -1;
+		goto free_cpu;
+
+	if (nr > (u32)cpu_nr) {
+		pr_err("Invalid HEADER_CPU_TOPOLOGY: nr_sibling_dies (%u) > nr_cpus_avail (%d)\n",
+		       nr, cpu_nr);
+		goto free_cpu;
+	}
 
 	env->nr_sibling_dies = nr;
 	size += sizeof(u32);
-- 
2.53.0

[PATCH 05/13] perf header: Sanity check HEADER_NUMA_TOPOLOGY

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Add validation to process_numa_topology() to harden against malformed
perf.data files:

- Upper bound check on nr_nodes (max 4096)
- Minimum section size check before allocating

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index acd6b07528e013a4..2f405776e5013c13 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -63,6 +63,7 @@
 #include <event-parse.h>
 #endif
 
+#define MAX_NUMA_NODES		4096
 #define MAX_SCHED_DOMAINS	64
 
 /*
@@ -3005,6 +3006,18 @@ static int process_numa_topology(struct feat_fd *ff, void *data __maybe_unused)
 	if (do_read_u32(ff, &nr))
 		return -1;
 
+	if (nr > MAX_NUMA_NODES) {
+		pr_err("Invalid HEADER_NUMA_TOPOLOGY: nr_nodes (%u) > %u\n",
+		       nr, MAX_NUMA_NODES);
+		return -1;
+	}
+
+	if (ff->size < sizeof(u32) + nr * (sizeof(u32) + 2 * sizeof(u64))) {
+		pr_err("Invalid HEADER_NUMA_TOPOLOGY: section too small (%zu) for %u nodes\n",
+		       ff->size, nr);
+		return -1;
+	}
+
 	nodes = calloc(nr, sizeof(*nodes));
 	if (!nodes)
 		return -ENOMEM;
-- 
2.53.0

[PATCH 06/13] perf header: Sanity check HEADER_MEM_TOPOLOGY

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Add validation to process_mem_topology() to harden against malformed
perf.data files:

- Upper bound check on nr_nodes (reuses MAX_NUMA_NODES, 4096)
- Minimum section size check before allocating

This is particularly important here since nr is u64, making unbounded
values especially dangerous.

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 2f405776e5013c13..2eb909672f826ca4 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3308,6 +3308,18 @@ static int process_mem_topology(struct feat_fd *ff,
 	if (do_read_u64(ff, &nr))
 		return -1;
 
+	if (nr > MAX_NUMA_NODES) {
+		pr_err("Invalid HEADER_MEM_TOPOLOGY: nr_nodes (%llu) > %u\n",
+		       (unsigned long long)nr, MAX_NUMA_NODES);
+		return -1;
+	}
+
+	if (ff->size < 3 * sizeof(u64) + nr * 2 * sizeof(u64)) {
+		pr_err("Invalid HEADER_MEM_TOPOLOGY: section too small (%zu) for %llu nodes\n",
+		       ff->size, (unsigned long long)nr);
+		return -1;
+	}
+
 	nodes = calloc(nr, sizeof(*nodes));
 	if (!nodes)
 		return -1;
-- 
2.53.0

[PATCH 07/13] perf header: Sanity check HEADER_PMU_MAPPINGS

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Add upper bound check on pmu_num in process_pmu_mappings() to harden
against malformed perf.data files (max 4096).

Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 2eb909672f826ca4..77035d9b138cb3cd 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -64,6 +64,7 @@
 #endif
 
 #define MAX_NUMA_NODES		4096
+#define MAX_PMU_MAPPINGS	4096
 #define MAX_SCHED_DOMAINS	64
 
 /*
@@ -3069,6 +3070,18 @@ static int process_pmu_mappings(struct feat_fd *ff, void *data __maybe_unused)
 		return 0;
 	}
 
+	if (pmu_num > MAX_PMU_MAPPINGS) {
+		pr_err("Invalid HEADER_PMU_MAPPINGS: pmu_num (%u) > %u\n",
+		       pmu_num, MAX_PMU_MAPPINGS);
+		return -1;
+	}
+
+	if (ff->size < sizeof(u32) + pmu_num * 2 * sizeof(u32)) {
+		pr_err("Invalid HEADER_PMU_MAPPINGS: section too small (%zu) for %u PMUs\n",
+		       ff->size, pmu_num);
+		return -1;
+	}
+
 	env->nr_pmu_mappings = pmu_num;
 	if (strbuf_init(&sb, 128) < 0)
 		return -1;
-- 
2.53.0

[PATCH 08/13] perf header: Sanity check HEADER_GROUP_DESC

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Add upper bound check on nr_groups in process_group_desc() to harden
against malformed perf.data files (max 32768), and move the env
assignment after validation.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 77035d9b138cb3cd..993e20debd5ca315 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -63,6 +63,7 @@
 #include <event-parse.h>
 #endif
 
+#define MAX_GROUP_DESC		32768
 #define MAX_NUMA_NODES		4096
 #define MAX_PMU_MAPPINGS	4096
 #define MAX_SCHED_DOMAINS	64
@@ -3132,12 +3133,25 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 	if (do_read_u32(ff, &nr_groups))
 		return -1;
 
-	env->nr_groups = nr_groups;
 	if (!nr_groups) {
 		pr_debug("group desc not available\n");
 		return 0;
 	}
 
+	if (nr_groups > MAX_GROUP_DESC) {
+		pr_err("Invalid HEADER_GROUP_DESC: nr_groups (%u) > %u\n",
+		       nr_groups, MAX_GROUP_DESC);
+		return -1;
+	}
+
+	if (ff->size < sizeof(u32) + nr_groups * 3 * sizeof(u32)) {
+		pr_err("Invalid HEADER_GROUP_DESC: section too small (%zu) for %u groups\n",
+		       ff->size, nr_groups);
+		return -1;
+	}
+
+	env->nr_groups = nr_groups;
+
 	desc = calloc(nr_groups, sizeof(*desc));
 	if (!desc)
 		return -1;
-- 
2.53.0

[PATCH 09/13] perf header: Sanity check HEADER_CACHE

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Add upper bound check on cache entry count in process_cache() to harden
against malformed perf.data files (max 32768).

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 993e20debd5ca315..749a522fe057e739 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -63,6 +63,7 @@
 #include <event-parse.h>
 #endif
 
+#define MAX_CACHE_ENTRIES	32768
 #define MAX_GROUP_DESC		32768
 #define MAX_NUMA_NODES		4096
 #define MAX_PMU_MAPPINGS	4096
@@ -3243,6 +3244,18 @@ static int process_cache(struct feat_fd *ff, void *data __maybe_unused)
 	if (do_read_u32(ff, &cnt))
 		return -1;
 
+	if (cnt > MAX_CACHE_ENTRIES) {
+		pr_err("Invalid HEADER_CACHE: cnt (%u) > %u\n",
+		       cnt, MAX_CACHE_ENTRIES);
+		return -1;
+	}
+
+	if (ff->size < 2 * sizeof(u32) + cnt * 7 * sizeof(u32)) {
+		pr_err("Invalid HEADER_CACHE: section too small (%zu) for %u entries\n",
+		       ff->size, cnt);
+		return -1;
+	}
+
 	caches = calloc(cnt, sizeof(*caches));
 	if (!caches)
 		return -1;
-- 
2.53.0

[PATCH 10/13] perf header: Sanity check HEADER_HYBRID_TOPOLOGY

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Add upper bound check on nr_nodes in process_hybrid_topology() to
harden against malformed perf.data files (reuses MAX_PMU_MAPPINGS,
4096).

Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 749a522fe057e739..a609fc7d959fae04 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3450,6 +3450,18 @@ static int process_hybrid_topology(struct feat_fd *ff,
 	if (do_read_u32(ff, &nr))
 		return -1;
 
+	if (nr > MAX_PMU_MAPPINGS) {
+		pr_err("Invalid HEADER_HYBRID_TOPOLOGY: nr_nodes (%u) > %u\n",
+		       nr, MAX_PMU_MAPPINGS);
+		return -1;
+	}
+
+	if (ff->size < sizeof(u32) + nr * 2 * sizeof(u32)) {
+		pr_err("Invalid HEADER_HYBRID_TOPOLOGY: section too small (%zu) for %u nodes\n",
+		       ff->size, nr);
+		return -1;
+	}
+
 	nodes = calloc(nr, sizeof(*nodes));
 	if (!nodes)
 		return -ENOMEM;
-- 
2.53.0

[PATCH 11/13] perf header: Sanity check HEADER_PMU_CAPS

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Add upper bound checks in PMU capabilities processing to harden against
malformed perf.data files:

- nr_pmu bounded to MAX_PMU_MAPPINGS (4096) in process_pmu_caps()
- nr_pmu_caps bounded to MAX_PMU_CAPS (512) in __process_pmu_caps()

Cc: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index a609fc7d959fae04..37c1afbc081672f1 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -66,6 +66,7 @@
 #define MAX_CACHE_ENTRIES	32768
 #define MAX_GROUP_DESC		32768
 #define MAX_NUMA_NODES		4096
+#define MAX_PMU_CAPS		512
 #define MAX_PMU_MAPPINGS	4096
 #define MAX_SCHED_DOMAINS	64
 
@@ -3677,6 +3678,12 @@ static int __process_pmu_caps(struct feat_fd *ff, int *nr_caps,
 	if (!nr_pmu_caps)
 		return 0;
 
+	if (nr_pmu_caps > MAX_PMU_CAPS) {
+		pr_err("Invalid pmu caps: nr_pmu_caps (%u) > %u\n",
+		       nr_pmu_caps, MAX_PMU_CAPS);
+		return -1;
+	}
+
 	*caps = calloc(nr_pmu_caps, sizeof(char *));
 	if (!*caps)
 		return -1;
@@ -3754,6 +3761,18 @@ static int process_pmu_caps(struct feat_fd *ff, void *data __maybe_unused)
 		return 0;
 	}
 
+	if (nr_pmu > MAX_PMU_MAPPINGS) {
+		pr_err("Invalid HEADER_PMU_CAPS: nr_pmu (%u) > %u\n",
+		       nr_pmu, MAX_PMU_MAPPINGS);
+		return -1;
+	}
+
+	if (ff->size < sizeof(u32) + nr_pmu * sizeof(u32)) {
+		pr_err("Invalid HEADER_PMU_CAPS: section too small (%zu) for %u PMUs\n",
+		       ff->size, nr_pmu);
+		return -1;
+	}
+
 	pmu_caps = calloc(nr_pmu, sizeof(*pmu_caps));
 	if (!pmu_caps)
 		return -ENOMEM;
-- 
2.53.0

[PATCH 12/13] perf header: Sanity check HEADER_BPF_PROG_INFO

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Add validation to process_bpf_prog_info() to harden against malformed
perf.data files:

- Upper bound on BPF program count (max 131072)
- Upper bound on per-program data_len (max 256MB)

Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 37c1afbc081672f1..705f1ab44bc93486 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -63,6 +63,8 @@
 #include <event-parse.h>
 #endif
 
+#define MAX_BPF_DATA_LEN	(256 * 1024 * 1024)
+#define MAX_BPF_PROGS		131072
 #define MAX_CACHE_ENTRIES	32768
 #define MAX_GROUP_DESC		32768
 #define MAX_NUMA_NODES		4096
@@ -3525,6 +3527,18 @@ static int process_bpf_prog_info(struct feat_fd *ff __maybe_unused, void *data _
 	if (do_read_u32(ff, &count))
 		return -1;
 
+	if (count > MAX_BPF_PROGS) {
+		pr_err("Invalid HEADER_BPF_PROG_INFO: count (%u) > %u\n",
+		       count, MAX_BPF_PROGS);
+		return -1;
+	}
+
+	if (ff->size < sizeof(u32) + count * (2 * sizeof(u32) + sizeof(u64))) {
+		pr_err("Invalid HEADER_BPF_PROG_INFO: section too small (%zu) for %u entries\n",
+		       ff->size, count);
+		return -1;
+	}
+
 	down_write(&env->bpf_progs.lock);
 
 	for (i = 0; i < count; ++i) {
@@ -3542,6 +3556,12 @@ static int process_bpf_prog_info(struct feat_fd *ff __maybe_unused, void *data _
 			goto out;
 		}
 
+		if (data_len > MAX_BPF_DATA_LEN) {
+			pr_warning("Invalid HEADER_BPF_PROG_INFO: data_len (%u) too large\n",
+				   data_len);
+			goto out;
+		}
+
 		info_linear = malloc(sizeof(struct perf_bpil) +
 				     data_len);
 		if (!info_linear)
-- 
2.53.0

[PATCH 13/13] perf header: Add sanity checks to HEADER_BPF_BTF processing

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Validate the BTF entry count and individual data sizes when reading
HEADER_BPF_BTF from perf.data files to prevent excessive memory
allocation from malformed files.

Reuses the MAX_BPF_PROGS (131072) and MAX_BPF_DATA_LEN (256 MB)
limits from HEADER_BPF_PROG_INFO processing.

Cc: Song Liu <song@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Assisted-by: Claude Code:claude-opus-4-6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 705f1ab44bc93486..f30e48eb3fc32da2 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3622,6 +3622,17 @@ static int process_bpf_btf(struct feat_fd *ff  __maybe_unused, void *data __mayb
 	if (do_read_u32(ff, &count))
 		return -1;
 
+	if (count > MAX_BPF_PROGS) {
+		pr_err("bpf btf count %u too large (max %u)\n", count, MAX_BPF_PROGS);
+		return -1;
+	}
+
+	if (ff->size < sizeof(u32) + count * 2 * sizeof(u32)) {
+		pr_err("Invalid HEADER_BPF_BTF: section too small (%zu) for %u entries\n",
+		       ff->size, count);
+		return -1;
+	}
+
 	down_write(&env->bpf_progs.lock);
 
 	for (i = 0; i < count; ++i) {
@@ -3632,6 +3643,12 @@ static int process_bpf_btf(struct feat_fd *ff  __maybe_unused, void *data __mayb
 		if (do_read_u32(ff, &data_size))
 			goto out;
 
+		if (data_size > MAX_BPF_DATA_LEN) {
+			pr_err("bpf btf data size %u too large (max %u)\n",
+			       data_size, MAX_BPF_DATA_LEN);
+			goto out;
+		}
+
 		node = malloc(sizeof(struct btf_node) + data_size);
 		if (!node)
 			goto out;
-- 
2.53.0

Re: [PATCH 08/13] perf header: Sanity check HEADER_GROUP_DESC

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [High] Zero-length string allocations and missing null-termination lead to an out-of-bounds read in `strcmp`.
--

commit c734f9dc578da0aba3155da45962b841c79c7260
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf header: Sanity check HEADER_GROUP_DESC

This commit adds an upper bound check on nr_groups in process_group_desc()
to harden against malformed perf.data files. It also moves the environment
assignment after validation.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 77035d9b138cb..993e20debd5ca 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c

[ ... ]

> @@ -3132,12 +3133,25 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
>  	if (do_read_u32(ff, &nr_groups))
>  		return -1;
>  
> -	env->nr_groups = nr_groups;
>  	if (!nr_groups) {
>  		pr_debug("group desc not available\n");
>  		return 0;
>  	}
>  
> +	if (nr_groups > MAX_GROUP_DESC) {
> +		pr_err("Invalid HEADER_GROUP_DESC: nr_groups (%u) > %u\n",
> +		       nr_groups, MAX_GROUP_DESC);
> +		return -1;
> +	}
> +
> +	if (ff->size < sizeof(u32) + nr_groups * 3 * sizeof(u32)) {
> +		pr_err("Invalid HEADER_GROUP_DESC: section too small (%zu) for %u groups\n",
> +		       ff->size, nr_groups);
> +		return -1;
> +	}
> +
> +	env->nr_groups = nr_groups;
> +
>  	desc = calloc(nr_groups, sizeof(*desc));
>  	if (!desc)
>  		return -1;

Since this patch hardens the parser against malformed files, should there
also be validation on the strings returned by do_read_string(ff) later in
this function?

If a malformed perf.data file provides a string length of 0,
do_read_string() can call malloc(0), which on systems like glibc
returns a valid 0-byte allocation. Because 0 bytes are requested,
the underlying read succeeds but provides no null terminator. It
also does not appear to enforce null-termination for lengths > 0.

When the code subsequently evaluates the string:

    if (strcmp(desc[i].name, "{anon_group}")) {
        evsel->group_name = desc[i].name;

Can this lead to an out-of-bounds read and crash the perf utility if
desc[i].name lacks a null terminator?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260410220905.200051-1-acme@kernel.org?part=8

Re: [PATCH 05/13] perf header: Sanity check HEADER_NUMA_TOPOLOGY

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [Low] The minimum section size calculation for HEADER_NUMA_TOPOLOGY omits the mandatory string length field per node.
--

commit 5064a9beb88eb8009f39a18c54d4d42f61a38a69
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf header: Sanity check HEADER_NUMA_TOPOLOGY

This commit adds validation to process_numa_topology() to harden against
malformed perf.data files. It introduces an upper bound check on nr_nodes
and a minimum section size check before allocating memory.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index acd6b07528e01..2f405776e5013 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c

[ ... ]

> @@ -3005,6 +3006,18 @@ static int process_numa_topology(struct feat_fd *ff, void *data __maybe_unused)
>  	if (do_read_u32(ff, &nr))
>  		return -1;
>  
> +	if (nr > MAX_NUMA_NODES) {
> +		pr_err("Invalid HEADER_NUMA_TOPOLOGY: nr_nodes (%u) > %u\n",
> +		       nr, MAX_NUMA_NODES);
> +		return -1;
> +	}
> +
> +	if (ff->size < sizeof(u32) + nr * (sizeof(u32) + 2 * sizeof(u64))) {

Does this calculation omit the mandatory string length field for each node?

The subsequent parsing loop reads the node ID, memory totals, and a CPU map
string:

        if (do_read_u32(ff, &n->node))
                goto error;
        if (do_read_u64(ff, &n->mem_total))
                goto error;
        if (do_read_u64(ff, &n->mem_free))
                goto error;
        str = do_read_string(ff);

Since do_read_string() always reads a u32 length prefix before the string
content, the true minimum size per node appears to be 24 bytes rather than 20
bytes (sizeof(u32) + 2 * sizeof(u64) + sizeof(u32)).

While truncated files will still safely fail later during the actual read
operations, this calculation allows slightly truncated files to bypass the
new validation check.

> +		pr_err("Invalid HEADER_NUMA_TOPOLOGY: section too small (%zu) for %u nodes\n",
> +		       ff->size, nr);
> +		return -1;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260410220905.200051-1-acme@kernel.org?part=5

Re: [PATCH 06/13] perf header: Sanity check HEADER_MEM_TOPOLOGY

[sashiko-bot]

Sashiko AI review found 2 potential issue(s):
- [Critical] Heap Buffer Overflow via Integer Truncation in do_read_bitmap()
- [Low] Inaccurate bounds check under-approximates minimum section size
--

commit 005d03d0f059f4f346c521c1c989a61ca017de89
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf header: Sanity check HEADER_MEM_TOPOLOGY

This commit adds validation to process_mem_topology() to protect against
malformed perf.data files. It introduces an upper bound check on nr_nodes
using MAX_NUMA_NODES and verifies the section size is large enough before
allocating memory.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
[ ... ]
> @@ -3308,6 +3308,18 @@ static int process_mem_topology(struct feat_fd *ff,
>  	if (do_read_u64(ff, &nr))
>  		return -1;
>  
> +	if (nr > MAX_NUMA_NODES) {
> +		pr_err("Invalid HEADER_MEM_TOPOLOGY: nr_nodes (%llu) > %u\n",
> +		       (unsigned long long)nr, MAX_NUMA_NODES);
> +		return -1;
> +	}
> +
> +	if (ff->size < 3 * sizeof(u64) + nr * 2 * sizeof(u64)) {

Does the per-node size calculation introduce a regression by
under-approximating the minimum section size?

Each memory node actually consumes at least 3 u64 fields because
do_read_bitmap() also reads a u64 size field before its payload:

process_mem_topology() {
    ...
        _R(node)
        _R(size)
        ...
        if (do_read_bitmap(ff, &n.set, &n.size))
    ...
}

And do_read_bitmap() reads the size:

do_read_bitmap() {
    ...
    ret = do_read_u64(ff, &size);
    ...
}

Should this check instead be nr * 3 * sizeof(u64) to account for the bitmap
size field?

> +		pr_err("Invalid HEADER_MEM_TOPOLOGY: section too small (%zu) for %llu nodes\n",
> +		       ff->size, (unsigned long long)nr);
> +		return -1;
> +	}
> +
>  	nodes = calloc(nr, sizeof(*nodes));

While this patch hardens topology parsing, is there a regression further down
in the parsing logic that could cause an out-of-bounds write?

When process_mem_topology() calls do_read_bitmap(), a 64-bit size is read and
passed to bitmap_zalloc():

do_read_bitmap() {
    u64 size, *p;
    ...
    ret = do_read_u64(ff, &size);
    if (ret)
        return ret;

    set = bitmap_zalloc(size);
    ...
}

If bitmap_zalloc() takes a 32-bit integer for the number of bits, a size
exceeding 32 bits would be truncated, resulting in a small allocation.

The subsequent loop then calculates its bounds using the full 64-bit value:

    for (i = 0; (u64) i < BITS_TO_U64(size); i++) {
        ret = do_read_u64(ff, p + i);
        ...
    }

Can this loop write past the end of the truncated allocation if a large size is
supplied?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260410220905.200051-1-acme@kernel.org?part=6

Re: [PATCH 07/13] perf header: Sanity check HEADER_PMU_MAPPINGS

[sashiko-bot]

Sashiko AI review found 2 potential issue(s):
- [High] State corruption on error paths leads to a guaranteed NULL pointer dereference when parsing malformed files.
- [High] Missing null-termination check on strings read from the file leads to an out-of-bounds read in `strbuf_addf`.
--

commit 5a25c2d6480a45a3a7a4134af439a72344304a5c
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf header: Sanity check HEADER_PMU_MAPPINGS

This commit adds an upper bound check on pmu_num and a section size check
in process_pmu_mappings() to harden against malformed perf.data files.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -3069,6 +3070,18 @@ static int process_pmu_mappings(struct feat_fd *ff, void *data __maybe_unused)
>  		return 0;
>  	}
>  
> +	if (pmu_num > MAX_PMU_MAPPINGS) {
> +		pr_err("Invalid HEADER_PMU_MAPPINGS: pmu_num (%u) > %u\n",
> +		       pmu_num, MAX_PMU_MAPPINGS);
> +		return -1;
> +	}
> +
> +	if (ff->size < sizeof(u32) + pmu_num * 2 * sizeof(u32)) {
> +		pr_err("Invalid HEADER_PMU_MAPPINGS: section too small (%zu) for %u PMUs\n",
> +		       ff->size, pmu_num);
> +		return -1;
> +	}
> +
>  	env->nr_pmu_mappings = pmu_num;
>  	if (strbuf_init(&sb, 128) < 0)
>  		return -1;

This isn't a bug introduced by this patch, but if strbuf_init() fails or if
an error occurs in the while loop, this function returns -1 but leaves
env->nr_pmu_mappings set to pmu_num. 

Because the caller perf_session__read_header() silently ignores the -1
return value from section processing, the session initialization succeeds.

Later, functions like perf_env__has_pmu_mapping() will iterate
env->nr_pmu_mappings times over the NULL env->pmu_mappings pointer:

bool perf_env__has_pmu_mapping(struct perf_env *env, const char *pmu_name)
{
	char *pmu_mapping = env->pmu_mappings, *colon;

	for (int i = 0; i < env->nr_pmu_mappings; ++i) {
		if (strtoul(pmu_mapping, &colon, 0) == ULONG_MAX || *colon != ':')
			goto out_error;

Could this lead to a NULL pointer dereference when passing pmu_mapping to
strtoul()? Should the assignment to env->nr_pmu_mappings be deferred
until env->pmu_mappings is successfully constructed?

>  	while (pmu_num) {
>  		if (do_read_u32(ff, &type))
>  			goto error;
>  
>  		name = do_read_string(ff);
>  		if (!name)
>  			goto error;
>  
>  		if (strbuf_addf(&sb, "%u:%s", type, name) < 0)

This isn't a bug introduced by this patch, but does do_read_string() ensure
null-termination of the read string?

If a malformed perf.data file provides a string exactly equal to len without
a trailing null byte, passing the buffer to strbuf_addf() could cause
vsnprintf to read past the end of the allocated buffer.

Could this lead to a heap out-of-bounds read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260410220905.200051-1-acme@kernel.org?part=7

Re: [PATCH 02/13] perf header: Bump up the max number of command line args allowed

[sashiko-bot]

Sashiko AI review found 2 potential issue(s):
- [High] Heap buffer overflow in process_cmdline due to unvalidated section size.
- [High] Out-of-bounds heap read and write due to missing null-termination in do_read_string.
--

commit 9eedbd20bb926c7a91d560c07ac34934b0d810bb
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf header: Bump up the max number of command line args allowed

This commit increases the arbitrary maximum limit of command line arguments
allowed when processing perf headers to accommodate wildcard expansions.

Link: https://sashiko.dev/#/patchset/20260408172846.96360-1-acme%40kernel.org

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2795,8 +2795,11 @@ process_event_desc(struct feat_fd *ff, void *data __maybe_unused)

[ ... ]

> -// Some reasonable arbitrary max for the number of command line arguments
> -#define MAX_CMDLINE_NR 32768
> +/*
> + * Some arbitrary max for the number of command line arguments,
> + * Wildcards can expand and end up with tons of command line args.
> + */
> +#define MAX_CMDLINE_NR 1048576
>  
>  static int process_cmdline(struct feat_fd *ff, void *data __maybe_unused)
>  {
>  	struct perf_env *env = &ff->ph->env;
>  	char *str, *cmdline = NULL, **argv = NULL;
>  	u32 nr, i, len = 0;
>  
>  	if (do_read_u32(ff, &nr))
>  		return -1;
>  
>  	if (nr > MAX_CMDLINE_NR)
>  		return -1;
>  
>  	env->nr_cmdline = nr;
>  
>  	cmdline = zalloc(ff->size + nr + 1);

This isn't a bug introduced by this commit, but could this allocation allow a
heap buffer overflow when parsing an untrusted perf.data file?

The buffer size relies on ff->size, which originates from the attacker
controlled section->size in the file header. If do_read_string() reads from
the file descriptor without enforcing that the bytes read stay within
ff->size, could an attacker set a tiny section->size to force a small
allocation, but provide large strings in the payload?

If so, the subsequent memcpy(argv[i], str, strlen(str) + 1) in
process_cmdline() could write unbounded data past the end of the cmdline
buffer.

Additionally, does do_read_string() guarantee null termination for the
strings it reads?

If the payload lacks a null byte and do_read_string() leaves the buffer
unterminated, calling strlen(str) on the result could scan out of bounds
into adjacent heap memory until it finds a null byte. The subsequent memcpy()
would then copy this out of bounds data into the cmdline buffer, resulting
in an out of bounds write.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260410220905.200051-1-acme@kernel.org?part=2

Re: [PATCH 04/13] perf header: Sanity check HEADER_CPU_TOPOLOGY

[sashiko-bot]

Sashiko AI review found 2 potential issue(s):
- [High] Regression: Freeing `env->cpu` on error paths creates a state inconsistency leading to a guaranteed NULL pointer dereference.
- [High] Pre-existing: Heap out-of-bounds read and information leak due to missing null-termination guarantees in `do_read_string()`.
--

commit b6aa3d453035257912e7800f950abadb5a643531
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf header: Sanity check HEADER_CPU_TOPOLOGY

This commit adds validation to process_cpu_topology() to harden against
malformed perf.data files. It bounds checks sibling counts and changes bare
returns to goto statements that free the env->cpu structure.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c

[ ... ]

> @@ -2887,7 +2898,13 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
>  	env->sibling_cores = strbuf_detach(&sb, NULL);
>  
>  	if (do_read_u32(ff, &nr))
> -		return -1;
> +		goto free_cpu;
> +
> +	if (nr > (u32)cpu_nr) {
> +		pr_err("Invalid HEADER_CPU_TOPOLOGY: nr_sibling_threads (%u) > nr_cpus_avail (%d)\n",
> +		       nr, cpu_nr);
> +		goto free_cpu;
> +	}

Does freeing env->cpu on error paths here create a state inconsistency?

If perf_session__read_header() ignores the return value of
perf_header__process_sections() and proceeds, env->nr_cpus_avail will still
be greater than 0, but env->cpu will be NULL because it was freed.

Later consumers like perf_env__get_socket_aggr_by_cpu() might blindly
dereference env->cpu based on nr_cpus_avail being greater than zero:

perf_env__get_socket_aggr_by_cpu()
    env->cpu[cpu.cpu].socket_id

Before this change, the partially initialized array was left intact and
safely zero-initialized, which avoided a null pointer dereference. 

The memory leak this attempts to address appears to be safely handled already,
as perf_env__exit() properly frees env->cpu during session cleanup. Could
these new error paths just return directly to avoid the crash?

A check of subsequent commits in this series indicates this resulting state
inconsistency remains unaddressed.

[ ... ]

> @@ -2936,7 +2953,13 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
>  		return 0;
>  
>  	if (do_read_u32(ff, &nr))
> -		return -1;
> +		goto free_cpu;
> +
> +	if (nr > (u32)cpu_nr) {
> +		pr_err("Invalid HEADER_CPU_TOPOLOGY: nr_sibling_dies (%u) > nr_cpus_avail (%d)\n",
> +		       nr, cpu_nr);
> +		goto free_cpu;
> +	}

This isn't a regression from this patch, but looking at the surrounding code
that processes these sibling strings:

str = do_read_string(ff);
if (!str)
    goto error;

if (strbuf_add(&sb, str, strlen(str) + 1) < 0)

Is it possible for do_read_string() to return a string without a null
terminator? 

If a malformed file provides a string without a null byte, could strlen()
read past the end of the heap allocation until it finds one, potentially
leaking heap data when the topology information is dumped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260410220905.200051-1-acme@kernel.org?part=4

Re: [PATCH 03/13] perf header: Sanity check HEADER_NRCPUS and HEADER_CPU_DOMAIN_INFO

[sashiko-bot]

Sashiko AI review found 4 potential issue(s):
- [Medium] Mismatch in domain array traversal bounds causes a memory leak on malformed domain indices.
- [Medium] Unbounded `nr_cpus_avail` allows massive memory allocations and integer cast sign extension.
- [Medium] Integer overflow in the section size validation check bypasses bounds validation.
- [Low] Format specifier mismatch in error messages prints large values as negative numbers.
--

commit 05b748d77f989fb396557569862fea65e5d2e722
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf header: Sanity check HEADER_NRCPUS and HEADER_CPU_DOMAIN_INFO

This commit adds sanity checks to process_nrcpus and process_cpu_domain_info
to validate CPU counts and prevent duplicate domain allocations.

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
[ ... ]
> @@ -2722,6 +2724,13 @@ static int process_nrcpus(struct feat_fd *ff, void *data __maybe_unused)
>  	ret = do_read_u32(ff, &nr_cpus_online);
>  	if (ret)
>  		return ret;
> +
> +	if (nr_cpus_online > nr_cpus_avail) {
> +		pr_err("Invalid HEADER_NRCPUS: nr_cpus_online (%u) > nr_cpus_avail (%u)\n",
> +		       nr_cpus_online, nr_cpus_avail);
> +		return -1;
> +	}
> +

Since there is no absolute upper limit checked for nr_cpus_avail here, could
a malformed perf.data file provide an excessively large value?

For example, if nr_cpus_online is 1 and nr_cpus_avail is 0x40000000,
it seems this check would pass.

Later, env->nr_cpus_avail is cast to int, which would result in a negative
value if nr_cpus_avail exceeds INT_MAX. Additionally, in
process_cpu_domain_info, this could lead to a massive memory allocation
when allocating the cd_map array.

[ ... ]
> @@ -3698,6 +3707,17 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
>  	nra = env->nr_cpus_avail;
>  	nr = env->nr_cpus_online;
>  
> +	if (nra == 0 || nr == 0) {
> +		pr_err("Invalid HEADER_CPU_DOMAIN_INFO: missing HEADER_NRCPUS\n");
> +		return -1;
> +	}
> +
> +	if (ff->size < 2 * sizeof(u32) + nr * 2 * sizeof(u32)) {

Can this size validation be bypassed due to an integer overflow?

Because nr is a 32-bit unsigned integer, if a crafted file provides a very
large value like 0x80000000, nr * 2 could overflow to 0 before the promotion
to size_t. This would evaluate the requirement to just 8 bytes, completely
bypassing the bounds check.

> +		pr_err("Invalid HEADER_CPU_DOMAIN_INFO: section too small (%zu) for %u CPUs\n",
> +		       (size_t)ff->size, nr);
> +		return -1;
> +	}
> +
[ ... ]
> @@ -3714,6 +3734,18 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * Sanity check: real systems have at most ~10 sched domain levels
> +	 * (SMT, CLS, MC, PKG + NUMA hops). Reject obviously bogus values
> +	 * from malformed perf.data files before they cause excessive
> +	 * allocation in the per-CPU loop.
> +	 */
> +	if (max_sched_domains > MAX_SCHED_DOMAINS) {
> +		pr_err("Invalid HEADER_CPU_DOMAIN_INFO: max_sched_domains %u > %u\n",
> +		       max_sched_domains, MAX_SCHED_DOMAINS);
> +		return -1;
> +	}

This isn't a bug, but some of the error messages in this function (such as
the nearby checks for cpu >= nra and domain >= max_sched_domains) use the
%d format specifier for unsigned 32-bit integers. If a malformed file
provides values >= 0x80000000, would they be logged as confusing negative
numbers? Updating them to %u might improve error logging clarity.

[ ... ]
> @@ -3760,7 +3797,13 @@ static int process_cpu_domain_info(struct feat_fd *ff, void *data __maybe_unused
>  			if (!d_info)
>  				return -1;
>  
> -			assert(cd_map[cpu]->domains[domain] == NULL);
> +			if (cd_map[cpu]->domains[domain]) {
> +				pr_err("Invalid HEADER_CPU_DOMAIN_INFO: duplicate domain %u for cpu %u\n",
> +				       domain, cpu);
> +				free(d_info);
> +				return -1;
> +			}
> +
>  			cd_map[cpu]->domains[domain] = d_info;

If a malformed file specifies a domain index such that
nr_domains <= domain < max_sched_domains, does this create a memory leak?

The array is sized by max_sched_domains, so the allocation succeeds and is
stored safely here. However, if free_cpu_domain_info only loops up to
nr_domains - 1 during teardown, any structures placed at an index >=
nr_domains (including their dynamically allocated strings) might never be
accessed and permanently leaked.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260410220905.200051-1-acme@kernel.org?part=3