Re: [PATCH 3/5] perf jitdump: Free unwinding data even when eh_frame_hdr_size is zero

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unsafe open() call in jit_emit_elf allows arbitrary file overwrite via symlink attack.
- [Medium] Stale unwinding and debug data persist on malformed JIT records due to ignored return values and missing cleanup on early returns.
--

commit 57898e6821f20c63014ed2610a5c18278ca4444d
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Free unwinding data even when eh_frame_hdr_size is zero

This commit modifies jit_repipe_code_load() to unconditionally free
the unwinding data if it is present. Previously, it only freed the data
if eh_frame_hdr_size was also non-zero, which caused unwinding sections
to be duplicated and mmap2 lengths to be inflated.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index e0d5cc9a82818..efb40d93e33ae 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -530,7 +530,7 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
>  		jd->nr_debug_entries = 0;
>  	}
>  
> -	if (jd->unwinding_data && jd->eh_frame_hdr_size) {
> +	if (jd->unwinding_data) {
>  		zfree(&jd->unwinding_data);
>  		jd->eh_frame_hdr_size = 0;
>  		jd->unwinding_mapped_size = 0;

[Severity: Medium]
This isn't a bug introduced by this patch, but earlier in
jit_repipe_code_load() we return without cleaning up state:

tools/perf/util/jitdump.c:jit_repipe_code_load() {
    ...
	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;
	}
    ...
}

If a malformed code load record triggers this early error return, the
pending unwinding and debug data aren't freed or cleared.

Is it expected that jit_process_dump() continues parsing when this happens?

tools/perf/util/jitdump.c:jit_process_dump() {
    ...
	while ((jr = jit_get_next_entry(jd))) {
		switch(jr->prefix.id) {
		case JIT_CODE_LOAD:
			ret = jit_repipe_code_load(jd, jr);
			break;
    ...
}

Because the return value is captured but not evaluated for loop termination,
can this stale unwinding and debug data be erroneously applied to subsequent
valid events?

Could an attacker supply a crafted perf.data file with malformed records
to trigger this state corruption?

[Severity: High]
This is a pre-existing issue, but does this open() call allow an arbitrary
file overwrite?

tools/perf/util/jitdump.c:jit_emit_elf() {
    ...
	nsinfo__mountns_enter(jd->nsi, &nsc);
	fd = open(filename, O_CREAT|O_TRUNC|O_WRONLY, 0644);
	saved_errno = errno;
	nsinfo__mountns_exit(&nsc);
    ...
}

Since the filename path is predictable and based on the attacker-controlled
mmap path, the missing O_EXCL and O_NOFOLLOW flags appear to leave this
vulnerable to a symlink attack.

If a local attacker creates a symlink at the predictable output path in a
shared directory pointing to a sensitive file, will a privileged user running
perf inject silently follow the symlink and overwrite the target file
with an attacker-controlled ELF payload?

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