* [PATCHES v1 0/4] perf tools: Fix OOB reads, NULL deref, and resource leaks
@ 2026-06-04 20:11 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
` (3 more replies)
0 siblings, 4 replies; 10+ 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
Hi,
Four pre-existing bugs found by sashiko-bot during AI-assisted review
of the perf-data-validation hardening series. All are independent of
that series -- they are latent bugs in surrounding code exposed during
review.
1. test_bit(sample->cpu, cpu_bitmap) reads out of bounds when
PERF_SAMPLE_CPU is absent (sample->cpu == (u32)-1) in annotate,
diff, report, and sched timehist.
2. Thread reference leak in perf sched latency_switch_event() -- one
error path does 'return -1' instead of 'goto out_put'.
3. NULL pointer dereference in perf sched latency_runtime_event() --
thread_atoms_search() called before the NULL check on the thread
returned by machine__findnew_thread().
4. Heap buffer overflow in perf sched map --compact mode --
comp_cpus allocated based on host CPU count but indexed using a
MAX_CPUS-sized bitmap, overflowing when analyzing recordings from
machines with more CPUs than the host. Also removes the now-dead
sysconf(_SC_NPROCESSORS_CONF) initialization whose sole consumer
was the replaced allocation, since max_cpu is dynamically updated
from the recording's events during processing.
All four require crafted or unusual perf.data inputs to trigger.
Verified with gcc and clang builds, checkpatch, and perf test.
Arnaldo Carvalho de Melo (4):
perf tools: Guard test_bit from out-of-bounds sample CPU
perf sched: Fix thread reference leak in latency_switch_event
perf sched: Fix NULL dereference in latency_runtime_event
perf sched: Fix comp_cpus heap overflow with cross-machine recordings
tools/perf/builtin-annotate.c | 3 ++-
tools/perf/builtin-diff.c | 3 ++-
tools/perf/builtin-report.c | 3 ++-
tools/perf/builtin-sched.c | 16 +++++++++-------
4 files changed, 15 insertions(+), 10 deletions(-)
Developed with AI assistance (Claude/sashiko), tagged in commits.
Thanks a lot,
- Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread
* [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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread
* [PATCH 2/4] perf sched: Fix thread reference leak in latency_switch_event
2026-06-04 20:49 [PATCHES v2 0/4] perf tools: Fix OOB reads, NULL deref, and resource leaks Arnaldo Carvalho de Melo
@ 2026-06-04 20:49 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-04 20:49 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] 10+ messages in thread
end of thread, other threads:[~2026-06-04 20:49 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` [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
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
-- strict thread matches above, loose matches on Subject: below --
2026-06-04 20:49 [PATCHES v2 0/4] perf tools: Fix OOB reads, NULL deref, and resource leaks Arnaldo Carvalho de Melo
2026-06-04 20:49 ` [PATCH 2/4] perf sched: Fix thread reference leak in latency_switch_event Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox