Linux Perf Users
 help / color / mirror / Atom feed
* [PATCHES v1 0/12] perf jitdump: Input validation hardening
@ 2026-08-05 13:29 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
                   ` (11 more replies)
  0 siblings, 12 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:29 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, Stephane Eranian, Stefano Sanfilippo

Hi,

This series addresses twelve classes of input validation and resource
handling bugs in the jitdump file format parser that could cause OOB
memory access or memory leaks when processing maliciously crafted or
corrupted jitdump files.

All issues were discovered by sashiko-bot during automated review of the
jitdump code path.  The bugs affect both the native-endian and byte-swap
code paths, with some checks previously only enforced during
byte-swapping.

Critical fix:

  code_size validation was bypassable via int truncation.  A record with
  code_size in [2^31, record_size - 56] passed the existing bounds check
  in jit_repipe_code_load() but truncated to a negative int when sent to
  jit_process_code_load(), so the code pointer landed ~2GiB past the
  record buffer, defeating the memchr() NUL scan and corrupting the
  injected ELF.

  Before: code_size = 0x80000010 passes the range check, then truncates
          to a negative offset
  After:  code_size > INT_MAX is rejected up front; all subsequent code
          size arithmetic stays within the record buffer

Issues fixed:

  Validation and bounds checks:

  - Validate code_size against both the record size and INT_MAX in
    jit_repipe_code_load()
  - Prevent integer underflow in the debug info size calculation
  - Bounds-check the debug entry byte-swap loop
  - Validate debug entries on the native (non-swap) path, matching the
    existing byte-swap path checks
  - Validate sym string NUL-termination in code load, bounding the
    strlen() scan to the code blob
  - Validate unwinding sizes against the record payload before allocating
  - Check the snprintf() return before computing the header size

  Stream and record handling:

  - Fix the extended header read that always failed, causing records to
    be misparsed
  - Use dirname()'s return value in jit_open(), fixing ENOTDIR failures
  - Fix funlockfile() being called on an unlocked stream in the
    jit_open() error path

  Resource management:

  - Free the event in jit_repipe_code_move()
  - Fix debug_data and unwinding_data leaks when records are overwritten

Each patch includes a Fixes: tag pointing to the offending commit,
dating back to jitdump mmap injection support (9b07e27f88b9cd78), source
line info support (598b7c6919c7bbcc), and unwinding support
(0284fecd13b6db3e), all from the original 2016 jitdump work.

Testing: Built and tested on x86_64.  No existing tests cover jitdump
parsing with malformed input; test suite expansion is left for future
work.  The final series was re-reviewed after the fixes (build-checked,
Fixes: tags verified); no regressions found.

AI assistance: This series was developed with assistance from Claude
(claude-opus-4.6) and Opencode (mimo-v2.5-free) for code analysis, patch
generation, and commit message composition.

Best regards,

- Arnaldo

Arnaldo Carvalho de Melo (12):
  perf jitdump: Fix extended header read that always fails
  perf jitdump: Validate code_size against total_size in code load
  perf jitdump: Prevent integer underflow in debug info size calculation
  perf jitdump: Bounds-check debug entry byte-swap loop
  perf jitdump: Check snprintf return before computing header size
  perf jitdump: Fix funlockfile on unlocked stream in jit_open() error path
  perf jitdump: Free event in jit_repipe_code_move()
  perf jitdump: Fix debug_data and unwinding_data leaks
  perf jitdump: Use dirname() return value in jit_open()
  perf jitdump: Validate debug entries on native (non-swap) path
  perf jitdump: Validate sym string NUL-termination in code load
  perf jitdump: Validate unwinding sizes against record payload

 tools/perf/util/jitdump.c | 126 ++++++++++++++++++++++++++++++++------
 1 file changed, 108 insertions(+), 18 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 01/12] perf jitdump: Fix extended header read that always fails
  2026-08-05 13:29 [PATCHES v1 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
@ 2026-08-05 13:30 ` 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
                   ` (10 subsequent siblings)
  11 siblings, 2 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

jit_open() sets bsz = bs before the fread() that uses bs - bsz as the
read size, making the expression always evaluate to zero.  fread() with
size 0 returns 0, which triggers the ret != 1 error path — so extended
jitdump headers (total_size > sizeof(header)) have been silently broken
since the original implementation.

Additionally, when 0 < bs <= bsz the if (bs > bsz) block is skipped
entirely, leaving extended header bytes unread in the stream.  Subsequent
jit_get_next_entry() calls then parse those leftover bytes as a
jr_prefix, corrupting the record stream.

Fix by separating the buffer growth from the read: realloc only when
bs > bsz, then unconditionally fread bs bytes when bs > 0.

Fixes: 9b07e27f88b9cd78 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 83005b30b9bf3fd7..4b7c7ba7cd95ddbb 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -224,10 +224,12 @@ jit_open(struct jit_buf_desc *jd, const char *name)
 		n = realloc(buf, bs);
 		if (!n)
 			goto error;
-		bsz = bs;
 		buf = n;
-		/* read extra we do not know about */
-		ret = fread(buf, bs - bsz, 1, jd->in);
+		bsz = bs;
+	}
+	if (bs > 0) {
+		/* consume extended header bytes from the stream */
+		ret = fread(buf, bs, 1, jd->in);
 		if (ret != 1)
 			goto error;
 	}
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 02/12] perf jitdump: Validate code_size against total_size in code load
  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:30 ` 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
                   ` (9 subsequent siblings)
  11 siblings, 2 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

jit_repipe_code_load() reads code_size from the jitdump record and uses
it to compute a pointer to the code blob:

  code = (unsigned long)jr + jr->load.p.total_size - csize;

An oversized code_size underflows the pointer arithmetic, causing OOB
reads into earlier heap memory.  Validate that code_size fits within the
record (total_size - sizeof(jr->load)) before the pointer computation.

code_size is uint64_t but csize is int; values above INT_MAX wrap
negative when narrowed into csize, which defeats the bounds check and
sends the code pointer past the end of the record.  Reject those too.

Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Assisted-by: Opencode:mimo-v2.5-free
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 4b7c7ba7cd95ddbb..3195f94187164066 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -9,6 +9,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <inttypes.h>
+#include <limits.h>
 #include <byteswap.h>
 #include <sys/stat.h>
 #include <sys/mman.h>
@@ -450,6 +451,16 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
 	csize = jr->load.code_size;
 	usize = jd->unwinding_mapped_size;
 	addr  = jr->load.code_addr;
+
+	/* code blob lives at the end of the record, validate it fits */
+	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;
+	}
+
 	sym   = (void *)((unsigned long)jr + sizeof(jr->load));
 	code  = (unsigned long)jr + jr->load.p.total_size - csize;
 	count = jr->load.code_index;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 03/12] perf jitdump: Prevent integer underflow in debug info size calculation
  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:30 ` [PATCH 02/12] perf jitdump: Validate code_size against total_size in code load Arnaldo Carvalho de Melo
@ 2026-08-05 13:30 ` Arnaldo Carvalho de Melo
  2026-08-05 14:56   ` sashiko-bot
  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
                   ` (8 subsequent siblings)
  11 siblings, 2 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian,
	Stefano Sanfilippo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

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.

Fixes: 598b7c6919c7 ("perf jit: add source line info support")
Fixes: 0284fecd13b6 ("perf jit: Add unwinding support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Stefano Sanfilippo <ssanfilippo@chromium.org>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 3195f94187164066..787f8a03dae87908 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -669,6 +669,10 @@ static int jit_repipe_debug_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->info))
+		return -1;
+
 	sz  = jr->prefix.total_size - sizeof(jr->info);
 	data = malloc(sz);
 	if (!data)
@@ -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)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 04/12] perf jitdump: Bounds-check debug entry byte-swap loop
  2026-08-05 13:29 [PATCHES v1 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  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 13:30 ` 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
                   ` (7 subsequent siblings)
  11 siblings, 2 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

The byte-swap loop for JIT_CODE_DEBUG_INFO uses array indexing
(jr->info.entries[n]) to iterate debug entries.  struct debug_entry has
a flexible array member name[], so each entry has a different size.
Array indexing computes offsets assuming fixed-size elements, landing
inside variable-length name strings after the first entry and
byte-swapping garbage.

Additionally, nr_entry is read from untrusted jitdump input without
validation against total_size, so a crafted value causes OOB reads.

Replace the array indexing with debug_entry_next() pointer arithmetic
(which correctly accounts for the variable-length name) and
bounds-check each entry against the record's total_size before
byte-swapping.

Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 787f8a03dae87908..078d3304d2b7ebce 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -318,14 +318,32 @@ jit_get_next_entry(struct jit_buf_desc *jd)
 	switch(id) {
 	case JIT_CODE_DEBUG_INFO:
 		if (jd->needs_bswap) {
+			void *end = (void *)jr + jr->prefix.total_size;
+			struct debug_entry *ent;
 			uint64_t n;
+
 			jr->info.code_addr = bswap_64(jr->info.code_addr);
 			jr->info.nr_entry  = bswap_64(jr->info.nr_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);
+
+			/*
+			 * debug_entry has a variable-length name[], so array
+			 * indexing would compute wrong offsets — use
+			 * debug_entry_next() and bounds-check each entry.
+			 */
+			ent = &jr->info.entries[0];
+			for (n = 0; n < jr->info.nr_entry; n++) {
+				if ((void *)ent + sizeof(*ent) > end)
+					break;
+				/* name must be NUL-terminated within the record */
+				if (!memchr(ent->name, '\0', (char *)end - ent->name))
+					break;
+				ent->addr    = bswap_64(ent->addr);
+				ent->lineno  = bswap_32(ent->lineno);
+				ent->discrim = bswap_32(ent->discrim);
+				ent = debug_entry_next(ent);
 			}
+			/* clamp so downstream consumers don't overrun */
+			jr->info.nr_entry = n;
 		}
 		break;
 	case JIT_CODE_UNWINDING_INFO:
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 05/12] perf jitdump: Check snprintf return before computing header size
  2026-08-05 13:29 [PATCHES v1 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2026-08-05 13:30 ` [PATCH 04/12] perf jitdump: Bounds-check debug entry byte-swap loop Arnaldo Carvalho de Melo
@ 2026-08-05 13:30 ` Arnaldo Carvalho de Melo
  2026-08-05 14:25   ` sashiko-bot
  2026-08-05 19:07   ` Ian Rogers
  2026-08-05 13:30 ` [PATCH 06/12] perf jitdump: Fix funlockfile on unlocked stream in jit_open() error path Arnaldo Carvalho de Melo
                   ` (6 subsequent siblings)
  11 siblings, 2 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

snprintf() returns the would-have-been length on truncation.  When the
jitted filename exceeds PATH_MAX, the unclamped 'size' value inflates
sizeof(event->mmap2.filename) - size into a massive underflow, causing
the header.size computation to write an oversized header.  The
subsequent write to 'id = event + header.size - idr_size' then corrupts
the heap.

Clamp size to PATH_MAX - 1 after snprintf in both jit_repipe_code_load()
and jit_repipe_code_move().

Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 078d3304d2b7ebce..fd11e07bf00b7978 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -493,6 +493,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 >= PATH_MAX)
+		size = PATH_MAX - 1;
 
 	size++; /* for \0 */
 
@@ -623,6 +626,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 >= PATH_MAX)
+		size = PATH_MAX - 1;
 
 	size++; /* for \0 */
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 06/12] perf jitdump: Fix funlockfile on unlocked stream in jit_open() error path
  2026-08-05 13:29 [PATCHES v1 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2026-08-05 13:30 ` [PATCH 05/12] perf jitdump: Check snprintf return before computing header size Arnaldo Carvalho de Melo
@ 2026-08-05 13:30 ` 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
                   ` (5 subsequent siblings)
  11 siblings, 2 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

If the malloc() for the initial read buffer fails, jit_open() jumps to
the error label which calls funlockfile(jd->in).  However, flockfile()
is called later in the function, so at this point the stream was never
locked.  Calling funlockfile() on an unlocked stream is undefined
behavior per POSIX.

Split the error path into two labels: 'error' (after flockfile) calls
funlockfile before cleanup, 'error_noflock' (before flockfile) skips
the unlock.

Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index fd11e07bf00b7978..c3f11d1c1d76d6c7 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -155,7 +155,7 @@ jit_open(struct jit_buf_desc *jd, const char *name)
 
 	buf = malloc(bsz);
 	if (!buf)
-		goto error;
+		goto error_noflock;
 
 	/*
 	 * protect from writer modifying the file while we are reading it
@@ -244,8 +244,9 @@ jit_open(struct jit_buf_desc *jd, const char *name)
 
 	return 0;
 error:
-	free(buf);
 	funlockfile(jd->in);
+error_noflock:
+	free(buf);
 	fclose(jd->in);
 	return retval;
 }
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 07/12] perf jitdump: Free event in jit_repipe_code_move()
  2026-08-05 13:29 [PATCHES v1 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  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 13:30 ` 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
                   ` (4 subsequent siblings)
  11 siblings, 2 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

jit_repipe_code_move() allocates a perf_event with calloc but never
frees it — the 'out' label exits with only perf_sample__exit().

The sibling function jit_repipe_code_load() correctly calls
free(event) at its out label.  Add the same free(event) to
jit_repipe_code_move().

Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index c3f11d1c1d76d6c7..4f52b143cd0da296 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -683,6 +683,7 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
 		build_id__mark_dso_hit(tool, event, &sample, jd->machine);
 out:
 	perf_sample__exit(&sample);
+	free(event);
 	return ret;
 }
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 08/12] perf jitdump: Fix debug_data and unwinding_data leaks
  2026-08-05 13:29 [PATCHES v1 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
                   ` (6 preceding siblings ...)
  2026-08-05 13:30 ` [PATCH 07/12] perf jitdump: Free event in jit_repipe_code_move() Arnaldo Carvalho de Melo
@ 2026-08-05 13:30 ` 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
                   ` (3 subsequent siblings)
  11 siblings, 2 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

jit_repipe_debug_info() overwrites jd->debug_data without freeing the
previous allocation.  If two consecutive JIT_CODE_DEBUG_INFO records
appear without an intervening LOAD record consuming the data, the first
allocation leaks.

The sibling jit_repipe_unwinding_info() already frees the old
jd->unwinding_data before reassignment — add the same pattern to
jit_repipe_debug_info() using zfree().

Also add cleanup of both buffers in jit_close() so they are freed when
the jitdump session ends, even if no LOAD record consumed them.

Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 4f52b143cd0da296..3085091b95a517ae 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -118,6 +118,8 @@ jit_close(struct jit_buf_desc *jd)
 	funlockfile(jd->in);
 	fclose(jd->in);
 	jd->in = NULL;
+	zfree(&jd->debug_data);
+	zfree(&jd->unwinding_data);
 }
 
 static int
@@ -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);
 	jd->debug_data       = data;
 
 	/*
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 09/12] perf jitdump: Use dirname() return value in jit_open()
  2026-08-05 13:29 [PATCHES v1 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
                   ` (7 preceding siblings ...)
  2026-08-05 13:30 ` [PATCH 08/12] perf jitdump: Fix debug_data and unwinding_data leaks Arnaldo Carvalho de Melo
@ 2026-08-05 13:30 ` 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
                   ` (2 subsequent siblings)
  11 siblings, 2 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

jit_open() calls dirname(jd->dir) but ignores the return value.  POSIX
says dirname() may return a pointer to internal static storage — glibc
does this when the path has no '/', returning "." from a static buffer
and leaving jd->dir unchanged with the original filename.

Capture the return value and copy it back to jd->dir when dirname()
returns a different pointer.

Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 3085091b95a517ae..02840dbf8a1fc16c 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -146,6 +146,7 @@ jit_open(struct jit_buf_desc *jd, const char *name)
 	ssize_t bs, bsz = 0;
 	void *n, *buf = NULL;
 	int ret, retval = -1;
+	char *dname;
 
 	nsinfo__mountns_enter(jd->nsi, &nsc);
 	jd->in = fopen(name, "r");
@@ -241,7 +242,9 @@ jit_open(struct jit_buf_desc *jd, const char *name)
 	 */
 	strncpy(jd->dir, name, PATH_MAX - 1);
 	jd->dir[PATH_MAX - 1] = '\0';
-	dirname(jd->dir);
+	dname = dirname(jd->dir);
+	if (dname != jd->dir)
+		strlcpy(jd->dir, dname, PATH_MAX);
 	free(buf);
 
 	return 0;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 10/12] perf jitdump: Validate debug entries on native (non-swap) path
  2026-08-05 13:29 [PATCHES v1 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
                   ` (8 preceding siblings ...)
  2026-08-05 13:30 ` [PATCH 09/12] perf jitdump: Use dirname() return value in jit_open() Arnaldo Carvalho de Melo
@ 2026-08-05 13:30 ` 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 13:30 ` [PATCH 12/12] perf jitdump: Validate unwinding sizes against record payload Arnaldo Carvalho de Melo
  11 siblings, 2 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

The bounds-checking and nr_entry clamping added for the byte-swap path
only runs when jd->needs_bswap is true.  On native-endian files, nr_entry
passes through unvalidated to jit_repipe_debug_info(), which stores it
as jd->nr_debug_entries.  Downstream, jit_process_debug_info() in
genelf_debug.c iterates nr_debug_entries times via debug_entry_next(),
which calls strlen() on each entry's name field — a crafted nr_entry
causes OOB reads and writes.

Add bounds-checked iteration in jit_repipe_debug_info() that validates
each debug_entry fits in the payload and its name is NUL-terminated
before calling debug_entry_next().  Clamp nr_debug_entries to the count
of valid entries.

Fixes: 598b7c6919c7bbcc ("perf jit: add source line info support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 02840dbf8a1fc16c..87612ef3e232598e 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -694,8 +694,10 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
 
 static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr)
 {
-	void *data;
-	size_t sz;
+	struct debug_entry *ent;
+	void *data, *end;
+	size_t sz, valid;
+	uint64_t i;
 
 	if (!(jd && jr))
 		return -1;
@@ -715,10 +717,25 @@ static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr)
 	jd->debug_data       = data;
 
 	/*
-	 * we must use nr_entry instead of size here because
-	 * we cannot distinguish actual entry from padding otherwise
+	 * Clamp nr_debug_entries to entries that actually fit in the
+	 * payload.  The byte-swap path already does this for cross-endian
+	 * files; validate on the native path too, since downstream
+	 * jit_process_debug_info() iterates via debug_entry_next() which
+	 * calls strlen() on each entry's name field.
 	 */
-	jd->nr_debug_entries = jr->info.nr_entry;
+	end = data + sz;
+	ent = data;
+	valid = 0;
+	for (i = 0; i < jr->info.nr_entry; i++) {
+		if ((void *)ent + sizeof(*ent) > end)
+			break;
+		/* name must be NUL-terminated within the payload */
+		if (!memchr(ent->name, '\0', (char *)end - ent->name))
+			break;
+		ent = debug_entry_next(ent);
+		valid++;
+	}
+	jd->nr_debug_entries = valid;
 
 	return 0;
 }
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 11/12] perf jitdump: Validate sym string NUL-termination in code load
  2026-08-05 13:29 [PATCHES v1 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
                   ` (9 preceding siblings ...)
  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 13:30 ` 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
  11 siblings, 1 reply; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

jit_repipe_code_load() computes sym = (void *)jr + sizeof(jr->load) and
passes it to jit_emit_elf() which calls strlen(sym) via jit_write_elf().
If code_size equals total_size - sizeof(jr->load), the sym pointer
aliases the code blob with no NUL terminator, and strlen() scans past
the buffer into adjacent heap memory.

Add a memchr() check to verify the symbol name is NUL-terminated within
the region between the load header and the code blob before use.

Fixes: 598b7c6919c7bbcc ("perf jit: add source line info support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 87612ef3e232598e..5f3a53f818c29f58 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -487,6 +487,13 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
 
 	sym   = (void *)((unsigned long)jr + sizeof(jr->load));
 	code  = (unsigned long)jr + jr->load.p.total_size - csize;
+
+	/* sym string lives between the load header and the code blob */
+	if (!memchr(sym, '\0', code - (unsigned long)sym)) {
+		pr_warning("jitdump: unterminated symbol name in code_load record\n");
+		return -1;
+	}
+
 	count = jr->load.code_index;
 	idr_size = jd->machine->id_hdr_size;
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 12/12] perf jitdump: Validate unwinding sizes against record payload
  2026-08-05 13:29 [PATCHES v1 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
                   ` (10 preceding siblings ...)
  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 13:30 ` Arnaldo Carvalho de Melo
  2026-08-05 19:20   ` Ian Rogers
  11 siblings, 1 reply; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stefano Sanfilippo,
	Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

jit_repipe_unwinding_info() copies unwinding_size and eh_frame_hdr_size
from the jitdump record into jd-> fields without checking them against
the actual payload size.  Downstream, jit_add_eh_frame_info() in
genelf.c computes unwinding_table_size = unwinding_size -
eh_frame_hdr_size, which underflows when eh_frame_hdr_size >
unwinding_size.  The result is passed as d->d_size to libelf, causing
an OOB heap read into the output ELF file.

Validate that unwinding_size fits within the record payload and that
eh_frame_hdr_size does not exceed unwinding_size before allocating or
storing the values, so a bogus record cannot force a large allocation
that is then discarded.

mapped_size is likewise taken from the record and was narrowed into an
int for the mmap2 len computation in jit_repipe_code_load() and
jit_repipe_code_move(); values above INT_MAX would turn negative,
producing a wrong mmap2 length.  Use uint64_t for usize so the value
cannot truncate.

Fixes: 0284fecd13b6db3e ("perf jit: Add unwinding support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stefano Sanfilippo <ssanfilippo@chromium.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Assisted-by: Opencode:mimo-v2.5-free
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 5f3a53f818c29f58..385c19b864aeee4c 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;
@@ -543,7 +544,7 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
 
 	event->mmap2.pgoff = GEN_ELF_TEXT_OFFSET;
 	event->mmap2.start = addr;
-	event->mmap2.len   = usize ? ALIGN_8(csize) + usize : csize;
+	event->mmap2.len   = usize ? ALIGN_8((uint64_t)csize) + usize : (uint64_t)csize;
 	event->mmap2.pid   = pid;
 	event->mmap2.tid   = tid;
 	event->mmap2.ino   = st.st_ino;
@@ -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;
@@ -761,6 +762,18 @@ jit_repipe_unwinding_info(struct jit_buf_desc *jd, union jr_entry *jr)
 		return -1;
 
 	unwinding_data_size  = jr->prefix.total_size - sizeof(jr->unwinding);
+
+	/*
+	 * Validate sizes before allocating — jit_add_eh_frame_info()
+	 * computes unwinding_size - eh_frame_hdr_size and uses the
+	 * result as a buffer length for libelf.
+	 */
+	if (jr->unwinding.unwinding_size > unwinding_data_size ||
+	    jr->unwinding.eh_frame_hdr_size > jr->unwinding.unwinding_size) {
+		pr_warning("jitdump: invalid unwinding sizes in unwinding_info record\n");
+		return -1;
+	}
+
 	unwinding_data = malloc(unwinding_data_size);
 	if (!unwinding_data)
 		return -1;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/12] perf jitdump: Fix extended header read that always fails
  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
  1 sibling, 0 replies; 41+ messages in thread
From: sashiko-bot @ 2026-08-05 13:46 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users

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

Pre-existing issues:
- [High] Missing size validation in `jit_get_next_entry` allows out-of-bounds heap access and arbitrary memory exfiltration.
- [High] Memory leak of the `event` structure in `jit_repipe_code_move`.
- [High] Memory leak of `debug_data` and `unwinding_data` buffers.
- [Medium] Undefined behavior from calling `funlockfile()` on an unlocked stream during early initialization error paths.
- [Low] Ineffective use of `flockfile()` for cross-process file synchronization.
--

--- Patch [1]: [PATCH 01/12] perf jitdump: Fix extended header read that always fails ---
commit 9c0b715100e3938e32e2120f1ac2090e75d6c0a1
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Fix extended header read that always fails

This commit fixes an issue where jit_open() sets bsz = bs before the
fread() that uses bs - bsz as the read size, making the read size zero.
It separates the buffer growth from the read to ensure extended header
bytes are correctly consumed.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 83005b30b9bf3..4b7c7ba7cd95d 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -224,10 +224,12 @@ jit_open(struct jit_buf_desc *jd, const char *name)
>  		n = realloc(buf, bs);
>  		if (!n)
>  			goto error;
> -		bsz = bs;
>  		buf = n;
> -		/* read extra we do not know about */
> -		ret = fread(buf, bs - bsz, 1, jd->in);
> +		bsz = bs;
> +	}
> +	if (bs > 0) {
> +		/* consume extended header bytes from the stream */
> +		ret = fread(buf, bs, 1, jd->in);
>  		if (ret != 1)
>  			goto error;
>  	}

[Severity: Medium]
This is a pre-existing issue, but I noticed a potential problem in the error
handling earlier in jit_open().

When the initial buffer allocation fails:

buf = malloc(bsz);
if (!buf)
    goto error;

/*
 * protect from writer modifying the file while we are reading it
 */
flockfile(jd->in);
// ...
error:
    free(buf);
    funlockfile(jd->in);

Does this cause undefined behavior by calling funlockfile() on a stream that
was never locked by flockfile()?

[Severity: Low]
This isn't a bug introduced by this patch, but is flockfile() providing the
intended synchronization here?

/*
 * protect from writer modifying the file while we are reading it
 */
flockfile(jd->in);

The comment mentions protecting against a writer modifying the file while
reading, but flockfile() only provides thread-level synchronization within the
same process. It doesn't lock the file descriptor at the OS level against a
separate JIT compiler process.

Might a file-level lock be needed instead?

[Severity: High]
This is a pre-existing issue, but could we have an out-of-bounds heap access
when reading the record prefix?

In jit_get_next_entry(), the bounds check only verifies the prefix size:

bs = (size_t)size;
if (bs < sizeof(*prefix))
    return NULL;

If a malformed file provides a prefix->total_size smaller than the specific
record struct (e.g., 17 bytes instead of the 56 bytes needed for
struct jr_code_load), the fread() will succeed but leave stale data.

Later, functions like jit_repipe_code_load() use this stale heap data:

csize = jr->load.code_size;
// ...
code  = (unsigned long)jr + jr->load.p.total_size - csize;

Can this result in an out-of-bounds pointer calculation and arbitrary memory
being exfiltrated to the output ELF file?

[Severity: High]
This is a pre-existing issue, but does jit_repipe_code_move() leak the event
structure?

event = calloc(1, sizeof(*event) + 16);
if (!event)
    return -1;
// ...
out:
    perf_sample__exit(&sample);
    return ret;

Unlike other repipe functions that free the event, the event pointer here
appears to go out of scope without being freed, potentially causing the tool
to OOM on large dumps with many JIT_CODE_MOVE records.

[Severity: High]
This is a pre-existing issue, but is there a memory leak when processing debug
info records?

In jit_repipe_debug_info(), memory is allocated and assigned unconditionally:

data = malloc(sz);
if (!data)
    return -1;

memcpy(data, &jr->info.entries, sz);

jd->debug_data = data;

This replaces jd->debug_data without freeing any previously held allocation.

Additionally, in jit_process():

nsinfo__put(jd.nsi);
free(jd.buf);

return ret;

It looks like jd.debug_data and jd.unwinding_data are not cleaned up when
processing finishes. Should these buffers be freed?

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 02/12] perf jitdump: Validate code_size against total_size in code load
  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
  1 sibling, 0 replies; 41+ messages in thread
From: sashiko-bot @ 2026-08-05 13:59 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users

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

Pre-existing issues:
- [High] `jit_repipe_code_move()` contains a memory leak and a heap buffer overflow due to `snprintf` truncation.
- [Medium] Missing cleanup of `jd->debug_data` and `jd->unwinding_data` causes memory leaks.
- [Medium] Missing bounds check in `jit_repipe_code_move()` allows reading uninitialized heap memory and injecting it into the output `perf.data`.
- [Medium] Integer underflow when parsing `JIT_CODE_UNWINDING_INFO` size causes a massive out-of-bounds heap read and SIGSEGV.
- [Medium] Callback errors are silently swallowed in the `jit_process_dump()` loop, masking data validation failures.
--

--- Patch [2]: [PATCH 02/12] perf jitdump: Validate code_size against total_size in code load ---
commit 386282de6b8b07ac74f2456d302053e3738822ce
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Validate code_size against total_size in code load

This commit adds bounds checking for code_size in the jitdump record to
prevent out-of-bounds heap reads when processing code load entries.

> --- tools/perf/util/jitdump.c
> +++ tools/perf/util/jitdump.c
[ ... ]
> @@ -450,6 +451,16 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
>  	csize = jr->load.code_size;
>  	usize = jd->unwinding_mapped_size;
>  	addr  = jr->load.code_addr;
> +
> +	/* code blob lives at the end of the record, validate it fits */
> +	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;
> +	}
> +

[Severity: Medium]
This isn't a bug introduced by this patch, but does this early return skip
the cleanup for jd->debug_data and jd->unwinding_data?

When parsing consecutive debug info records, the previous state allocations
are unconditionally overwritten without being freed. If the bounds check
fails here or the stream ends, the buffers appear to leak since neither
the processing loop nor jit_close() cleans them up.

[Severity: Medium]
This is a pre-existing issue, but will the caller actually abort when we
return -1 here?

Looking at jit_process_dump(), the loop assigns the return value but doesn't
seem to check it before continuing:

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;
    ...
}

Does this mean errors are silently swallowed and parsing proceeds on
corrupted state, masking the data validation failures?

>  	sym   = (void *)((unsigned long)jr + sizeof(jr->load));

[Severity: Medium]
This is a pre-existing issue, but does jit_repipe_code_move() lack a
similar bounds check?

The generic parser only guarantees a minimum size of 16 bytes for a record,
but the move function accesses fields unconditionally:

jit_repipe_code_move() {
    ...
    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 allow reading uninitialized heap memory and injecting it into
the output perf.data if the record is truncated?

[Severity: High]
This is a pre-existing issue, but is there a memory leak and potential
heap buffer overflow in jit_repipe_code_move()?

Unlike jit_repipe_code_load(), the move function fails to clamp the snprintf
return value for the size:

jit_repipe_code_move() {
    ...
    size = snprintf(filename, PATH_MAX, "%s/jitted-%d-%" PRIu64 ".so",
                    jd->dir,
                    nspid,
                    jr->move.code_index);
    size++; /* for \0 */
    ...
}

If the parsed path exceeds PATH_MAX, snprintf returns the would-be length,
which inflates event->mmap2.header.size. Does this corrupt the subsequent
id pointer offset and lead to an out-of-bounds write?

Also, the dynamically allocated event buffer appears to be leaked entirely
on both success and error paths before the function returns:

jit_repipe_code_move() {
    ...
out:
    perf_sample__exit(&sample);
    return ret;
}

[Severity: Medium]
This is a pre-existing issue, but could jit_repipe_unwinding_info()
suffer from integer underflow?

If a malformed file supplies a total_size less than sizeof(jr->unwinding),
the unwinding_data_size calculation underflows into a very large positive
integer:

jit_repipe_unwinding_info() {
    ...
    unwinding_data_size  = jr->prefix.total_size - sizeof(jr->unwinding);
    unwinding_data = malloc(unwinding_data_size);
    ...
    memcpy(unwinding_data, &jr->unwinding.unwinding_data,
           unwinding_data_size);
    ...
}

Could this lead to an immediate out-of-bounds read crash if the subsequent
malloc succeeds overcommitting?

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 09/12] perf jitdump: Use dirname() return value in jit_open()
  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
  1 sibling, 0 replies; 41+ messages in thread
From: sashiko-bot @ 2026-08-05 14:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users

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

Pre-existing issues:
- [High] Missing total_size validation in JIT_CODE_MOVE record processing allows uninitialized heap memory to be leaked into the output perf.data file.
- [High] An insufficient memory allocation in jit_repipe_code_move() causes a heap out-of-bounds read when writing the generated event to the output file.
--

--- Patch [9]: [PATCH 09/12] perf jitdump: Use dirname() return value in jit_open() ---
commit 8e32408a1e72fef109700c74dd502aaacc3125c7
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Use dirname() return value in jit_open()

jit_open() calls dirname(jd->dir) but ignores the return value.  POSIX
says dirname() may return a pointer to internal static storage — glibc
does this when the path has no '/', returning "." from a static buffer
and leaving jd->dir unchanged with the original filename.

Capture the return value and copy it back to jd->dir when dirname()
returns a different pointer.

Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

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

[ ... ]

> static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
> {
> 	struct perf_sample sample;
> 	union perf_event *event;
> 	const struct perf_tool *tool = jd->session->tool;
> 	char *filename;
> 	size_t size;
> 	struct stat st;
> 	int usize;
> 	u16 idr_size;
> 	int ret;
> 	pid_t nspid, pid, tid;
> 	struct {
> 		u32 pid, tid;
> 		u64 time;
> 	} *id;
> 
> 	nspid = jr->load.pid;
> 	pid   = jr_entry_pid(jd, jr);
> 	tid   = jr_entry_tid(jd, jr);

[Severity: High]
This isn't a bug introduced by this patch, but is there a missing bounds check
here?

Unlike jit_repipe_code_load(), this function appears to blindly access fields
from the code move record without first verifying that the record total_size
is at least sizeof(jr->move).

If a parsed jitdump file contains a truncated code move record, could this
read uninitialized fields from the jd->buf buffer and leak leftover heap data
into the output perf.data file?

> 	usize = jd->unwinding_mapped_size;
> 	idr_size = jd->machine->id_hdr_size;
> 
> 	/*
> 	 * +16 to account for sample_id_all (hack)
> 	 */
> 	event = calloc(1, sizeof(*event) + 16);

[Severity: High]
This is a pre-existing issue, but is this allocation large enough for the
generated event?

The memory is allocated with a hardcoded + 16 extra bytes. If the session's
sample_type includes multiple fields (like CPU, TIME, and TID), idr_size can
easily exceed 16 bytes.

> 	if (!event)
> 		return -1;

[ ... ]

> 	event->mmap2.header.type = PERF_RECORD_MMAP2;
> 	event->mmap2.header.misc = PERF_RECORD_MISC_USER;
> 	event->mmap2.header.size = (sizeof(event->mmap2) -
> 			(sizeof(event->mmap2.filename) - size) + idr_size);

[Severity: High]
This is also a pre-existing issue related to the allocation above, but might
this cause a heap out-of-bounds read?

When the header size is calculated here using the dynamically larger idr_size,
the size will exceed the allocated space by (idr_size - 16) bytes.

Would this cause perf_data__write() to read out-of-bounds heap memory?

Should the code dynamically allocate (sizeof(*event) + idr_size) instead?

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 05/12] perf jitdump: Check snprintf return before computing header size
  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
  1 sibling, 0 replies; 41+ messages in thread
From: sashiko-bot @ 2026-08-05 14:25 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users

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

Pre-existing issues:
- [High] Insufficient heap allocation in `jit_repipe_code_move` when `idr_size` exceeds 16 bytes causes an out-of-bounds heap read and information leak.
- [High] The dynamically allocated `event` buffer is never freed in `jit_repipe_code_move()`.
- [High] Hardcoded sample ID structure layout causes a heap out-of-bounds write and event corruption when standard layout assumptions are violated.
- [High] Predictable filenames are opened without `O_EXCL` and `O_NOFOLLOW` in a potentially shared directory, enabling a symlink attack to overwrite arbitrary files.
--

--- Patch [5]: [PATCH 05/12] perf jitdump: Check snprintf return before computing header size ---
commit c4d5fd2e9b69d8618378d2b3d3ea73b74c28f945
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Check snprintf return before computing header size

This commit clamps the return value of snprintf to PATH_MAX - 1 when computing
the length of a generated JIT filename. This prevents an out-of-bounds heap
read/write caused by size underflow when the jitted filename exceeds PATH_MAX.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 078d3304d2b7e..fd11e07bf00b7 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -493,6 +493,9 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
>  			jd->dir,
>  			nspid,
>  			count);

[Severity: High]
This is a pre-existing issue, but does this predictable filename creation allow
for a symlink attack? 

The filename is constructed using jd->dir, which could be a world-writable or
attacker-controlled path. When jit_emit_elf() subsequently creates this file,
it omits O_EXCL and O_NOFOLLOW:

    fd = open(filename, O_CREAT|O_TRUNC|O_WRONLY, 0644);

If a local attacker places a symlink in this directory pointing to a sensitive
file like /etc/shadow, could this result in an arbitrary file overwrite when
the file is opened and truncated?

[Severity: High]
This isn't a bug introduced by this patch, but can the sample ID structure
layout assumption result in out-of-bounds heap writes?

In jit_repipe_code_load(), the pointer is mapped to the trailing bytes of the
payload and cast to a hardcoded struct:

    id = (void *)((unsigned long)event + event->mmap.header.size - idr_size);
    if (jd->sample_type & PERF_SAMPLE_TID) {
        id->pid  = pid;
        id->tid  = tid;
    }
    if (jd->sample_type & PERF_SAMPLE_TIME)
        id->time = convert_timestamp(jd, jr->load.p.timestamp);

If the user configures the perf session to omit TID or include other fields
preceding TID, idr_size might only be 8 bytes for time. Writing to id->time
(at offset 8) would then write exactly 8 bytes out of bounds of the allocated
heap space. The identical struct mapping is also repeated later in
jit_repipe_code_move().

> +	/* snprintf returns would-be length on truncation, clamp to buffer */
> +	if (size >= PATH_MAX)
> +		size = PATH_MAX - 1;
>  
>  	size++; /* for \0 */

[ ... ]

> @@ -623,6 +626,9 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
>  	         jd->dir,
>  		 nspid,
>  		 jr->move.code_index);

[Severity: High]
This isn't a bug introduced by this patch, but might the hardcoded buffer
allocation size in jit_repipe_code_move() lead to an out-of-bounds read?

The event buffer is allocated with a fixed 16 bytes for the sample ID:

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

However, the header size is computed using the dynamic idr_size:

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

If idr_size exceeds 16 bytes (for example, if the perf session is configured
with PERF_SAMPLE_TID | PERF_SAMPLE_TIME | PERF_SAMPLE_ID), header.size will
exceed the allocated bounds. When the event is later written in
jit_inject_event():

    size = perf_data__write(jd->output, event, event->header.size);

Could this result in reading past the end of the heap buffer and leaking
sensitive heap contents to the output file?

[Severity: High]
This is a pre-existing issue, but is there a memory leak of the dynamically
allocated event buffer in jit_repipe_code_move()?

The function allocates the event buffer, but appears to return on both success
and error paths without freeing it:

out:
	perf_sample__exit(&sample);
	return ret;

Since perf can process millions of events in a single run, could leaking this
buffer per move event rapidly exhaust system memory and lead to an
Out-Of-Memory crash during standard analysis?

> +	/* snprintf returns would-be length on truncation, clamp to buffer */
> +	if (size >= PATH_MAX)
> +		size = PATH_MAX - 1;
>  
>  	size++; /* for \0 */

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 10/12] perf jitdump: Validate debug entries on native (non-swap) path
  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
  1 sibling, 0 replies; 41+ messages in thread
From: sashiko-bot @ 2026-08-05 14:26 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users

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

Pre-existing issues:
- [High] The JVMTI agent and debug_entry array iterates over tightly packed, variable-length structures without padding, causing unaligned memory accesses when reading or writing 64-bit and 32-bit fields.
- [High] The functions `jit_get_next_entry()` and `jit_repipe_code_move()` lack bounds checking to ensure that the parsed `JIT_CODE_MOVE` record is at least `sizeof(struct jr_code_move)` bytes in length.
- [High] The arguments `unwinding_header_size` and `unwinding_size` are silently truncated from 64-bit to 32-bit before being passed to `jit_write_elf()`.
--

--- Patch [10]: [PATCH 10/12] perf jitdump: Validate debug entries on native (non-swap) path ---
commit 1922967864de6630181cc1d54ced351d4019f568
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Validate debug entries on native (non-swap) path

This commit fixes an out-of-bounds read and write on native-endian files
by adding bounds checking and NUL-termination validation for debug
entries in jit_repipe_debug_info(). It clamps nr_debug_entries to the
count of valid entries that fit in the payload.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 02840dbf8a1fc..87612ef3e2325 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -694,8 +694,10 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)

[Severity: High]
This isn't a bug introduced by this patch, but does jit_repipe_code_move()
lack bounds checking for the JIT_CODE_MOVE record size?

If a crafted JIT dump provides a JIT_CODE_MOVE record with a total_size
smaller than sizeof(struct jr_code_move), jit_get_next_entry() might only
read the provided smaller size into jd->buf.

Could extracting fields like jr->move.new_code_addr in jit_repipe_code_move()
then read out-of-bounds heap memory and leak stale data into the generated
perf.data file?

>  
>  static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr)
>  {
> -	void *data;
> -	size_t sz;
> +	struct debug_entry *ent;
> +	void *data, *end;
> +	size_t sz, valid;
> +	uint64_t i;

[ ... ]

> @@ -715,10 +717,25 @@ static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr)
>  	jd->debug_data       = data;
>  
>  	/*
> -	 * we must use nr_entry instead of size here because
> -	 * we cannot distinguish actual entry from padding otherwise
> +	 * Clamp nr_debug_entries to entries that actually fit in the
> +	 * payload.  The byte-swap path already does this for cross-endian
> +	 * files; validate on the native path too, since downstream
> +	 * jit_process_debug_info() iterates via debug_entry_next() which
> +	 * calls strlen() on each entry's name field.
>  	 */
> -	jd->nr_debug_entries = jr->info.nr_entry;
> +	end = data + sz;
> +	ent = data;
> +	valid = 0;
> +	for (i = 0; i < jr->info.nr_entry; i++) {
> +		if ((void *)ent + sizeof(*ent) > end)
> +			break;
> +		/* name must be NUL-terminated within the payload */
> +		if (!memchr(ent->name, '\0', (char *)end - ent->name))
> +			break;
> +		ent = debug_entry_next(ent);

[Severity: High]
This is a pre-existing issue, but does advancing the pointer this way lead
to unaligned memory accesses?

Looking at tools/perf/util/jitdump.h, debug_entry_next() computes the next
pointer without enforcing 8-byte alignment:

static inline struct debug_entry *
debug_entry_next(struct debug_entry *ent)
{
	void *a = ent + 1;
	size_t l = strlen(ent->name) + 1;
	return a + l;
}

Since the variable-length name string is not padded, the returned pointer
might not be 64-bit aligned. 

When jit_process_debug_info() or jit_get_next_entry() subsequently accesses
ent->addr (a 64-bit integer), could this trigger a SIGBUS crash on
architectures that enforce strict alignment?

> +		valid++;
> +	}
> +	jd->nr_debug_entries = valid;
>  
>  	return 0;
>  }

[Severity: High]
This is also a pre-existing issue, but does jit_emit_elf() silently truncate
64-bit size values?

In tools/perf/util/jitdump.c, jit_repipe_code_load() passes the 64-bit fields
jd->eh_frame_hdr_size and jd->unwinding_size to jit_emit_elf():

ret = jit_emit_elf(jd, filename, sym, addr, (const void *)uaddr, csize,
                   jd->debug_data, jd->nr_debug_entries,
                   jd->unwinding_data, jd->eh_frame_hdr_size,
                   jd->unwinding_size);

However, jit_emit_elf() defines these parameters as 32-bit integers:

void *unwinding,
uint32_t unwinding_header_size,
uint32_t unwinding_size)

If a crafted JIT dump provides 64-bit sizes that are truncated when passed
to jit_emit_elf(), could this result in an unwinding_size smaller than
unwinding_header_size?

When jit_write_elf() widens these back to 64-bit and subtracts them, could
this underflow the unwinding_table_size and cause massive out-of-bounds
operations during ELF generation?

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 04/12] perf jitdump: Bounds-check debug entry byte-swap loop
  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
  1 sibling, 0 replies; 41+ messages in thread
From: sashiko-bot @ 2026-08-05 14:27 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users

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

Pre-existing issues:
- [High] Incomplete fix for out-of-bounds read/write in JIT_CODE_DEBUG_INFO parsing
- [High] Missing bounds validation for JIT_CODE_UNWINDING_INFO sizes leads to OOB heap read and information leak
- [High] Missing null-terminator validation for JIT_CODE_LOAD symbol string causes OOB read and information leak
--

--- Patch [4]: [PATCH 04/12] perf jitdump: Bounds-check debug entry byte-swap loop ---
commit 2ca1b158f7ca3eafd8478fe7713f2806ce7b4fce
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: perf jitdump: Bounds-check debug entry byte-swap loop

This commit replaces array indexing with debug_entry_next() pointer arithmetic
for JIT_CODE_DEBUG_INFO records, and adds bounds-checking against the record's
total_size. This prevents out-of-bounds reads during byte-swapping of untrusted
jitdump input.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 787f8a03dae87..078d3304d2b7e 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -318,14 +318,32 @@ jit_get_next_entry(struct jit_buf_desc *jd)
>  	switch(id) {
>  	case JIT_CODE_DEBUG_INFO:
>  		if (jd->needs_bswap) {
> +			void *end = (void *)jr + jr->prefix.total_size;

[Severity: High]
This is a pre-existing issue, but does this skip validation for native-endian
files?

The bounds checking and clamping are only performed when jd->needs_bswap is true.
If the file is native-endian, the untrusted nr_entry is passed directly to the
downstream loop without validation:

tools/perf/util/genelf_debug.c:jit_process_debug_info() {
    ...
    for (i = 0; i < nr_debug_entries; i++) {
        ent->addr = ent->addr - code_addr;
        ent = debug_entry_next(ent);
    }
    ...
}

Could this lead to out-of-bounds heap reads and writes when processing a crafted
JIT_CODE_DEBUG_INFO record with native endianness?

[ ... ]

> +			/* clamp so downstream consumers don't overrun */
> +			jr->info.nr_entry = n;
>  		}
>  		break;
>  	case JIT_CODE_UNWINDING_INFO:

[Severity: High]
This is a pre-existing issue, but is the unwinding_size bounded by the actual
record size?

In jit_repipe_unwinding_info(), jd->unwinding_size is set directly from the
untrusted jr->unwinding.unwinding_size. Later, this unvalidated size is passed
to libelf:

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

Could this cause an out-of-bounds heap read and embed adjacent heap memory into
the generated ELF file?

>  		if (jd->needs_bswap) {
>  			jr->unwinding.unwinding_size = bswap_64(jr->unwinding.unwinding_size);
>  			jr->unwinding.eh_frame_hdr_size = bswap_64(jr->unwinding.eh_frame_hdr_size);
>  			jr->unwinding.mapped_size = bswap_64(jr->unwinding.mapped_size);
>  		}
>  		break;
>  	case JIT_CODE_CLOSE:
>  		break;
>  	case JIT_CODE_LOAD:

[Severity: High]
This is a pre-existing issue, but is the symbol string in JIT_CODE_LOAD checked
for null-termination within the record bounds?

In jit_repipe_code_load(), the string pointer is derived from the record
without validation:

tools/perf/util/jitdump.c:jit_repipe_code_load() {
    ...
    sym = (void *)((unsigned long)jr + sizeof(jr->load));
    ...
}

This unvalidated string is later passed to strlen() and strcpy() in
jit_write_elf(), which could scan past the record into adjacent heap memory and
leak data into the generated ELF string table.

>  		if (jd->needs_bswap) {
>  			jr->load.pid       = bswap_32(jr->load.pid);
>  			jr->load.tid       = bswap_32(jr->load.tid);

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 07/12] perf jitdump: Free event in jit_repipe_code_move()
  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
  1 sibling, 0 replies; 41+ messages in thread
From: sashiko-bot @ 2026-08-05 14:33 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users

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

Pre-existing issues:
- [High] Potential heap out-of-bounds read and information leak due to hardcoded memory allocation padding versus dynamic header size.
--

--- Patch [7]: [PATCH 07/12] perf jitdump: Free event in jit_repipe_code_move() ---
commit 11d0d0b8d917333552119016252e41efe5f10dbc
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Free event in jit_repipe_code_move()

jit_repipe_code_move() allocates a perf_event with calloc but never
frees it — the 'out' label exits with only perf_sample__exit().

Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index c3f11d1c1d76d..4f52b143cd0da 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -683,6 +683,7 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)

[Severity: High]
This isn't a bug introduced by this patch, but could the hardcoded + 16
allocation padding lead to an out-of-bounds heap read in
jit_repipe_code_move()?

The event buffer is allocated with a hardcoded 16 bytes of padding:

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

Later, the header size is computed dynamically using idr_size:

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

If the jitted filename path length reaches PATH_MAX, the subtraction
(sizeof(...) - size) approaches 0, making the header.size evaluate to
sizeof(event->mmap2) + idr_size.

When idr_size (the sample ID header payload size) exceeds 16 bytes, which
can happen when multiple PERF_SAMPLE_* flags are requested, the computed
header.size will exceed the allocated chunk size.

Could this cause jit_inject_event() to call perf_data__write() with a size
larger than the allocation, potentially leaking uninitialized heap memory
into the perf.data output file?

>  		build_id__mark_dso_hit(tool, event, &sample, jd->machine);
>  out:
>  	perf_sample__exit(&sample);
> +	free(event);
>  	return ret;
>  }

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 08/12] perf jitdump: Fix debug_data and unwinding_data leaks
  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
  1 sibling, 0 replies; 41+ messages in thread
From: sashiko-bot @ 2026-08-05 14:35 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users

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 <acme@redhat.com>

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 <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

> 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/20260805133013.235016-1-acme@kernel.org?part=8

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 03/12] perf jitdump: Prevent integer underflow in debug info size calculation
  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
  2026-08-05 18:59   ` Ian Rogers
  1 sibling, 0 replies; 41+ messages in thread
From: sashiko-bot @ 2026-08-05 14:56 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 06/12] perf jitdump: Fix funlockfile on unlocked stream in jit_open() error path
  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
  1 sibling, 0 replies; 41+ messages in thread
From: sashiko-bot @ 2026-08-05 15:48 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users

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

Pre-existing issues:
- [Critical] Unvalidated nr_entry field in JIT_CODE_DEBUG_INFO leads to arbitrary heap out-of-bounds read and write.
- [High] Insecure file creation in jit_emit_elf allows arbitrary file overwrite via symlink attacks, potentially leading to privilege escalation.
--

--- Patch [6]: [PATCH 06/12] perf jitdump: Fix funlockfile on unlocked stream in jit_open() error path ---
commit 9c3ece5485fc42d2bde29b52e999638dd26c389f
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Fix funlockfile on unlocked stream in jit_open() error path

If the malloc() for the initial read buffer fails, jit_open() jumps to
the error label which calls funlockfile(jd->in).  However, flockfile()
is called later in the function, so at this point the stream was never
locked.  Calling funlockfile() on an unlocked stream is undefined
behavior per POSIX.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index fd11e07bf00b7..c3f11d1c1d76d 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -155,7 +155,7 @@ jit_open(struct jit_buf_desc *jd, const char *name)
>  
>  	buf = malloc(bsz);
>  	if (!buf)
> -		goto error;
> +		goto error_noflock;
>  
>  	/*
>  	 * protect from writer modifying the file while we are reading it
> @@ -244,8 +244,9 @@ jit_open(struct jit_buf_desc *jd, const char *name)
>  
>  	return 0;
>  error:
> -	free(buf);
>  	funlockfile(jd->in);
> +error_noflock:
> +	free(buf);
>  	fclose(jd->in);
>  	return retval;
>  }

[Severity: High]
This is a pre-existing issue, but I noticed an insecure file creation bug
in jit_emit_elf() while reviewing this file:

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);
    ...
}

The open() call truncates and writes to the file, but lacks O_EXCL or
O_NOFOLLOW.

Does this leave the file creation vulnerable to symlink-based file overwrite
attacks?

If a local attacker pre-creates a symlink matching the expected .so output
filename (e.g., jitted-<pid>-<count>.so) in a world-writable directory (like
/tmp), could they point it to a sensitive system file such as /etc/shadow?

When a privileged user runs perf inject on the attacker's perf.data file,
could the target file be truncated and overwritten with an ELF binary
containing jitted code?


[Severity: Critical]
This is a pre-existing issue, but the nr_entry field is completely unvalidated
against the actual JIT_CODE_DEBUG_INFO record size when jd->needs_bswap is
false (native endianness):

tools/perf/util/jitdump.c:jit_get_next_entry() {
    ...
	case JIT_CODE_DEBUG_INFO:
		if (jd->needs_bswap) {
    ...
			/* clamp so downstream consumers don't overrun */
			jr->info.nr_entry = n;
		}
		break;
    ...
}

If a maliciously crafted jitdump file provides an artificially large nr_entry,
could it cause this loop in jit_process_debug_info() to blindly iterate
based on the unvalidated value?

tools/perf/util/genelf_debug.c:jit_process_debug_info() {
    ...
	for (i = 0; i < nr_debug_entries; i++) {
		ent->addr = ent->addr - code_addr;
		ent = debug_entry_next(ent);
	}
    ...
}

If so, it appears debug_entry_next() could read out of bounds, and the
ent->addr = ent->addr - code_addr; operation might repeatedly subtract fully
attacker-controlled values (code_addr) from arbitrary heap metadata.

Could this lead to arbitrary code execution when processing an untrusted file?

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/12] perf jitdump: Fix extended header read that always fails
  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
  1 sibling, 0 replies; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 18:51 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> jit_open() sets bsz = bs before the fread() that uses bs - bsz as the
> read size, making the expression always evaluate to zero.  fread() with
> size 0 returns 0, which triggers the ret != 1 error path — so extended
> jitdump headers (total_size > sizeof(header)) have been silently broken
> since the original implementation.
>
> Additionally, when 0 < bs <= bsz the if (bs > bsz) block is skipped
> entirely, leaving extended header bytes unread in the stream.  Subsequent
> jit_get_next_entry() calls then parse those leftover bytes as a
> jr_prefix, corrupting the record stream.
>
> Fix by separating the buffer growth from the read: realloc only when
> bs > bsz, then unconditionally fread bs bytes when bs > 0.
>
> Fixes: 9b07e27f88b9cd78 ("perf inject: Add jitdump mmap injection support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> ---
>  tools/perf/util/jitdump.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 83005b30b9bf3fd7..4b7c7ba7cd95ddbb 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -224,10 +224,12 @@ jit_open(struct jit_buf_desc *jd, const char *name)
>                 n = realloc(buf, bs);
>                 if (!n)
>                         goto error;
> -               bsz = bs;
>                 buf = n;
> -               /* read extra we do not know about */
> -               ret = fread(buf, bs - bsz, 1, jd->in);
> +               bsz = bs;
> +       }
> +       if (bs > 0) {
> +               /* consume extended header bytes from the stream */
> +               ret = fread(buf, bs, 1, jd->in);
>                 if (ret != 1)
>                         goto error;
>         }
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 02/12] perf jitdump: Validate code_size against total_size in code load
  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
  1 sibling, 0 replies; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 18:58 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> jit_repipe_code_load() reads code_size from the jitdump record and uses
> it to compute a pointer to the code blob:
>
>   code = (unsigned long)jr + jr->load.p.total_size - csize;
>
> An oversized code_size underflows the pointer arithmetic, causing OOB
> reads into earlier heap memory.  Validate that code_size fits within the
> record (total_size - sizeof(jr->load)) before the pointer computation.
>
> code_size is uint64_t but csize is int; values above INT_MAX wrap
> negative when narrowed into csize, which defeats the bounds check and
> sends the code pointer past the end of the record.  Reject those too.
>
> Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: Claude:claude-opus-4.6
> Assisted-by: Opencode:mimo-v2.5-free
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/jitdump.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 4b7c7ba7cd95ddbb..3195f94187164066 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -9,6 +9,7 @@
>  #include <fcntl.h>
>  #include <unistd.h>
>  #include <inttypes.h>
> +#include <limits.h>

very minor nit: The header files are only partially ordered which
means we may accidentally double include header files later, etc. `git
clang-format` should be able to automatically clean this up.

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

>  #include <byteswap.h>
>  #include <sys/stat.h>
>  #include <sys/mman.h>
> @@ -450,6 +451,16 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
>         csize = jr->load.code_size;
>         usize = jd->unwinding_mapped_size;
>         addr  = jr->load.code_addr;
> +
> +       /* code blob lives at the end of the record, validate it fits */
> +       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;
> +       }
> +
>         sym   = (void *)((unsigned long)jr + sizeof(jr->load));
>         code  = (unsigned long)jr + jr->load.p.total_size - csize;
>         count = jr->load.code_index;
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 03/12] perf jitdump: Prevent integer underflow in debug info size calculation
  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
@ 2026-08-05 18:59   ` Ian Rogers
  1 sibling, 0 replies; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 18:59 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian, Stefano Sanfilippo

On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> 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.
>
> Fixes: 598b7c6919c7 ("perf jit: add source line info support")
> Fixes: 0284fecd13b6 ("perf jit: Add unwinding support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Stefano Sanfilippo <ssanfilippo@chromium.org>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> ---
>  tools/perf/util/jitdump.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 3195f94187164066..787f8a03dae87908 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -669,6 +669,10 @@ static int jit_repipe_debug_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->info))
> +               return -1;
> +
>         sz  = jr->prefix.total_size - sizeof(jr->info);
>         data = malloc(sz);
>         if (!data)
> @@ -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)
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 04/12] perf jitdump: Bounds-check debug entry byte-swap loop
  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
  1 sibling, 0 replies; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 19:00 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> The byte-swap loop for JIT_CODE_DEBUG_INFO uses array indexing
> (jr->info.entries[n]) to iterate debug entries.  struct debug_entry has
> a flexible array member name[], so each entry has a different size.
> Array indexing computes offsets assuming fixed-size elements, landing
> inside variable-length name strings after the first entry and
> byte-swapping garbage.
>
> Additionally, nr_entry is read from untrusted jitdump input without
> validation against total_size, so a crafted value causes OOB reads.
>
> Replace the array indexing with debug_entry_next() pointer arithmetic
> (which correctly accounts for the variable-length name) and
> bounds-check each entry against the record's total_size before
> byte-swapping.
>
> Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> ---
>  tools/perf/util/jitdump.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 787f8a03dae87908..078d3304d2b7ebce 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -318,14 +318,32 @@ jit_get_next_entry(struct jit_buf_desc *jd)
>         switch(id) {
>         case JIT_CODE_DEBUG_INFO:
>                 if (jd->needs_bswap) {
> +                       void *end = (void *)jr + jr->prefix.total_size;
> +                       struct debug_entry *ent;
>                         uint64_t n;
> +
>                         jr->info.code_addr = bswap_64(jr->info.code_addr);
>                         jr->info.nr_entry  = bswap_64(jr->info.nr_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);
> +
> +                       /*
> +                        * debug_entry has a variable-length name[], so array
> +                        * indexing would compute wrong offsets — use
> +                        * debug_entry_next() and bounds-check each entry.
> +                        */
> +                       ent = &jr->info.entries[0];
> +                       for (n = 0; n < jr->info.nr_entry; n++) {
> +                               if ((void *)ent + sizeof(*ent) > end)
> +                                       break;
> +                               /* name must be NUL-terminated within the record */
> +                               if (!memchr(ent->name, '\0', (char *)end - ent->name))
> +                                       break;
> +                               ent->addr    = bswap_64(ent->addr);
> +                               ent->lineno  = bswap_32(ent->lineno);
> +                               ent->discrim = bswap_32(ent->discrim);
> +                               ent = debug_entry_next(ent);
>                         }
> +                       /* clamp so downstream consumers don't overrun */
> +                       jr->info.nr_entry = n;
>                 }
>                 break;
>         case JIT_CODE_UNWINDING_INFO:
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 05/12] perf jitdump: Check snprintf return before computing header size
  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
  1 sibling, 1 reply; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 19:07 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> snprintf() returns the would-have-been length on truncation.  When the
> jitted filename exceeds PATH_MAX, the unclamped 'size' value inflates
> sizeof(event->mmap2.filename) - size into a massive underflow, causing
> the header.size computation to write an oversized header.  The
> subsequent write to 'id = event + header.size - idr_size' then corrupts
> the heap.
>
> Clamp size to PATH_MAX - 1 after snprintf in both jit_repipe_code_load()
> and jit_repipe_code_move().
>
> Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/jitdump.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 078d3304d2b7ebce..fd11e07bf00b7978 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -493,6 +493,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 >= PATH_MAX)
> +               size = PATH_MAX - 1;

Given the recent fixes to reading /proc/pid/maps where it was assumed
the file paths would be limited to PATH_MAX and it turns out that
PATH_MAX doesn't really do that and we had potential buffer overruns
during synthesis, I wonder it would be more intention revealing here
to use "sizeof(event->mmap2.filename)" rather than PATH_MAX. Other
than the constant used, I agree with the change and using PATH_MAX
isn't wrong.

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

>
>         size++; /* for \0 */
>
> @@ -623,6 +626,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 >= PATH_MAX)
> +               size = PATH_MAX - 1;
>
>         size++; /* for \0 */
>
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 06/12] perf jitdump: Fix funlockfile on unlocked stream in jit_open() error path
  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
  1 sibling, 0 replies; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 19:09 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> If the malloc() for the initial read buffer fails, jit_open() jumps to
> the error label which calls funlockfile(jd->in).  However, flockfile()
> is called later in the function, so at this point the stream was never
> locked.  Calling funlockfile() on an unlocked stream is undefined
> behavior per POSIX.
>
> Split the error path into two labels: 'error' (after flockfile) calls
> funlockfile before cleanup, 'error_noflock' (before flockfile) skips
> the unlock.
>
> Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> ---
>  tools/perf/util/jitdump.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index fd11e07bf00b7978..c3f11d1c1d76d6c7 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -155,7 +155,7 @@ jit_open(struct jit_buf_desc *jd, const char *name)
>
>         buf = malloc(bsz);
>         if (!buf)
> -               goto error;
> +               goto error_noflock;
>
>         /*
>          * protect from writer modifying the file while we are reading it
> @@ -244,8 +244,9 @@ jit_open(struct jit_buf_desc *jd, const char *name)
>
>         return 0;
>  error:
> -       free(buf);
>         funlockfile(jd->in);
> +error_noflock:
> +       free(buf);
>         fclose(jd->in);
>         return retval;
>  }
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 07/12] perf jitdump: Free event in jit_repipe_code_move()
  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
  1 sibling, 0 replies; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 19:09 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> jit_repipe_code_move() allocates a perf_event with calloc but never
> frees it — the 'out' label exits with only perf_sample__exit().
>
> The sibling function jit_repipe_code_load() correctly calls
> free(event) at its out label.  Add the same free(event) to
> jit_repipe_code_move().
>
> Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> ---
>  tools/perf/util/jitdump.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index c3f11d1c1d76d6c7..4f52b143cd0da296 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -683,6 +683,7 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
>                 build_id__mark_dso_hit(tool, event, &sample, jd->machine);
>  out:
>         perf_sample__exit(&sample);
> +       free(event);
>         return ret;
>  }
>
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 08/12] perf jitdump: Fix debug_data and unwinding_data leaks
  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
  1 sibling, 0 replies; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 19:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> jit_repipe_debug_info() overwrites jd->debug_data without freeing the
> previous allocation.  If two consecutive JIT_CODE_DEBUG_INFO records
> appear without an intervening LOAD record consuming the data, the first
> allocation leaks.
>
> The sibling jit_repipe_unwinding_info() already frees the old
> jd->unwinding_data before reassignment — add the same pattern to
> jit_repipe_debug_info() using zfree().
>
> Also add cleanup of both buffers in jit_close() so they are freed when
> the jitdump session ends, even if no LOAD record consumed them.
>
> Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> ---
>  tools/perf/util/jitdump.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 4f52b143cd0da296..3085091b95a517ae 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -118,6 +118,8 @@ jit_close(struct jit_buf_desc *jd)
>         funlockfile(jd->in);
>         fclose(jd->in);
>         jd->in = NULL;
> +       zfree(&jd->debug_data);
> +       zfree(&jd->unwinding_data);
>  }
>
>  static int
> @@ -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);
>         jd->debug_data       = data;
>
>         /*
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 09/12] perf jitdump: Use dirname() return value in jit_open()
  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
  1 sibling, 0 replies; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 19:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 5, 2026 at 6:32 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> jit_open() calls dirname(jd->dir) but ignores the return value.  POSIX
> says dirname() may return a pointer to internal static storage — glibc
> does this when the path has no '/', returning "." from a static buffer
> and leaving jd->dir unchanged with the original filename.
>
> Capture the return value and copy it back to jd->dir when dirname()
> returns a different pointer.
>
> Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/jitdump.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 3085091b95a517ae..02840dbf8a1fc16c 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -146,6 +146,7 @@ jit_open(struct jit_buf_desc *jd, const char *name)
>         ssize_t bs, bsz = 0;
>         void *n, *buf = NULL;
>         int ret, retval = -1;
> +       char *dname;
>
>         nsinfo__mountns_enter(jd->nsi, &nsc);
>         jd->in = fopen(name, "r");
> @@ -241,7 +242,9 @@ jit_open(struct jit_buf_desc *jd, const char *name)
>          */
>         strncpy(jd->dir, name, PATH_MAX - 1);
>         jd->dir[PATH_MAX - 1] = '\0';
> -       dirname(jd->dir);
> +       dname = dirname(jd->dir);
> +       if (dname != jd->dir)
> +               strlcpy(jd->dir, dname, PATH_MAX);

nit: prefer "sizeof(jd->dir)" over PATH_MAX.

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

>         free(buf);
>
>         return 0;
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 10/12] perf jitdump: Validate debug entries on native (non-swap) path
  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
  1 sibling, 0 replies; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 19:19 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 5, 2026 at 6:32 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> The bounds-checking and nr_entry clamping added for the byte-swap path
> only runs when jd->needs_bswap is true.  On native-endian files, nr_entry
> passes through unvalidated to jit_repipe_debug_info(), which stores it
> as jd->nr_debug_entries.  Downstream, jit_process_debug_info() in
> genelf_debug.c iterates nr_debug_entries times via debug_entry_next(),
> which calls strlen() on each entry's name field — a crafted nr_entry
> causes OOB reads and writes.
>
> Add bounds-checked iteration in jit_repipe_debug_info() that validates
> each debug_entry fits in the payload and its name is NUL-terminated
> before calling debug_entry_next().  Clamp nr_debug_entries to the count
> of valid entries.
>
> Fixes: 598b7c6919c7bbcc ("perf jit: add source line info support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> ---
>  tools/perf/util/jitdump.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 02840dbf8a1fc16c..87612ef3e232598e 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -694,8 +694,10 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
>
>  static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr)
>  {
> -       void *data;
> -       size_t sz;
> +       struct debug_entry *ent;
> +       void *data, *end;
> +       size_t sz, valid;
> +       uint64_t i;
>
>         if (!(jd && jr))
>                 return -1;
> @@ -715,10 +717,25 @@ static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr)
>         jd->debug_data       = data;
>
>         /*
> -        * we must use nr_entry instead of size here because
> -        * we cannot distinguish actual entry from padding otherwise
> +        * Clamp nr_debug_entries to entries that actually fit in the
> +        * payload.  The byte-swap path already does this for cross-endian
> +        * files; validate on the native path too, since downstream
> +        * jit_process_debug_info() iterates via debug_entry_next() which
> +        * calls strlen() on each entry's name field.
>          */
> -       jd->nr_debug_entries = jr->info.nr_entry;
> +       end = data + sz;
> +       ent = data;
> +       valid = 0;
> +       for (i = 0; i < jr->info.nr_entry; i++) {
> +               if ((void *)ent + sizeof(*ent) > end)
> +                       break;
> +               /* name must be NUL-terminated within the payload */
> +               if (!memchr(ent->name, '\0', (char *)end - ent->name))
> +                       break;
> +               ent = debug_entry_next(ent);
> +               valid++;
> +       }
> +       jd->nr_debug_entries = valid;
>
>         return 0;
>  }
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 11/12] perf jitdump: Validate sym string NUL-termination in code load
  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
  0 siblings, 0 replies; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 19:19 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 5, 2026 at 6:32 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> jit_repipe_code_load() computes sym = (void *)jr + sizeof(jr->load) and
> passes it to jit_emit_elf() which calls strlen(sym) via jit_write_elf().
> If code_size equals total_size - sizeof(jr->load), the sym pointer
> aliases the code blob with no NUL terminator, and strlen() scans past
> the buffer into adjacent heap memory.
>
> Add a memchr() check to verify the symbol name is NUL-terminated within
> the region between the load header and the code blob before use.
>
> Fixes: 598b7c6919c7bbcc ("perf jit: add source line info support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> ---
>  tools/perf/util/jitdump.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 87612ef3e232598e..5f3a53f818c29f58 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -487,6 +487,13 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
>
>         sym   = (void *)((unsigned long)jr + sizeof(jr->load));
>         code  = (unsigned long)jr + jr->load.p.total_size - csize;
> +
> +       /* sym string lives between the load header and the code blob */
> +       if (!memchr(sym, '\0', code - (unsigned long)sym)) {
> +               pr_warning("jitdump: unterminated symbol name in code_load record\n");
> +               return -1;
> +       }
> +
>         count = jr->load.code_index;
>         idr_size = jd->machine->id_hdr_size;
>
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 12/12] perf jitdump: Validate unwinding sizes against record payload
  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
  0 siblings, 0 replies; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 19:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stefano Sanfilippo, Stephane Eranian

On Wed, Aug 5, 2026 at 6:32 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> jit_repipe_unwinding_info() copies unwinding_size and eh_frame_hdr_size
> from the jitdump record into jd-> fields without checking them against
> the actual payload size.  Downstream, jit_add_eh_frame_info() in
> genelf.c computes unwinding_table_size = unwinding_size -
> eh_frame_hdr_size, which underflows when eh_frame_hdr_size >
> unwinding_size.  The result is passed as d->d_size to libelf, causing
> an OOB heap read into the output ELF file.
>
> Validate that unwinding_size fits within the record payload and that
> eh_frame_hdr_size does not exceed unwinding_size before allocating or
> storing the values, so a bogus record cannot force a large allocation
> that is then discarded.
>
> mapped_size is likewise taken from the record and was narrowed into an
> int for the mmap2 len computation in jit_repipe_code_load() and
> jit_repipe_code_move(); values above INT_MAX would turn negative,
> producing a wrong mmap2 length.  Use uint64_t for usize so the value
> cannot truncate.
>
> Fixes: 0284fecd13b6db3e ("perf jit: Add unwinding support")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stefano Sanfilippo <ssanfilippo@chromium.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: Claude:claude-opus-4.6
> Assisted-by: Opencode:mimo-v2.5-free
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> ---
>  tools/perf/util/jitdump.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 5f3a53f818c29f58..385c19b864aeee4c 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;
> @@ -543,7 +544,7 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
>
>         event->mmap2.pgoff = GEN_ELF_TEXT_OFFSET;
>         event->mmap2.start = addr;
> -       event->mmap2.len   = usize ? ALIGN_8(csize) + usize : csize;
> +       event->mmap2.len   = usize ? ALIGN_8((uint64_t)csize) + usize : (uint64_t)csize;
>         event->mmap2.pid   = pid;
>         event->mmap2.tid   = tid;
>         event->mmap2.ino   = st.st_ino;
> @@ -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;
> @@ -761,6 +762,18 @@ jit_repipe_unwinding_info(struct jit_buf_desc *jd, union jr_entry *jr)
>                 return -1;
>
>         unwinding_data_size  = jr->prefix.total_size - sizeof(jr->unwinding);
> +
> +       /*
> +        * Validate sizes before allocating — jit_add_eh_frame_info()
> +        * computes unwinding_size - eh_frame_hdr_size and uses the
> +        * result as a buffer length for libelf.
> +        */
> +       if (jr->unwinding.unwinding_size > unwinding_data_size ||
> +           jr->unwinding.eh_frame_hdr_size > jr->unwinding.unwinding_size) {
> +               pr_warning("jitdump: invalid unwinding sizes in unwinding_info record\n");
> +               return -1;
> +       }
> +
>         unwinding_data = malloc(unwinding_data_size);
>         if (!unwinding_data)
>                 return -1;
> --
> 2.55.0
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 05/12] perf jitdump: Check snprintf return before computing header size
  2026-08-05 19:07   ` Ian Rogers
@ 2026-08-05 19:45     ` Arnaldo Carvalho de Melo
  2026-08-05 21:29       ` Ian Rogers
  0 siblings, 1 reply; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 19:45 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 05, 2026 at 12:07:23PM -0700, Ian Rogers wrote:
> On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > snprintf() returns the would-have-been length on truncation.  When the
> > jitted filename exceeds PATH_MAX, the unclamped 'size' value inflates
> > sizeof(event->mmap2.filename) - size into a massive underflow, causing
> > the header.size computation to write an oversized header.  The
> > subsequent write to 'id = event + header.size - idr_size' then corrupts
> > the heap.
> >
> > Clamp size to PATH_MAX - 1 after snprintf in both jit_repipe_code_load()
> > and jit_repipe_code_move().
> >
> > Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> > Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> > Cc: Stephane Eranian <eranian@google.com>
> > Assisted-by: Claude:claude-opus-4.6
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/util/jitdump.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> > index 078d3304d2b7ebce..fd11e07bf00b7978 100644
> > --- a/tools/perf/util/jitdump.c
> > +++ b/tools/perf/util/jitdump.c
> > @@ -493,6 +493,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 >= PATH_MAX)
> > +               size = PATH_MAX - 1;
> 
> Given the recent fixes to reading /proc/pid/maps where it was assumed
> the file paths would be limited to PATH_MAX and it turns out that
> PATH_MAX doesn't really do that and we had potential buffer overruns
> during synthesis, I wonder it would be more intention revealing here
> to use "sizeof(event->mmap2.filename)" rather than PATH_MAX. Other
> than the constant used, I agree with the change and using PATH_MAX
> isn't wrong.

We need to go on having our tools/perf/AGENTS.md with all those rules
:-)
 
> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!

- Arnaldo
 
> Thanks,
> Ian
> 
> >
> >         size++; /* for \0 */
> >
> > @@ -623,6 +626,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 >= PATH_MAX)
> > +               size = PATH_MAX - 1;
> >
> >         size++; /* for \0 */
> >
> > --
> > 2.55.0
> >

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 01/12] perf jitdump: Fix extended header read that always fails
  2026-08-05 21:26 [PATCHES v2 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
@ 2026-08-05 21:26 ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-05 21:26 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

jit_open() sets bsz = bs before the fread() that uses bs - bsz as the
read size, making the expression always evaluate to zero.  fread() with
size 0 returns 0, which triggers the ret != 1 error path — so extended
jitdump headers (total_size > sizeof(header)) have been silently broken
since the original implementation.

Additionally, when 0 < bs <= bsz the if (bs > bsz) block is skipped
entirely, leaving extended header bytes unread in the stream.  Subsequent
jit_get_next_entry() calls then parse those leftover bytes as a
jr_prefix, corrupting the record stream.

Fix by separating the buffer growth from the read: realloc only when
bs > bsz, then unconditionally fread bs bytes when bs > 0.

Fixes: 9b07e27f88b9cd78 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude:claude-opus-4.6
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 83005b30b9bf3fd7..4b7c7ba7cd95ddbb 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -224,10 +224,12 @@ jit_open(struct jit_buf_desc *jd, const char *name)
 		n = realloc(buf, bs);
 		if (!n)
 			goto error;
-		bsz = bs;
 		buf = n;
-		/* read extra we do not know about */
-		ret = fread(buf, bs - bsz, 1, jd->in);
+		bsz = bs;
+	}
+	if (bs > 0) {
+		/* consume extended header bytes from the stream */
+		ret = fread(buf, bs, 1, jd->in);
 		if (ret != 1)
 			goto error;
 	}
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH 05/12] perf jitdump: Check snprintf return before computing header size
  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
  0 siblings, 1 reply; 41+ messages in thread
From: Ian Rogers @ 2026-08-05 21:29 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 5, 2026 at 12:45 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Wed, Aug 05, 2026 at 12:07:23PM -0700, Ian Rogers wrote:
> > On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > >
> > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > >
> > > snprintf() returns the would-have-been length on truncation.  When the
> > > jitted filename exceeds PATH_MAX, the unclamped 'size' value inflates
> > > sizeof(event->mmap2.filename) - size into a massive underflow, causing
> > > the header.size computation to write an oversized header.  The
> > > subsequent write to 'id = event + header.size - idr_size' then corrupts
> > > the heap.
> > >
> > > Clamp size to PATH_MAX - 1 after snprintf in both jit_repipe_code_load()
> > > and jit_repipe_code_move().
> > >
> > > Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> > > Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> > > Cc: Stephane Eranian <eranian@google.com>
> > > Assisted-by: Claude:claude-opus-4.6
> > > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > ---
> > >  tools/perf/util/jitdump.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> > > index 078d3304d2b7ebce..fd11e07bf00b7978 100644
> > > --- a/tools/perf/util/jitdump.c
> > > +++ b/tools/perf/util/jitdump.c
> > > @@ -493,6 +493,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 >= PATH_MAX)
> > > +               size = PATH_MAX - 1;
> >
> > Given the recent fixes to reading /proc/pid/maps where it was assumed
> > the file paths would be limited to PATH_MAX and it turns out that
> > PATH_MAX doesn't really do that and we had potential buffer overruns
> > during synthesis, I wonder it would be more intention revealing here
> > to use "sizeof(event->mmap2.filename)" rather than PATH_MAX. Other
> > than the constant used, I agree with the change and using PATH_MAX
> > isn't wrong.
>
> We need to go on having our tools/perf/AGENTS.md with all those rules
> :-)

Yeah, I don't know of a plan for AGENTS.md files but there is always:
https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/perf.md

Perhaps we can add something like:
```
## PATH_MAX avoidance

The PATH_MAX constant isn't a limit to paths in places like
/proc/pid/maps, and reading paths from these locations can lead to
buffer overruns for buffers sized at PATH_MAX - as such prefer dynamic
memory allocation for paths. When referring to the size of PATH_MAX
sized char arrays in kernel system call and ring buffer data
structures, prefer to use sizeof the variable rather than the PATH_MAX
constant to make it clear the size and possible truncation is being
enforced by the kernel which may substitute long paths with the value
"//toolong".
```

Thanks,
Ian

> > Reviewed-by: Ian Rogers <irogers@google.com>
>
> Thanks!
>
> - Arnaldo
>
> > Thanks,
> > Ian
> >
> > >
> > >         size++; /* for \0 */
> > >
> > > @@ -623,6 +626,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 >= PATH_MAX)
> > > +               size = PATH_MAX - 1;
> > >
> > >         size++; /* for \0 */
> > >
> > > --
> > > 2.55.0
> > >

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 01/12] perf jitdump: Fix extended header read that always fails
  2026-08-06 12:35 [PATCHES v3 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
@ 2026-08-06 12:35 ` Arnaldo Carvalho de Melo
  2026-08-06 13:04   ` sashiko-bot
  0 siblings, 1 reply; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-06 12:35 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

jit_open() sets bsz = bs before the fread() that uses bs - bsz as the
read size, making the expression always evaluate to zero.  fread() with
size 0 returns 0, which triggers the ret != 1 error path — so extended
jitdump headers (total_size > sizeof(header)) have been silently broken
since the original implementation.

Additionally, when 0 < bs <= bsz the if (bs > bsz) block is skipped
entirely, leaving extended header bytes unread in the stream.  Subsequent
jit_get_next_entry() calls then parse those leftover bytes as a
jr_prefix, corrupting the record stream.

Fix by separating the buffer growth from the read: realloc only when
bs > bsz, then unconditionally fread bs bytes when bs > 0.

Fixes: 9b07e27f88b9cd78 ("perf inject: Add jitdump mmap injection support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude:claude-opus-4.6
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 83005b30b9bf3fd7..4b7c7ba7cd95ddbb 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -224,10 +224,12 @@ jit_open(struct jit_buf_desc *jd, const char *name)
 		n = realloc(buf, bs);
 		if (!n)
 			goto error;
-		bsz = bs;
 		buf = n;
-		/* read extra we do not know about */
-		ret = fread(buf, bs - bsz, 1, jd->in);
+		bsz = bs;
+	}
+	if (bs > 0) {
+		/* consume extended header bytes from the stream */
+		ret = fread(buf, bs, 1, jd->in);
 		if (ret != 1)
 			goto error;
 	}
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/12] perf jitdump: Fix extended header read that always fails
  2026-08-06 12:35 ` [PATCH 01/12] perf jitdump: Fix extended header read that always fails Arnaldo Carvalho de Melo
@ 2026-08-06 13:04   ` sashiko-bot
  0 siblings, 0 replies; 41+ messages in thread
From: sashiko-bot @ 2026-08-06 13:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users

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

Pre-existing issues:
- [High] Missing bounds checks for JIT records allow out-of-bounds memory accesses.
- [High] The error path for a failed malloc() calls funlockfile() on an un-locked FILE pointer.
--

--- Patch [1]: [PATCH 01/12] perf jitdump: Fix extended header read that always fails ---
commit dc8d1e17077d95aa9e9434ca752e0c0e9bb0e0de
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Fix extended header read that always fails

This commit fixes an issue in jit_open() where extended header bytes were
not being properly read. It separates the buffer reallocation from the
stream read, ensuring the extended header bytes are correctly consumed.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 83005b30b9bf3..4b7c7ba7cd95d 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential missing bounds check
in this file while reviewing.

In jit_repipe_code_load(), csize and sym are extracted without bounds checks
against total_size:

tools/perf/util/jitdump.c:jit_repipe_code_load() {
    ...
    csize = jr->load.code_size;
    ...
    sym   = (void *)((unsigned long)jr + sizeof(jr->load));
    code  = (unsigned long)jr + jr->load.p.total_size - csize;
    ...
}

Could this allow code to point out-of-bounds if total_size is not validated
against the record size?

There seems to be a similar issue in jit_repipe_unwinding_info():

tools/perf/util/jitdump.c:jit_repipe_unwinding_info() {
    ...
    unwinding_data_size  = jr->prefix.total_size - sizeof(jr->unwinding);
    unwinding_data = malloc(unwinding_data_size);
    ...
    memcpy(unwinding_data, &jr->unwinding.unwinding_data,
           unwinding_data_size);
    ...
}

If total_size is smaller than sizeof(jr->unwinding), could this underflow
unwinding_data_size and lead to a large allocation and an out-of-bounds
memcpy?

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 05/12] perf jitdump: Check snprintf return before computing header size
  2026-08-05 21:29       ` Ian Rogers
@ 2026-08-07 12:00         ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-07 12:00 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Namhyung Kim, Ingo Molnar, Thomas Gleixner, James Clark,
	Jiri Olsa, Adrian Hunter, Clark Williams, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot,
	Stephane Eranian

On Wed, Aug 05, 2026 at 02:29:47PM -0700, Ian Rogers wrote:
> On Wed, Aug 5, 2026 at 12:45 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > On Wed, Aug 05, 2026 at 12:07:23PM -0700, Ian Rogers wrote:
> > > On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > +++ b/tools/perf/util/jitdump.c
> > > > @@ -493,6 +493,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 >= PATH_MAX)
> > > > +               size = PATH_MAX - 1;

> > > Given the recent fixes to reading /proc/pid/maps where it was assumed
> > > the file paths would be limited to PATH_MAX and it turns out that
> > > PATH_MAX doesn't really do that and we had potential buffer overruns
> > > during synthesis, I wonder it would be more intention revealing here
> > > to use "sizeof(event->mmap2.filename)" rather than PATH_MAX. Other
> > > than the constant used, I agree with the change and using PATH_MAX
> > > isn't wrong.

> > We need to go on having our tools/perf/AGENTS.md with all those rules
> > :-)
 
> Yeah, I don't know of a plan for AGENTS.md files but there is always:
> https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/perf.md
 
> Perhaps we can add something like:
> ```
> ## PATH_MAX avoidance
> 
> The PATH_MAX constant isn't a limit to paths in places like
> /proc/pid/maps, and reading paths from these locations can lead to
> buffer overruns for buffers sized at PATH_MAX - as such prefer dynamic
> memory allocation for paths. When referring to the size of PATH_MAX
> sized char arrays in kernel system call and ring buffer data
> structures, prefer to use sizeof the variable rather than the PATH_MAX
> constant to make it clear the size and possible truncation is being
> enforced by the kernel which may substitute long paths with the value
> "//toolong".
> ```

Looks sensible, I encourage you to submit a patch adding that:

Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2026-08-07 12:01 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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 01/12] perf jitdump: Fix extended header read that always fails Arnaldo Carvalho de Melo
2026-08-06 12:35 [PATCHES v3 0/12] perf jitdump: Input validation hardening Arnaldo Carvalho de Melo
2026-08-06 12:35 ` [PATCH 01/12] perf jitdump: Fix extended header read that always fails Arnaldo Carvalho de Melo
2026-08-06 13:04   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox