Re: [PATCH 01/12] perf jitdump: Fix extended header read that always fails

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds checks for JIT records allow out-of-bounds memory accesses.
- [High] The error path for a failed malloc() calls funlockfile() on an un-locked FILE pointer.
--

--- Patch [1]: [PATCH 01/12] perf jitdump: Fix extended header read that always fails ---
commit dc8d1e17077d95aa9e9434ca752e0c0e9bb0e0de
Author: Arnaldo Carvalho de Melo <[email protected]>

perf jitdump: Fix extended header read that always fails

This commit fixes an issue in jit_open() where extended header bytes were
not being properly read. It separates the buffer reallocation from the
stream read, ensuring the extended header bytes are correctly consumed.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 83005b30b9bf3..4b7c7ba7cd95d 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential missing bounds check
in this file while reviewing.

In jit_repipe_code_load(), csize and sym are extracted without bounds checks
against total_size:

tools/perf/util/jitdump.c:jit_repipe_code_load() {
    ...
    csize = jr->load.code_size;
    ...
    sym   = (void *)((unsigned long)jr + sizeof(jr->load));
    code  = (unsigned long)jr + jr->load.p.total_size - csize;
    ...
}

Could this allow code to point out-of-bounds if total_size is not validated
against the record size?

There seems to be a similar issue in jit_repipe_unwinding_info():

tools/perf/util/jitdump.c:jit_repipe_unwinding_info() {
    ...
    unwinding_data_size  = jr->prefix.total_size - sizeof(jr->unwinding);
    unwinding_data = malloc(unwinding_data_size);
    ...
    memcpy(unwinding_data, &jr->unwinding.unwinding_data,
           unwinding_data_size);
    ...
}

If total_size is smaller than sizeof(jr->unwinding), could this underflow
unwinding_data_size and lead to a large allocation and an out-of-bounds
memcpy?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1