Re: [PATCH 12/12] perf jitdump: Validate unwinding sizes against record payload

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Stale unwinding data leak and corruption in subsequent JIT ELF files due to missing cleanup when `eh_frame_hdr_size` is zero.
- [High] Missing bounds check in `jit_repipe_code_move` leaks uninitialized heap memory into `perf.data`.
--

commit 606a51d613d656e670cf8772d98c0b537c8eb055
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Validate unwinding sizes against record payload

This commit validates unwinding sizes against the record payload to prevent
out-of-bounds heap reads when eh_frame_hdr_size exceeds unwinding_size. It
also updates mapped_size to use a uint64_t to avoid mmap2 length truncation.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 5898a7d8eb962..d25a9fe9b020c 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -462,7 +462,8 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
>  	u16 idr_size;
>  	const char *sym;
>  	uint64_t count;
> -	int ret, csize, usize;
> +	int ret, csize;
> +	uint64_t usize;
>  	pid_t nspid, pid, tid;
>  	struct {
>  		u32 pid, tid;
>  		u64 time;
>  	} *id;

[Severity: High]
This is a pre-existing issue, but does jit_repipe_code_load() leak stale
unwinding data to subsequent JIT ELF files if eh_frame_hdr_size is zero?

A JIT dump can contain an unwinding record with unwinding_size > 0 but
eh_frame_hdr_size == 0, which is valid and permitted by the new size
validation. 

When processing the subsequent code_load event, it correctly uses this
unwinding data, but appears to skip the cleanup block because
jd->eh_frame_hdr_size is zero:

	if (jd->unwinding_data && jd->eh_frame_hdr_size) {
		zfree(&jd->unwinding_data);
		jd->eh_frame_hdr_size = 0;
		...
	}

Could this cause all subsequent code_load events to erroneously embed the
same stale unwinding table, corrupting the generated ELF files for unrelated
JIT methods?

[ ... ]

> @@ -612,7 +613,7 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
>  	char *filename;
>  	size_t size;
>  	struct stat st;
> -	int usize;
> +	uint64_t usize;
>  	u16 idr_size;
>  	int ret;
>  	pid_t nspid, pid, tid;

[Severity: High]
This is a pre-existing issue, but does jit_repipe_code_move() lack bounds
checking for the record payload?

A malformed JIT dump could provide a JIT_CODE_MOVE record with a total_size
smaller than sizeof(struct jr_code_move). jit_get_next_entry() reallocates
the buffer to exactly total_size bytes, but jit_repipe_code_move() doesn't
validate total_size before unconditionally accessing fields past the fixed
record prefix:

	nspid = jr->load.pid;
	...
	event->mmap2.start = jr->move.new_code_addr;
	event->mmap2.len   = usize ? ALIGN_8(jr->move.code_size) + usize
				   : jr->move.code_size;

Could this result in uninitialized out-of-bounds heap memory being assigned
to event fields that are subsequently written to the output perf.data stream?

[ ... ]

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