Re: [PATCH 02/12] perf jitdump: Validate code_size against total_size in code load

On Wed, Aug 05, 2026 at 09:42:41PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
> 
> New issues:
> - [Medium] jd->debug_data and jd->unwinding_data are leaked on the new validation failure path.

This is fixed in a later patch in this series, I'll reorder the series
so that this leak doesn't gets introduced to then be fixed in another
patch.

All the pre-existing issues reported are being tracked in TODO files for
upcoming series, one is already in the making.

Thanks,

- Arnaldo
 
> Pre-existing issues:
> - [High] Integer underflow when calculating unwinding_data_size in jit_repipe_unwinding_info leads to a massive heap allocation and out-of-bounds read.
> - [High] Missing snprintf truncation clamping in jit_repipe_code_move and jit_repipe_code_load leads to an undersized event allocation calculation and out-of-bounds heap write.
> - [High] jit_get_next_entry iterates over an unvalidated nr_entry array using fixed-size indexing for a variable-length structure, leading to out-of-bounds byte-swapping.
> - [High] The parser lacks total_size boundary checks for JIT_CODE_MOVE, leading to uninitialized heap reads.
> - [High] Lack of validation between buffer size and nr_entry leads to heap OOB read and write in debug info parsing.
> - [Medium] The perf_event allocated via calloc is unconditionally leaked in jit_repipe_code_move.
> - [Medium] Multiple memory leaks of dynamically allocated buffers in the jitdump parser, including missing cleanup in jit_close and overwrites in debug info.
> - [Medium] The ALIGN_8 macro evaluates using signed arithmetic, causing integer wrap and massive length corruption for mmap2 events.
> - [Medium] Truncation of 64-bit unwinding_mapped_size to 32-bit int can corrupt mmap2 event length.
> --
> 
> --- Patch [2]: [PATCH 02/12] perf jitdump: Validate code_size against total_size in code load ---
> commit 97b091976e049b90397f99d555b22a4934abb49d
> Author: Arnaldo Carvalho de Melo <[email protected]>
> 
> perf jitdump: Validate code_size against total_size in code load
> 
> This commit introduces validation checks to ensure that the code_size
> does not exceed the available size in the JIT load record. It adds bounds
> checking to prevent out-of-bounds heap reads when calculating the code
> blob pointer.
> 
> > diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> > index 4b7c7ba7cd95d..3195f94187164 100644
> > --- a/tools/perf/util/jitdump.c
> > +++ b/tools/perf/util/jitdump.c
> > @@ -9,6 +9,7 @@
> >  #include <fcntl.h>
> >  #include <unistd.h>
> >  #include <inttypes.h>
> > +#include <limits.h>
> >  #include <byteswap.h>
> >  #include <sys/stat.h>
> >  #include <sys/mman.h>
> > @@ -450,6 +451,16 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
> >  	csize = jr->load.code_size;
> >  	usize = jd->unwinding_mapped_size;
> >  	addr  = jr->load.code_addr;
> > +
> > +	/* code blob lives at the end of the record, validate it fits */
> > +	if (jr->load.p.total_size < sizeof(jr->load) ||
> > +	    jr->load.code_size > jr->load.p.total_size - sizeof(jr->load) ||
> > +	    jr->load.code_size > INT_MAX) {
> > +		pr_warning("jitdump: invalid code_size %" PRIu64 " (total_size=%u) in code_load record\n",
> > +			   (uint64_t)jr->load.code_size, jr->load.p.total_size);
> > +		return -1;
> 
> [Severity: Medium]
> Does this early return skip freeing the buffered debug and unwinding data?
> 
> Later in the function, these dynamically allocated buffers are freed:
> 
>     if (jd->debug_data && jd->nr_debug_entries) {
>             zfree(&jd->debug_data);
>             jd->nr_debug_entries = 0;
>     }
> 
>     if (jd->unwinding_data && jd->eh_frame_hdr_size) {
>             zfree(&jd->unwinding_data);
>             ...
> 
> If the code returns -1 here, do jd->debug_data and jd->unwinding_data remain
> allocated, permanently leaking since jit_close() does not appear to clean them
> up?
> 
> > +	}
> > +
> >  	sym   = (void *)((unsigned long)jr + sizeof(jr->load));
> >  	code  = (unsigned long)jr + jr->load.p.total_size - csize;
> >  	count = jr->load.code_index;
> 
> [ ... ]
> 
> Other findings in this file:
> 
> [Severity: High]
> This is a pre-existing issue, but does integer underflow occur when calculating
> unwinding_data_size 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), does the unsigned 32-bit
> subtraction underflow to a massive value, causing malloc to allocate too much
> memory and memcpy to perform an out-of-bounds read?
> 
> 
> [Severity: Medium]
> This is a pre-existing issue, but is the perf_event leaked in
> jit_repipe_code_move()?
> 
> tools/perf/util/jitdump.c:jit_repipe_code_move() {
>     ...
>     event = calloc(1, sizeof(*event) + 16);
>     if (!event)
>         return -1;
>     ...
>     return ret;
> }
> 
> The event pointer is allocated via calloc but is never freed on the success
> or error return paths. Does this gradually exhaust memory when parsing large
> dumps?
> 
> 
> [Severity: High]
> This is a pre-existing issue, but does missing snprintf truncation clamping
> lead to out-of-bounds heap writes?
> 
> tools/perf/util/jitdump.c:jit_repipe_code_move() {
>     ...
>     size = snprintf(filename, PATH_MAX, "%s/jitted-%d-%" PRIu64 ".so", ...);
>     size++; /* for \0 */
>     event->mmap2.header.size = (sizeof(event->mmap2) - (sizeof(event->mmap2.filename) - size) + idr_size);
>     ...
> }
> 
> When generating the filename, snprintf returns the length that would have
> been written. If this exceeds PATH_MAX, does the returned size wrap in the
> unsigned arithmetic for header.size, underflowing it and pushing the trailing
> id pointer out of the allocated event buffer bounds?
> 
> 
> [Severity: Medium]
> This is a pre-existing issue, but are dynamic buffers repeatedly leaked in
> jit_repipe_debug_info() and jit_close()?
> 
> tools/perf/util/jitdump.c:jit_repipe_debug_info() {
>     ...
>     data = malloc(sz);
>     /* ... */
>     jd->debug_data       = data;
>     ...
> }
> 
> Repeated JIT_CODE_DEBUG_INFO records will malloc a new buffer and overwrite
> jd->debug_data without freeing the old one. Additionally, jit_close() frees
> the jd struct resources but does not seem to clean up internal heap buffers
> like debug_data and unwinding_data. Does this leave them un-freed
> indefinitely?
> 
> 
> [Severity: High]
> This is a pre-existing issue, but does standard array indexing on a variable
> length struct cause memory corruption during bswap?
> 
> tools/perf/util/jitdump.c:jit_get_next_entry() {
>     ...
>     for (n = 0 ; n < jr->info.nr_entry; n++) {
>         jr->info.entries[n].addr    = bswap_64(jr->info.entries[n].addr);
>     ...
> }
> 
> In cross-endian mode, this iterates over an unvalidated nr_entry array using
> fixed-size indexing. Since debug_entry includes a variable-length name[]
> string inline, doesn't fixed-size array indexing read from and byte-swap
> completely incorrect offsets?
> 
> 
> [Severity: High]
> This is a pre-existing issue, but does jit_repipe_code_move() lack boundary
> checks for the JIT_CODE_MOVE total_size?
> 
> tools/perf/util/jitdump.c:jit_repipe_code_move() {
>     ...
>     event->mmap2.start = jr->move.new_code_addr;
>     event->mmap2.len   = usize ? ALIGN_8(jr->move.code_size) + usize : jr->move.code_size;
>     ...
> }
> 
> Unlike the new validation added to JIT_CODE_LOAD in this patch,
> jit_repipe_code_move() lacks a check confirming that total_size is greater
> than or equal to sizeof(jr->move). Can malformed records cause the parser
> to read uninitialized heap or stale data from jd->buf into perf.data?
> 
> 
> [Severity: Medium]
> This is a pre-existing issue, but does the ALIGN_8 macro silently overflow
> into a negative value?
> 
> tools/perf/util/jitdump.c:jit_repipe_code_load() {
>     ...
>     int ret, csize, usize;
>     /* ... */
>     event->mmap2.len   = usize ? ALIGN_8(csize) + usize : csize;
>     ...
> }
> 
> Since csize is typed as a signed int, does calculating ALIGN_8(csize) with a
> value near INT_MAX trigger signed integer overflow to a negative value, which
> is then sign-extended into a corrupted uint64_t length?
> 
> 
> [Severity: Medium]
> This is a pre-existing issue, but can the truncation of unwinding_mapped_size
> corrupt the mmap2 event length?
> 
> tools/perf/util/jitdump.c:jit_repipe_code_move() {
>     ...
>     int usize;
>     /* ... */
>     usize = jd->unwinding_mapped_size;
>     /* ... */
>     event->mmap2.len   = usize ? ALIGN_8(jr->move.code_size) + usize : jr->move.code_size;
>     ...
> }
> 
> The 64-bit jd->unwinding_mapped_size is truncated to a 32-bit signed integer
> (usize). Could a maliciously crafted mapped_size result in a negative or
> truncated value, corrupting the final event->mmap2.len?
> 
> 
> [Severity: High]
> This is a pre-existing issue, but does the lack of validation between buffer
> size and nr_entry lead to heap out-of-bounds reads and writes?
> 
> tools/perf/util/jitdump.c:jit_repipe_debug_info() {
>     ...
>     jd->nr_debug_entries = jr->info.nr_entry;
>     ...
> }
> 
> The parser sets jd->nr_debug_entries from the unvalidated nr_entry file field
> without confirming the allocated sz bytes can accommodate them. When
> jit_process_debug_info() loops nr_entry times calling strlen on heap data,
> does this result in out-of-bounds heap writes?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2