All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Arnaldo Carvalho de Melo" <acme@kernel.org>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 03/12] perf jitdump: Prevent integer underflow in debug info size calculation
Date: Wed, 05 Aug 2026 14:56:56 +0000	[thread overview]
Message-ID: <20260805145657.9EC201F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260805133013.235016-4-acme@kernel.org>

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

Pre-existing issues:
- [High] Missing validation of unwinding_size and eh_frame_hdr_size allows arbitrary heap memory extraction via ELF generation.
- [High] Broken byte-swapping logic for JIT_CODE_DEBUG_INFO records corrupts data and writes out of bounds, bypassing length validation.
- [Low] Lingering memory leak of `jd->debug_data` and `jd->unwinding_data` upon processing completion.
- [Low] Conditional cleanup in `jit_repipe_code_load()` leaves buffers un-freed on edge cases.
--

--- Patch [3]: [PATCH 03/12] perf jitdump: Prevent integer underflow in debug info size calculation ---
commit 4aaa391def5b9dab180e25ac7cc8112352a1ca2a
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

    perf jitdump: Prevent integer underflow in debug info size calculation
    
    jit_repipe_debug_info() and jit_repipe_unwinding_info() compute payload
    sizes by subtracting the fixed header size from total_size:
    
      sz = jr->prefix.total_size - sizeof(jr->info);
    
    When total_size is smaller than the header struct (from a truncated or
    corrupted jitdump record), the subtraction underflows to a massive
    value, causing an oversized allocation followed by an OOB memcpy.
    
    Validate that total_size covers at least the fixed header before the
    subtraction in both functions.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 3195f94187164..787f8a03dae87 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -696,6 +700,10 @@ jit_repipe_unwinding_info(struct jit_buf_desc *jd, union jr_entry *jr)
>  	if (!(jd && jr))
>  		return -1;
>  
> +	/* total_size must cover at least the fixed header */
> +	if (jr->prefix.total_size < sizeof(jr->unwinding))
> +		return -1;
> +
>  	unwinding_data_size  = jr->prefix.total_size - sizeof(jr->unwinding);
>  	unwinding_data = malloc(unwinding_data_size);
>  	if (!unwinding_data)

[Severity: High]
This is a pre-existing issue, but is it possible for a maliciously crafted
JIT dump file to provide massive unwinding_size and eh_frame_hdr_size values?

In jit_repipe_unwinding_info(), untrusted sizes are directly assigned to the
descriptor without validating that they fit within the actual allocated
payload:

tools/perf/util/jitdump.c:jit_repipe_unwinding_info() {
    ...
	jd->eh_frame_hdr_size = jr->unwinding.eh_frame_hdr_size;
	jd->unwinding_size = jr->unwinding.unwinding_size;
    ...
}

Later, in jit_add_eh_frame_info(), these unvalidated values could cause an
underflow or massive size calculation:

tools/perf/util/genelf.c:jit_add_eh_frame_info() {
    ...
	uint64_t unwinding_table_size = unwinding_size - unwinding_header_size;
    ...
	d->d_size = unwinding_table_size;
	d->d_buf = unwinding;
    ...
}

Could this force libelf to read massive amounts of out-of-bounds heap memory
and embed it into the output ELF file?

[Severity: High]
This isn't a bug introduced by this patch, but does the byte-swapping logic
for JIT_CODE_DEBUG_INFO records risk an out-of-bounds access before length
validation occurs?

In jit_get_next_entry(), a cross-endian file with a maliciously large nr_entry
could cause issues:

tools/perf/util/jitdump.c:jit_get_next_entry() {
    ...
			for (n = 0 ; n < jr->info.nr_entry; n++) {
				jr->info.entries[n].addr    = bswap_64(jr->info.entries[n].addr);
				jr->info.entries[n].lineno  = bswap_32(jr->info.entries[n].lineno);
				jr->info.entries[n].discrim = bswap_32(jr->info.entries[n].discrim);
			}
    ...
}

Since struct debug_entry contains a flexible array member (const char name[]),
the entries are variable-length.

Does using fixed-size array indexing (jr->info.entries[n]) read and byte-swap
memory beyond the allocated jd->buf boundaries?

[Severity: Low]
This is a pre-existing issue, but is there a conditional cleanup leak in
jit_repipe_code_load()?

tools/perf/util/jitdump.c:jit_repipe_code_load() {
    ...
	if (jd->debug_data && jd->nr_debug_entries) {
		zfree(&jd->debug_data);
		jd->nr_debug_entries = 0;
	}
    ...
}

If a JIT_CODE_DEBUG_INFO record allocates memory because total_size >
sizeof(header), but has 0 entries (jd->nr_debug_entries == 0), does
jd->debug_data remain allocated but fail the secondary check, causing a leak?

[Severity: Low]
This is a pre-existing issue, but is there a memory leak of jd->debug_data
and jd->unwinding_data upon processing completion?

In jit_process(), when the function tears down, it frees jd.buf but does not
appear to clean up jd->debug_data and jd->unwinding_data:

tools/perf/util/jitdump.c:jit_process() {
    ...
	nsinfo__put(jd.nsi);
	free(jd.buf);

	return ret;
}

Additionally, in jit_repipe_debug_info(), does the assignment to
jd->debug_data overwrite the previous pointer without properly freeing
it first?

tools/perf/util/jitdump.c:jit_repipe_debug_info() {
    ...
	jd->debug_data       = data;
    ...
}

Could these issues lead to lingering memory leaks on truncated or
malformed JIT dump files?

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

  reply	other threads:[~2026-08-05 14:56 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05 13:29 [PATCHES v1 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
2026-08-05 13:30 ` [PATCH 01/12] perf jitdump: Fix extended header read that always fails Arnaldo Carvalho de Melo
2026-08-05 13:46   ` sashiko-bot
2026-08-05 18:51   ` Ian Rogers
2026-08-05 13:30 ` [PATCH 02/12] perf jitdump: Validate code_size against total_size in code load Arnaldo Carvalho de Melo
2026-08-05 13:59   ` sashiko-bot
2026-08-05 18:58   ` Ian Rogers
2026-08-05 13:30 ` [PATCH 03/12] perf jitdump: Prevent integer underflow in debug info size calculation Arnaldo Carvalho de Melo
2026-08-05 14:56   ` sashiko-bot [this message]
2026-08-05 18:59   ` Ian Rogers
2026-08-05 13:30 ` [PATCH 04/12] perf jitdump: Bounds-check debug entry byte-swap loop Arnaldo Carvalho de Melo
2026-08-05 14:27   ` sashiko-bot
2026-08-05 19:00   ` Ian Rogers
2026-08-05 13:30 ` [PATCH 05/12] perf jitdump: Check snprintf return before computing header size Arnaldo Carvalho de Melo
2026-08-05 14:25   ` sashiko-bot
2026-08-05 19:07   ` Ian Rogers
2026-08-05 19:45     ` Arnaldo Carvalho de Melo
2026-08-05 21:29       ` Ian Rogers
2026-08-07 12:00         ` Arnaldo Carvalho de Melo
2026-08-05 13:30 ` [PATCH 06/12] perf jitdump: Fix funlockfile on unlocked stream in jit_open() error path Arnaldo Carvalho de Melo
2026-08-05 15:48   ` sashiko-bot
2026-08-05 19:09   ` Ian Rogers
2026-08-05 13:30 ` [PATCH 07/12] perf jitdump: Free event in jit_repipe_code_move() Arnaldo Carvalho de Melo
2026-08-05 14:33   ` sashiko-bot
2026-08-05 19:09   ` Ian Rogers
2026-08-05 13:30 ` [PATCH 08/12] perf jitdump: Fix debug_data and unwinding_data leaks Arnaldo Carvalho de Melo
2026-08-05 14:35   ` sashiko-bot
2026-08-05 19:10   ` Ian Rogers
2026-08-05 13:30 ` [PATCH 09/12] perf jitdump: Use dirname() return value in jit_open() Arnaldo Carvalho de Melo
2026-08-05 14:13   ` sashiko-bot
2026-08-05 19:16   ` Ian Rogers
2026-08-05 13:30 ` [PATCH 10/12] perf jitdump: Validate debug entries on native (non-swap) path Arnaldo Carvalho de Melo
2026-08-05 14:26   ` sashiko-bot
2026-08-05 19:19   ` Ian Rogers
2026-08-05 13:30 ` [PATCH 11/12] perf jitdump: Validate sym string NUL-termination in code load Arnaldo Carvalho de Melo
2026-08-05 19:19   ` Ian Rogers
2026-08-05 13:30 ` [PATCH 12/12] perf jitdump: Validate unwinding sizes against record payload Arnaldo Carvalho de Melo
2026-08-05 19:20   ` Ian Rogers
  -- strict thread matches above, loose matches on Subject: below --
2026-08-05 21:26 [PATCHES v2 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
2026-08-05 21:26 ` [PATCH 03/12] perf jitdump: Prevent integer underflow in debug info size calculation Arnaldo Carvalho de Melo
2026-08-05 21:46   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260805145657.9EC201F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=acme@kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.