Re: [PATCH 06/12] perf jitdump: Check snprintf return before computing header size

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Insufficient heap allocation in `jit_repipe_code_move()` leads to an out-of-bounds read and heap disclosure when writing the event to `perf.data`.
- [High] Heap out-of-bounds write due to brittle, hardcoded sample ID struct layout.
--

commit 9bf23bcb5ee8564df7189ee4fbf6479763732a6c
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Check snprintf return before computing header size

This commit clamps the string size returned by snprintf() to prevent an
integer underflow when computing header.size in jit_repipe_code_load() and
jit_repipe_code_move(). This fixes a heap corruption that occurs when jitted
filenames exceed PATH_MAX.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 7efbaa07f1ba7..45f0e21b0e780 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -495,6 +495,9 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
>  			jd->dir,
>  			nspid,
>  			count);
> +	/* snprintf returns would-be length on truncation, clamp to buffer */
> +	if (size >= sizeof(event->mmap2.filename))
> +		size = sizeof(event->mmap2.filename) - 1;

[Severity: High]
This is a pre-existing issue, but is it possible for the allocation of event
earlier in this function to cause an out-of-bounds write?

In jit_repipe_code_load(), event is allocated with exactly idr_size trailing
bytes:

    event = calloc(1, sizeof(*event) + idr_size);

Later in the function, it writes to id->time based on a hardcoded struct layout:

    struct {
        u32 pid, tid;
        u64 time;
    } *id;

    if (jd->sample_type & PERF_SAMPLE_TIME)
        id->time = convert_timestamp(jd, jr->load.p.timestamp);

If PERF_SAMPLE_TID is missing from the session but PERF_SAMPLE_TIME is
present, idr_size may only be 8. Would writing to id->time at offset 8
strictly overflow the 8-byte dynamic allocation?

>  
>  	size++; /* for \0 */
>  

[ ... ]

> @@ -625,6 +628,9 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
>  	         jd->dir,
>  		 nspid,
>  		 jr->move.code_index);
> +	/* snprintf returns would-be length on truncation, clamp to buffer */
> +	if (size >= sizeof(event->mmap2.filename))
> +		size = sizeof(event->mmap2.filename) - 1;

[Severity: High]
This is a pre-existing issue, but can this lead to an out-of-bounds read and
heap disclosure when the event is injected?

In jit_repipe_code_move(), the event buffer is allocated with a fixed 16-byte
offset for the sample ID:

    /*
     * +16 to account for sample_id_all (hack)
     */
    event = calloc(1, sizeof(*event) + 16);

However, the total event size is calculated using the full dynamic idr_size:

    event->mmap2.header.size = (sizeof(event->mmap2) -
            (sizeof(event->mmap2.filename) - size) + idr_size);

If idr_size > 16 (e.g. PERF_SAMPLE_CPU and PERF_SAMPLE_ID are set) and the
filename length approaches PATH_MAX, could header.size strictly exceed the
allocated buffer bounds? When jit_inject_event(jd, event) is called later,
could the oversized header size cause it to read past the allocation and leak
heap memory into the perf.data file?

>  
>  	size++; /* for \0 */
>

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