* [PATCH 1/4] perf tools: Guard test_bit from out-of-bounds sample CPU
2026-06-04 20:11 [PATCHES v1 0/4] perf tools: Fix OOB reads, NULL deref, and resource leaks Arnaldo Carvalho de Melo
@ 2026-06-04 20:11 ` Arnaldo Carvalho de Melo
2026-06-04 20:11 ` [PATCH 2/4] perf sched: Fix thread reference leak in latency_switch_event Arnaldo Carvalho de Melo
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-04 20:11 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, Anton Blanchard, sashiko-bot,
Claude Opus 4.6
From: Arnaldo Carvalho de Melo <acme@redhat.com>
When PERF_SAMPLE_CPU is absent from a perf.data file, sample->cpu is
initialized to (u32)-1 by evsel__parse_sample(). Five call sites pass
this value directly to test_bit(sample->cpu, cpu_bitmap), reading
massively out of bounds past the DECLARE_BITMAP(..., MAX_NR_CPUS)
allocation of 4096 bits.
Add a sample->cpu >= MAX_NR_CPUS guard before each test_bit() call,
matching the existing safe pattern in builtin-kwork.c. This catches
both the (u32)-1 sentinel and any corrupted CPU value exceeding the
bitmap size.
Fixes: 5d67be97f890 ("perf report/annotate/script: Add option to specify a CPU range")
Cc: Anton Blanchard <anton@samba.org>
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-annotate.c | 3 ++-
tools/perf/builtin-diff.c | 3 ++-
tools/perf/builtin-report.c | 3 ++-
tools/perf/builtin-sched.c | 6 ++++--
4 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index b918f9eed5fd2441..8a0eb30eac24fdbc 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -295,7 +295,8 @@ static int process_sample_event(const struct perf_tool *tool,
goto out_put;
}
- if (ann->cpu_list && !test_bit(sample->cpu, ann->cpu_bitmap))
+ if (ann->cpu_list && (sample->cpu >= MAX_NR_CPUS ||
+ !test_bit(sample->cpu, ann->cpu_bitmap)))
goto out_put;
if (!al.filtered &&
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 9592f44b6545bab6..9fa8e900637b0d71 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -416,7 +416,8 @@ static int diff__process_sample_event(const struct perf_tool *tool,
goto out;
}
- if (cpu_list && !test_bit(sample->cpu, cpu_bitmap)) {
+ if (cpu_list && (sample->cpu >= MAX_NR_CPUS ||
+ !test_bit(sample->cpu, cpu_bitmap))) {
ret = 0;
goto out;
}
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 6f044c3df8937dc5..dd1309c320943ea4 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -298,7 +298,8 @@ static int process_sample_event(const struct perf_tool *tool,
if (symbol_conf.hide_unresolved && al.sym == NULL)
goto out_put;
- if (rep->cpu_list && !test_bit(sample->cpu, rep->cpu_bitmap))
+ if (rep->cpu_list && (sample->cpu >= MAX_NR_CPUS ||
+ !test_bit(sample->cpu, rep->cpu_bitmap)))
goto out_put;
if (sort__mode == SORT_MODE__BRANCH) {
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 4de2baf03c5036dc..e7bd3f331cb8e889 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2192,7 +2192,8 @@ static void timehist_print_sample(struct perf_sched *sched,
char nstr[30];
u64 wait_time;
- if (cpu_list && !test_bit(sample->cpu, cpu_bitmap))
+ if (cpu_list && (sample->cpu >= MAX_NR_CPUS ||
+ !test_bit(sample->cpu, cpu_bitmap)))
return;
timestamp__scnprintf_usec(t, tstr, sizeof(tstr));
@@ -2871,7 +2872,8 @@ static int timehist_sched_change_event(const struct perf_tool *tool,
}
if (!sched->idle_hist || thread__tid(thread) == 0) {
- if (!cpu_list || test_bit(sample->cpu, cpu_bitmap))
+ if (!cpu_list || (sample->cpu < MAX_NR_CPUS &&
+ test_bit(sample->cpu, cpu_bitmap)))
timehist_update_runtime_stats(tr, t, tprev);
if (sched->idle_hist) {
--
2.54.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/4] perf sched: Fix thread reference leak in latency_switch_event
2026-06-04 20:11 [PATCHES v1 0/4] perf tools: Fix OOB reads, NULL deref, and resource leaks Arnaldo Carvalho de Melo
2026-06-04 20:11 ` [PATCH 1/4] perf tools: Guard test_bit from out-of-bounds sample CPU Arnaldo Carvalho de Melo
@ 2026-06-04 20:11 ` Arnaldo Carvalho de Melo
2026-06-04 20:11 ` [PATCH 3/4] perf sched: Fix NULL dereference in latency_runtime_event Arnaldo Carvalho de Melo
2026-06-04 20:11 ` [PATCH 4/4] perf sched: Fix comp_cpus heap overflow with cross-machine recordings Arnaldo Carvalho de Melo
3 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-04 20:11 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, Claude Opus 4.6
From: Arnaldo Carvalho de Melo <acme@redhat.com>
In latency_switch_event(), after acquiring thread references for
sched_out and sched_in via machine__findnew_thread(), the first
add_sched_out_event() failure path does 'return -1', bypassing the
out_put label that calls thread__put() on both references.
The second and third add_sched_out_event() failures correctly use
'goto out_put'. Fix the first one to match.
Fixes: b91fc39f4ad7 ("perf machine: Protect the machine->threads with a rwlock")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-sched.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index e7bd3f331cb8e889..13b801496a01271e 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1180,7 +1180,7 @@ static int latency_switch_event(struct perf_sched *sched,
}
}
if (add_sched_out_event(out_events, prev_state, timestamp))
- return -1;
+ goto out_put;
in_events = thread_atoms_search(&sched->atom_root, sched_in, &sched->cmp_pid);
if (!in_events) {
--
2.54.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/4] perf sched: Fix NULL dereference in latency_runtime_event
2026-06-04 20:11 [PATCHES v1 0/4] perf tools: Fix OOB reads, NULL deref, and resource leaks Arnaldo Carvalho de Melo
2026-06-04 20:11 ` [PATCH 1/4] perf tools: Guard test_bit from out-of-bounds sample CPU Arnaldo Carvalho de Melo
2026-06-04 20:11 ` [PATCH 2/4] perf sched: Fix thread reference leak in latency_switch_event Arnaldo Carvalho de Melo
@ 2026-06-04 20:11 ` Arnaldo Carvalho de Melo
2026-06-04 20:30 ` sashiko-bot
2026-06-04 20:11 ` [PATCH 4/4] perf sched: Fix comp_cpus heap overflow with cross-machine recordings Arnaldo Carvalho de Melo
3 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-04 20:11 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, Claude Opus 4.6
From: Arnaldo Carvalho de Melo <acme@redhat.com>
latency_runtime_event() passes the return value of
machine__findnew_thread() directly to thread_atoms_search() at line
1216, before checking for NULL at line 1220. thread_atoms_search()
calls pid_cmp() which dereferences the thread pointer via
thread__tid(), causing a NULL pointer dereference if the allocation
fails.
All other callers of thread_atoms_search() in this file
(latency_switch_event, latency_wakeup_event,
latency_migrate_task_event) correctly check for NULL first.
Move the atoms assignment after the NULL check to match the pattern
used by the other callers.
Fixes: b91fc39f4ad7 ("perf machine: Protect the machine->threads with a rwlock")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-sched.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 13b801496a01271e..36da451447b5e59f 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1213,13 +1213,15 @@ static int latency_runtime_event(struct perf_sched *sched,
const u32 pid = perf_sample__intval(sample, "pid");
const u64 runtime = perf_sample__intval(sample, "runtime");
struct thread *thread = machine__findnew_thread(machine, -1, pid);
- struct work_atoms *atoms = thread_atoms_search(&sched->atom_root, thread, &sched->cmp_pid);
+ struct work_atoms *atoms;
u64 timestamp = sample->time;
int cpu = sample->cpu, err = -1;
if (thread == NULL)
return -1;
+ atoms = thread_atoms_search(&sched->atom_root, thread, &sched->cmp_pid);
+
/* perf.data is untrusted input — CPU may be absent or corrupted */
if (cpu >= MAX_CPUS || cpu < 0) {
pr_warning("WARNING: at offset %#" PRIx64 ": out-of-bound sample CPU %d, skipping sample\n",
--
2.54.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] perf sched: Fix NULL dereference in latency_runtime_event
2026-06-04 20:11 ` [PATCH 3/4] perf sched: Fix NULL dereference in latency_runtime_event Arnaldo Carvalho de Melo
@ 2026-06-04 20:30 ` sashiko-bot
2026-06-04 20:41 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 9+ messages in thread
From: sashiko-bot @ 2026-06-04 20:30 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Using strcpy() to copy untrusted string payloads into a fixed-size buffer leads to a heap buffer overflow.
- [Critical] Unbounded attacker-controlled PID values are used to calculate memory allocation sizes, leading to integer overflows and OOB writes.
--
commit ab6f421859203bf939f45ab57b4ced38c5193ef5
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
perf sched: Fix NULL dereference in latency_runtime_event
This commit fixes a NULL pointer dereference in latency_runtime_event().
It moves the assignment to atoms using thread_atoms_search() after the
thread NULL check to match patterns used elsewhere in the file.
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 13b801496a012..36da451447b5e 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -1213,13 +1213,15 @@ static int latency_runtime_event(struct perf_sched *sched,
> const u32 pid = perf_sample__intval(sample, "pid");
> const u64 runtime = perf_sample__intval(sample, "runtime");
> struct thread *thread = machine__findnew_thread(machine, -1, pid);
> - struct work_atoms *atoms = thread_atoms_search(&sched->atom_root, thread, &sched->cmp_pid);
> + struct work_atoms *atoms;
> u64 timestamp = sample->time;
> int cpu = sample->cpu, err = -1;
>
> if (thread == NULL)
> return -1;
>
> + atoms = thread_atoms_search(&sched->atom_root, thread, &sched->cmp_pid);
> +
> /* perf.data is untrusted input — CPU may be absent or corrupted */
> if (cpu >= MAX_CPUS || cpu < 0) {
> pr_warning("WARNING: at offset %#" PRIx64 ": out-of-bound sample CPU %d, skipping sample\n",
[ ... ]
While reviewing this file, I noticed a couple of pre-existing issues in
register_pid():
tools/perf/builtin-sched.c:register_pid() {
...
if (pid >= (unsigned long)pid_max) {
BUG_ON((sched->pid_to_task = realloc(sched->pid_to_task, (pid + 1) *
sizeof(struct task_desc *))) == NULL);
while (pid >= (unsigned long)pid_max)
sched->pid_to_task[pid_max++] = NULL;
}
...
[Severity: Critical]
This isn't a bug introduced by this patch, but could an attacker-controlled
pid from an untrusted perf.data file overflow the size calculation here?
If pid is set to 0xFFFFFFFF, the pid + 1 calculation wraps to 0 on 32-bit
systems, resulting in a 0-byte allocation.
The subsequent while loop would then write NULL pointers sequentially out
of bounds, which could corrupt heap memory.
Also in the same function:
tools/perf/builtin-sched.c:register_pid() {
...
task = zalloc(sizeof(*task));
task->pid = pid;
task->nr = sched->nr_tasks;
strcpy(task->comm, comm);
...
[Severity: Critical]
This is a pre-existing issue, but does this strcpy() allow a heap buffer
overflow?
The comm string extracted from the perf.data payload is copied into the
fixed 20-byte task->comm array.
If the untrusted string is longer than 19 characters, this could overflow
the struct task_desc heap allocation and overwrite adjacent fields.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260604201119.1702338-1-acme@kernel.org?part=3
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] perf sched: Fix NULL dereference in latency_runtime_event
2026-06-04 20:30 ` sashiko-bot
@ 2026-06-04 20:41 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-04 20:41 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-perf-users
On Thu, Jun 04, 2026 at 08:30:10PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Using strcpy() to copy untrusted string payloads into a fixed-size buffer leads to a heap buffer overflow.
> - [Critical] Unbounded attacker-controlled PID values are used to calculate memory allocation sizes, leading to integer overflows and OOB writes.
> --
Both pre-existing, added to TODO files for future series.
- Arnaldo
> commit ab6f421859203bf939f45ab57b4ced38c5193ef5
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> perf sched: Fix NULL dereference in latency_runtime_event
>
> This commit fixes a NULL pointer dereference in latency_runtime_event().
> It moves the assignment to atoms using thread_atoms_search() after the
> thread NULL check to match patterns used elsewhere in the file.
>
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index 13b801496a012..36da451447b5e 100644
> > --- a/tools/perf/builtin-sched.c
> > +++ b/tools/perf/builtin-sched.c
> > @@ -1213,13 +1213,15 @@ static int latency_runtime_event(struct perf_sched *sched,
> > const u32 pid = perf_sample__intval(sample, "pid");
> > const u64 runtime = perf_sample__intval(sample, "runtime");
> > struct thread *thread = machine__findnew_thread(machine, -1, pid);
> > - struct work_atoms *atoms = thread_atoms_search(&sched->atom_root, thread, &sched->cmp_pid);
> > + struct work_atoms *atoms;
> > u64 timestamp = sample->time;
> > int cpu = sample->cpu, err = -1;
> >
> > if (thread == NULL)
> > return -1;
> >
> > + atoms = thread_atoms_search(&sched->atom_root, thread, &sched->cmp_pid);
> > +
> > /* perf.data is untrusted input — CPU may be absent or corrupted */
> > if (cpu >= MAX_CPUS || cpu < 0) {
> > pr_warning("WARNING: at offset %#" PRIx64 ": out-of-bound sample CPU %d, skipping sample\n",
>
> [ ... ]
>
> While reviewing this file, I noticed a couple of pre-existing issues in
> register_pid():
>
> tools/perf/builtin-sched.c:register_pid() {
> ...
> if (pid >= (unsigned long)pid_max) {
> BUG_ON((sched->pid_to_task = realloc(sched->pid_to_task, (pid + 1) *
> sizeof(struct task_desc *))) == NULL);
> while (pid >= (unsigned long)pid_max)
> sched->pid_to_task[pid_max++] = NULL;
> }
> ...
>
> [Severity: Critical]
> This isn't a bug introduced by this patch, but could an attacker-controlled
> pid from an untrusted perf.data file overflow the size calculation here?
>
> If pid is set to 0xFFFFFFFF, the pid + 1 calculation wraps to 0 on 32-bit
> systems, resulting in a 0-byte allocation.
>
> The subsequent while loop would then write NULL pointers sequentially out
> of bounds, which could corrupt heap memory.
>
>
> Also in the same function:
>
> tools/perf/builtin-sched.c:register_pid() {
> ...
> task = zalloc(sizeof(*task));
> task->pid = pid;
> task->nr = sched->nr_tasks;
> strcpy(task->comm, comm);
> ...
>
> [Severity: Critical]
> This is a pre-existing issue, but does this strcpy() allow a heap buffer
> overflow?
>
> The comm string extracted from the perf.data payload is copied into the
> fixed 20-byte task->comm array.
>
> If the untrusted string is longer than 19 characters, this could overflow
> the struct task_desc heap allocation and overwrite adjacent fields.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260604201119.1702338-1-acme@kernel.org?part=3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] perf sched: Fix comp_cpus heap overflow with cross-machine recordings
2026-06-04 20:11 [PATCHES v1 0/4] perf tools: Fix OOB reads, NULL deref, and resource leaks Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2026-06-04 20:11 ` [PATCH 3/4] perf sched: Fix NULL dereference in latency_runtime_event Arnaldo Carvalho de Melo
@ 2026-06-04 20:11 ` Arnaldo Carvalho de Melo
2026-06-04 20:26 ` sashiko-bot
3 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-04 20:11 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, Claude Opus 4.6
From: Arnaldo Carvalho de Melo <acme@redhat.com>
setup_map_cpus() allocates comp_cpus based on
sysconf(_SC_NPROCESSORS_CONF), the host machine's CPU count. But
map_switch_event() indexes comp_cpus using cpus_nr derived from
bitmap_weight(comp_cpus_mask, MAX_CPUS), where comp_cpus_mask is
declared as DECLARE_BITMAP(..., MAX_CPUS) with MAX_CPUS=4096.
When analyzing a perf.data recording from a machine with more CPUs
than the analysis host (e.g. 128-CPU server recording analyzed on an
8-CPU laptop), cpus_nr exceeds the allocation size, causing a heap
buffer overflow.
Also fix a type mismatch: comp_cpus is 'struct perf_cpu *' (2 bytes
per element) but was allocated with sizeof(int) (4 bytes per element).
Allocate comp_cpus with MAX_CPUS entries using the correct element
size, matching the comp_cpus_mask bitmap bounds. Remove the
sysconf(_SC_NPROCESSORS_CONF) initialization of max_cpu — it was only
used for the comp_cpus allocation, and max_cpu is dynamically updated
from the recording's events during processing.
Fixes: 99623c628f54 ("perf sched: Add compact display option")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-sched.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 36da451447b5e59f..80e42536400a91c2 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -3573,10 +3573,8 @@ static int perf_sched__lat(struct perf_sched *sched)
static int setup_map_cpus(struct perf_sched *sched)
{
- sched->max_cpu.cpu = sysconf(_SC_NPROCESSORS_CONF);
-
if (sched->map.comp) {
- sched->map.comp_cpus = calloc(sched->max_cpu.cpu, sizeof(int));
+ sched->map.comp_cpus = calloc(MAX_CPUS, sizeof(*sched->map.comp_cpus));
if (!sched->map.comp_cpus)
return -1;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] perf sched: Fix comp_cpus heap overflow with cross-machine recordings
2026-06-04 20:11 ` [PATCH 4/4] perf sched: Fix comp_cpus heap overflow with cross-machine recordings Arnaldo Carvalho de Melo
@ 2026-06-04 20:26 ` sashiko-bot
2026-06-04 20:42 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 9+ messages in thread
From: sashiko-bot @ 2026-06-04 20:26 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removing the `sched->max_cpu.cpu` initialization introduces a severe off-by-one error, causing `perf sched map` to truncate the highest CPU from its output and print nothing for single-CPU traces.
--
commit 4c1487bde6fa91cfb4bca7de01bfd611065c7c9a
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
perf sched: Fix comp_cpus heap overflow with cross-machine recordings
This commit fixes a heap buffer overflow by allocating comp_cpus with
MAX_CPUS and the correct element size. It also removes the initial assignment
of max_cpu from the host's CPU count.
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 36da451447b5e..80e42536400a9 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -3573,10 +3573,8 @@ static int perf_sched__lat(struct perf_sched *sched)
>
> static int setup_map_cpus(struct perf_sched *sched)
> {
> - sched->max_cpu.cpu = sysconf(_SC_NPROCESSORS_CONF);
> -
[Severity: Medium]
Does removing this initialization cause an off-by-one error when rendering
the schedule map?
Without this initialization, sched->max_cpu.cpu tracks the maximum 0-based
CPU index seen in the trace. However, when compact mode is disabled,
map_switch_event() assigns this 0-based index directly to cpus_nr:
tools/perf/builtin-sched.c:map_switch_event() {
...
} else
cpus_nr = sched->max_cpu.cpu;
...
}
Then print_sched_map() iterates strictly less than cpus_nr:
tools/perf/builtin-sched.c:print_sched_map() {
...
for (int i = 0; i < cpus_nr; i++) {
...
}
Will this skip the highest CPU column entirely, and print nothing for
single-CPU traces where cpus_nr becomes 0?
> if (sched->map.comp) {
> - sched->map.comp_cpus = calloc(sched->max_cpu.cpu, sizeof(int));
> + sched->map.comp_cpus = calloc(MAX_CPUS, sizeof(*sched->map.comp_cpus));
> if (!sched->map.comp_cpus)
> return -1;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260604201119.1702338-1-acme@kernel.org?part=4
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] perf sched: Fix comp_cpus heap overflow with cross-machine recordings
2026-06-04 20:26 ` sashiko-bot
@ 2026-06-04 20:42 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-04 20:42 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-perf-users
On Thu, Jun 04, 2026 at 08:26:05PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Removing the `sched->max_cpu.cpu` initialization introduces a severe off-by-one error, causing `perf sched map` to truncate the highest CPU from its output and print nothing for single-CPU traces.
> --
>
> commit 4c1487bde6fa91cfb4bca7de01bfd611065c7c9a
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> perf sched: Fix comp_cpus heap overflow with cross-machine recordings
>
> This commit fixes a heap buffer overflow by allocating comp_cpus with
> MAX_CPUS and the correct element size. It also removes the initial assignment
> of max_cpu from the host's CPU count.
>
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index 36da451447b5e..80e42536400a9 100644
> > --- a/tools/perf/builtin-sched.c
> > +++ b/tools/perf/builtin-sched.c
> > @@ -3573,10 +3573,8 @@ static int perf_sched__lat(struct perf_sched *sched)
> >
> > static int setup_map_cpus(struct perf_sched *sched)
> > {
> > - sched->max_cpu.cpu = sysconf(_SC_NPROCESSORS_CONF);
> > -
>
> [Severity: Medium]
> Does removing this initialization cause an off-by-one error when rendering
> the schedule map?
>
> Without this initialization, sched->max_cpu.cpu tracks the maximum 0-based
> CPU index seen in the trace. However, when compact mode is disabled,
> map_switch_event() assigns this 0-based index directly to cpus_nr:
>
> tools/perf/builtin-sched.c:map_switch_event() {
> ...
> } else
> cpus_nr = sched->max_cpu.cpu;
> ...
> }
>
> Then print_sched_map() iterates strictly less than cpus_nr:
>
> tools/perf/builtin-sched.c:print_sched_map() {
> ...
> for (int i = 0; i < cpus_nr; i++) {
> ...
> }
>
> Will this skip the highest CPU column entirely, and print nothing for
> single-CPU traces where cpus_nr becomes 0?
Legit, v2 will have a fix for this, the sysconf() was just masking a
pre-existing problem.
- Arnaldo
> > if (sched->map.comp) {
> > - sched->map.comp_cpus = calloc(sched->max_cpu.cpu, sizeof(int));
> > + sched->map.comp_cpus = calloc(MAX_CPUS, sizeof(*sched->map.comp_cpus));
> > if (!sched->map.comp_cpus)
> > return -1;
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260604201119.1702338-1-acme@kernel.org?part=4
^ permalink raw reply [flat|nested] 9+ messages in thread