Re: [PATCH 08/12] perf jitdump: Fix debug_data and unwinding_data leaks

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

Pre-existing issues:
- [High] Error returns in `jit_process_dump` loop are ignored, causing subsequent records to consume stale metadata from failed parses.
- [Medium] `jit_inject` ignores the return value of `jit_process_dump` entirely, falsely reporting success on complete processing failures.
--

--- Patch [8]: [PATCH 08/12] perf jitdump: Fix debug_data and unwinding_data leaks ---
commit 57747ce97971858edaaa984c164ff79271eb0779
Author: Arnaldo Carvalho de Melo <[email protected]>

perf jitdump: Fix debug_data and unwinding_data leaks

This commit fixes memory leaks in perf jitdump by ensuring previous allocations
for debug_data and unwinding_data are freed before reassignment. It also adds
cleanup of both buffers when closing the jitdump session.

Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <[email protected]>
Cc: Stephane Eranian <[email protected]>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 4f52b143cd0da..3085091b95a51 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c

[ ... ]

> @@ -706,6 +708,7 @@ static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr)
>  
>  	memcpy(data, &jr->info.entries, sz);
>  
> +	zfree(&jd->debug_data);

[Severity: High]
This isn't a bug introduced by this patch, but does the error handling in
jit_repipe_debug_info() leave stale data active if a subsequent parse fails?

If malloc() fails or the payload size is malformed earlier in the function, it
returns -1 without clearing jd->debug_data. Does jit_process_dump() check this
error code, or does it continue and allow the next JIT_CODE_LOAD record to
consume the uncleared, stale debug data from a prior successful record?

tools/perf/util/jitdump.c:jit_process_dump() {
    ...
    case JIT_CODE_DEBUG_INFO:
        ret = jit_repipe_debug_info(jd, jr);
        break;
    ...
}

>  	jd->debug_data       = data;
>  
>  	/*

[Severity: Medium]
This is a pre-existing issue, but is it intentional that jit_inject() ignores
the return value of jit_process_dump() on complete failure?

If a fatal parsing or allocation error propagates out to ret, it looks like
the error code is logged in verbose mode but ultimately discarded from the
caller:

tools/perf/util/jitdump.c:jit_inject() {
    ...
    ret = jit_process_dump(jd);

    jit_close(jd);

    if (verbose > 0)
        fprintf(stderr, "injected: %s (%d)\n", path, ret);

    return 0;
}

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