Re: [PATCH 4/5] perf jitdump: Size code_move event allocation with idr_size

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Replacing the hardcoded +16 padding with +idr_size introduces a heap buffer overflow in jit_repipe_code_move() when PERF_SAMPLE_TID is missing but PERF_SAMPLE_TIME is present, and the filename length approaches PATH_MAX. This shares the same root cause as a pre-existing vulnerability in jit_repipe_code_load().
--

commit 631d6cebecdf5de246fc9c8ce57d27948626c5ac
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: perf jitdump: Size code_move event allocation with idr_size

This commit alters the event buffer allocation in jit_repipe_code_move()
to dynamically scale with idr_size rather than using a static padding.

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

[ ... ]

> @@ -636,9 +636,10 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
>  	idr_size = jd->machine->id_hdr_size;
>  
>  	/*
> -	 * +16 to account for sample_id_all (hack)
> +	 * Sample ID is written past the end of the mmap2 record; size
> +	 * the allocation to account for it instead of a hardcoded +16.
>  	 */
> -	event = calloc(1, sizeof(*event) + 16);
> +	event = calloc(1, sizeof(*event) + idr_size);

[Severity: Critical]
Does this regression introduce a heap buffer overflow when setting the event
time?

Looking later in jit_repipe_code_move(), we calculate the id pointer based on
header.size and idr_size:

    id = (void *)((unsigned long)event + event->mmap.header.size - idr_size);
    if (jd->sample_type & PERF_SAMPLE_TID) {
        id->pid  = pid;
        id->tid  = tid;
    }
    if (jd->sample_type & PERF_SAMPLE_TIME)
        id->time = convert_timestamp(jd, jr->load.p.timestamp);

If PERF_SAMPLE_TID is missing from the configured sample type but
PERF_SAMPLE_TIME is present, idr_size evaluates to 8, making the allocation
size become sizeof(*event) + 8.

Because id->time is located at an 8-byte offset within the id structure
(after pid and tid), writing to id->time accesses memory 8 bytes past the
start of id.

When the filename string approaches PATH_MAX and header.size nears the base
allocation size, this writes exactly past the end of the calloc heap
allocation. Could this memory corruption be triggered by parsing a
maliciously crafted perf.data file?

This is a pre-existing issue, but does this exact same pattern also cause
an out-of-bounds write in jit_repipe_code_load()?

Looking at jit_repipe_code_load(), it similarly uses the idr_size for its
allocation limit and unconditionally performs the same timestamp assignment:

tools/perf/util/jitdump.c:jit_repipe_code_load() {
    ...
    event = calloc(1, sizeof(*event) + idr_size);
    ...
    id = (void *)((unsigned long)event + event->mmap.header.size - idr_size);
    if (jd->sample_type & PERF_SAMPLE_TID) {
        id->pid  = pid;
        id->tid  = tid;
    }
    if (jd->sample_type & PERF_SAMPLE_TIME)
        id->time = convert_timestamp(jd, jr->load.p.timestamp);
    ...
}

Should both load and move operations account for the struct offset of
id->time when sizing the buffer?

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