[PATCH v1 0/5] perf tools: Fix jitdump and dso handling

[PATCH v1 0/5] perf tools: Fix jitdump and dso handling

[Arnaldo Carvalho de Melo]

Hi,

This series addresses five small fixes in the perf jitdump and dso
code that were found by sashiko-bot during automated review.

Patches 1 and 2 - unaligned-safe debug entries:

  - 1/5 perf jitdump: Byte-swap debug entries via unaligned-safe accessors
    The byte-swap loop in jit_get_next_entry() did 64-bit loads/stores
    through struct member access.  This seems to be UB when entries are
    unaligned after the first variable-length name[].  Use
    get_unaligned()/put_unaligned() for each field, as was done in the
    earlier bounds-check hardening.

  - 2/5 perf genelf: Use unaligned-safe accessors for debug entries
    The same packing issue on the native path.  As far as I can tell,
    jit_process_debug_info(), get_special_opcode() and
    emit_lineno_info() all read u64 addr and int lineno through struct
    access.  Convert them to unaligned-safe accessors, matching the
    layout the jitdump writers (LLVM, JVM agents) emit.

Patch 3 - stale unwinding state:

  - 3/5 perf jitdump: Free unwinding data even when eh_frame_hdr_size is zero
    jit_repipe_code_load() only cleared jd->unwinding_data when both
    unwinding_data and eh_frame_hdr_size were set.  If a record carries
    unwinding_data with eh_frame_hdr_size==0, so the answer would be that
    the check fails and the state is applied to all subsequent records.
    The record is validated upstream so eh_frame_hdr_size <= unwinding_size
    always holds.  Free based on the data pointer alone.

Patch 4 - event sizing:

  - 4/5 perf jitdump: Size code_move event allocation with idr_size
    jit_repipe_code_move() allocated event as sizeof(*event)+16 but
    computed header.size with +idr_size.  When idr_size>16, I believe
    header.size exceeds the allocation and perf_data__write() reads past
    the heap, leaking adjacent heap into perf.data.  Size with idr_size
    like jit_repipe_code_load() does.

Patch 5 - open list deadlock/race:

  - 5/5 perf dso: Defer dropping the open list reference until after the lock
    The reference taken by dso__list_add() cannot be dropped while
    holding dso__data_open_lock: dso__put() may call dso__data_close()
    which takes the same lock, deadlocking.  This seems to be the cause
    of the inconsistent list/counter state under REFCNT_CHECKING.  Fix by
    transferring the reference to a deferred node drained by
    dso__put_deferred() after every unlock.  Since the counter is now
    decremented under the lock, do_open()'s close_first_dso() no longer
    races with a stale count.

Regards,

- Arnaldo

Arnaldo Carvalho de Melo (5):
  perf jitdump: Byte-swap debug entries via unaligned-safe accessors
  perf genelf: Use unaligned-safe accessors for debug entries
  perf jitdump: Free unwinding data even when eh_frame_hdr_size is zero
  perf jitdump: Size code_move event allocation with idr_size
  perf dso: Defer dropping the open list reference until after the lock

 tools/perf/util/dso.c          | 75 ++++++++++++++++++++++++++++++++--
 tools/perf/util/genelf_debug.c | 30 ++++++++------
 tools/perf/util/jitdump.c      | 20 ++++++---
 3 files changed, 103 insertions(+), 22 deletions(-)

-- 
2.55.0

[PATCH 1/5] perf jitdump: Byte-swap debug entries via unaligned-safe accessors

[Arnaldo Carvalho de Melo]

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

debug_entry records are packed with a variable-length name[] field, so
entries after the first may start at addresses that are not naturally
aligned for their u64 addr and int lineno/discrim fields.  On strict
alignment architectures the byte-swap loop in jit_get_next_entry()
performed misaligned 64-bit loads and stores through struct member
access, which is undefined behavior.

Use get_unaligned()/put_unaligned() for the byte-swap of each field.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index d25a9fe9b020ce87..e0d5cc9a828189a2 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -14,6 +14,8 @@
 #include <sys/stat.h>
 #include <sys/mman.h>
 #include <linux/stringify.h>
+#include <linux/kernel.h>
+#include <linux/unaligned.h>
 
 #include "event.h"
 #include "debug.h"
@@ -343,9 +345,14 @@ jit_get_next_entry(struct jit_buf_desc *jd)
 				/* 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);
+				/*
+				 * debug entries are packed with a variable-length
+				 * name[], so entries after the first may be
+				 * unaligned: byte-swap via unaligned-safe accessors.
+				 */
+				put_unaligned(bswap_64(get_unaligned(&ent->addr)), &ent->addr);
+				put_unaligned(bswap_32(get_unaligned(&ent->lineno)), &ent->lineno);
+				put_unaligned(bswap_32(get_unaligned(&ent->discrim)), &ent->discrim);
 				ent = debug_entry_next(ent);
 			}
 			/* clamp so downstream consumers don't overrun */
-- 
2.55.0

[PATCH 2/5] perf genelf: Use unaligned-safe accessors for debug entries

[Arnaldo Carvalho de Melo]

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

debug_entry records are packed with a variable-length name[] field, so
entries after the first may start at addresses that are not naturally
aligned.  jit_process_debug_info(), get_special_opcode() and
emit_lineno_info() read and write the u64 addr and int lineno fields
through struct member access, which is undefined behavior on
strict-alignment architectures.

Use get_unaligned()/put_unaligned() to read and update each field,
matching the layout the jitdump writers (LLVM, JVM agents) emit, which
packs entries without padding.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/genelf_debug.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/genelf_debug.c b/tools/perf/util/genelf_debug.c
index 8588b3e35e008396..8244d3c64103b6be 100644
--- a/tools/perf/util/genelf_debug.c
+++ b/tools/perf/util/genelf_debug.c
@@ -12,6 +12,8 @@
  */
 #include <linux/compiler.h>
 #include <linux/zalloc.h>
+#include <linux/kernel.h>
+#include <linux/unaligned.h>
 #include <sys/types.h>
 #include <stdio.h>
 #include <getopt.h>
@@ -303,11 +305,13 @@ static ubyte get_special_opcode(struct debug_entry *ent,
 {
 	unsigned int temp;
 	unsigned long delta_addr;
+	unsigned int lineno = get_unaligned(&ent->lineno);
+	uint64_t addr = get_unaligned(&ent->addr);
 
 	/*
 	 * delta from line_base
 	 */
-	temp = (ent->lineno - last_line) - default_debug_line_header.line_base;
+	temp = (lineno - last_line) - default_debug_line_header.line_base;
 
 	if (temp >= default_debug_line_header.line_range)
 		return 0;
@@ -315,7 +319,7 @@ static ubyte get_special_opcode(struct debug_entry *ent,
 	/*
 	 * delta of addresses
 	 */
-	delta_addr = (ent->addr - last_vma) / default_debug_line_header.minimum_instruction_length;
+	delta_addr = (addr - last_vma) / default_debug_line_header.minimum_instruction_length;
 
 	/* This is not sufficient to ensure opcode will be in [0-256] but
 	 * sufficient to ensure when summing with the delta lineno we will
@@ -355,13 +359,15 @@ static void emit_lineno_info(struct buffer_ext *be,
 	unsigned long last_vma = 0;
 	char const  *cur_filename = NULL;
 	unsigned long cur_file_idx = 0;
-	int last_line = 1;
+	unsigned int last_line = 1;
 
 	emit_lne_set_address(be, (void *)code_addr);
 
 	for (i = 0; i < nr_entry; i++, ent = debug_entry_next(ent)) {
 		int need_copy = 0;
 		ubyte special_opcode;
+		unsigned int lineno = get_unaligned(&ent->lineno);
+		uint64_t addr = get_unaligned(&ent->addr);
 
 		/*
 		 * check if filename changed, if so add it
@@ -376,24 +382,24 @@ static void emit_lineno_info(struct buffer_ext *be,
 
 		special_opcode = get_special_opcode(ent, last_line, last_vma);
 		if (special_opcode != 0) {
-			last_line = ent->lineno;
-			last_vma  = ent->addr;
+			last_line = lineno;
+			last_vma  = addr;
 			emit_opcode(be, special_opcode);
 		} else {
 			/*
 			 * lines differ, emit line delta
 			 */
-			if (last_line != ent->lineno) {
-				emit_advance_lineno(be, ent->lineno - last_line);
-				last_line = ent->lineno;
+			if (last_line != lineno) {
+				emit_advance_lineno(be, lineno - last_line);
+				last_line = lineno;
 				need_copy = 1;
 			}
 			/*
 			 * addresses differ, emit address delta
 			 */
-			if (last_vma != ent->addr) {
-				emit_advance_pc(be, ent->addr - last_vma);
-				last_vma = ent->addr;
+			if (last_vma != addr) {
+				emit_advance_pc(be, addr - last_vma);
+				last_vma = addr;
 				need_copy = 1;
 			}
 			/*
@@ -480,7 +486,7 @@ jit_process_debug_info(uint64_t code_addr,
 	int i;
 
 	for (i = 0; i < nr_debug_entries; i++) {
-		ent->addr = ent->addr - code_addr;
+		put_unaligned(get_unaligned(&ent->addr) - code_addr, &ent->addr);
 		ent = debug_entry_next(ent);
 	}
 	add_compilation_unit(di, buffer_ext_size(dl));
-- 
2.55.0

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

[Arnaldo Carvalho de Melo]

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

jit_repipe_code_load() only cleared the unwinding state when both
unwinding_data and eh_frame_hdr_size were set.  When a record carries
unwinding data but eh_frame_hdr_size is 0, the cleanup condition fails
and the unwinding state persists in jd, being applied to all subsequent
JIT_CODE_LOAD and JIT_CODE_MOVE records, duplicating unwinding sections
in the generated ELF files and inflating their event->mmap2.len.

The record is validated upstream so eh_frame_hdr_size <= unwinding_size
always holds.  Free the unwinding data based on the data pointer alone.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

[PATCH 4/5] perf jitdump: Size code_move event allocation with idr_size

[Arnaldo Carvalho de Melo]

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

jit_repipe_code_move() allocated the mmap2 event with a hardcoded
+16, but computes event->mmap2.header.size as sizeof(event->mmap2)
minus unused filename bytes plus idr_size.  When idr_size is larger
than 16, header.size exceeds the allocation, so perf_data__write()
reads past the heap allocation, leaking adjacent heap memory into the
generated perf.data file.

Size the allocation with idr_size like jit_repipe_code_load() does.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: LLM
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 efb40d93e33ae664..689aa0a8c60bcd8b 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -636,9 +636,10 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
 	idr_size = jd->machine->id_hdr_size;
 
 	/*
-	 * +16 to account for sample_id_all (hack)
+	 * Sample ID is written past the end of the mmap2 record; size
+	 * the allocation to account for it instead of a hardcoded +16.
 	 */
-	event = calloc(1, sizeof(*event) + 16);
+	event = calloc(1, sizeof(*event) + idr_size);
 	if (!event)
 		return -1;
 
-- 
2.55.0

[PATCH 5/5] perf dso: Defer dropping the open list reference until after the lock

[Arnaldo Carvalho de Melo]

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

There was a problem in the code with some resources potentially being
left unbalanced, and the logic on dso__data_close() becoming confused
if the fd had been closed already.

The reference taken by dso__list_add() on the open list cannot be
dropped while holding the open lock: dso__put() may call
dso__data_close(), which takes dso__data_open_lock() itself,
deadlocking and leaving the list and its counter inconsistent for
concurrent threads.

Fix it by changing dso__list_del() to transfer the reference to a
deferred node, drained by dso__put_deferred() right after every
unlock of dso__data_open_lock().  Since the counter is now decremented
under the open lock, do_open()'s close_first_dso() no longer races
with a stale count.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c | 75 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 71 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 42bfe30a3b518e80..a4b2361bc7420084 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -531,18 +531,79 @@ static void dso__list_add(struct dso *dso) EXCLUSIVE_LOCKS_REQUIRED(_dso__data_o
 	dso__data_open_cnt++;
 }
 
+#ifdef REFCNT_CHECKING
+/*
+ * A deferred put: carries the reference taken by dso__list_add() for an
+ * entry removed from dso__data_open.  Dedicated nodes are used so that
+ * the dso_data's own open_entry node can be relinked by a concurrent
+ * dso__list_add() without corrupting this list or its reference.
+ */
+struct dso_data_put {
+	struct list_head entry;
+	struct dso *dso;
+};
+static LIST_HEAD(dso__data_open_put);
+#endif
+
 static void dso__list_del(struct dso *dso) EXCLUSIVE_LOCKS_REQUIRED(_dso__data_open_lock)
 {
-	list_del_init(&dso__data(dso)->open_entry);
 #ifdef REFCNT_CHECKING
-	mutex_unlock(dso__data_open_lock());
-	dso__put(dso__data(dso)->dso);
-	mutex_lock(dso__data_open_lock());
+	struct dso_data_put *put;
 #endif
+
+	list_del_init(&dso__data(dso)->open_entry);
 	WARN_ONCE(dso__data_open_cnt <= 0,
 		  "DSO data fd counter out of bounds.");
 	dso__data_open_cnt--;
+#ifdef REFCNT_CHECKING
+	/*
+	 * The reference taken in dso__list_add() cannot be dropped while
+	 * holding the open lock: dso__put() may call dso__data_close(),
+	 * which takes dso__data_open_lock itself, deadlocking and leaving
+	 * the list/counter state inconsistent for concurrent threads.
+	 * Transfer the reference to a deferred node drained by
+	 * dso__put_deferred() once the lock is released.
+	 */
+	put = zalloc(sizeof(*put));
+
+	if (put == NULL)
+		return;
+
+	put->dso = dso__data(dso)->dso;
+	dso__data(dso)->dso = NULL;
+	list_add_tail(&put->entry, &dso__data_open_put);
+#endif
+}
+
+#ifdef REFCNT_CHECKING
+/*
+ * Drop the references deferred by dso__list_del().  Must be called
+ * without holding dso__data_open_lock: dso__put() may re-enter it via
+ * dso__data_close().
+ */
+static void dso__put_deferred(void) LOCKS_EXCLUDED(_dso__data_open_lock)
+{
+	for (;;) {
+		struct dso_data_put *put;
+		struct dso *dso;
+
+		mutex_lock(dso__data_open_lock());
+		put = list_first_entry_or_null(&dso__data_open_put, struct dso_data_put, entry);
+		if (put == NULL) {
+			mutex_unlock(dso__data_open_lock());
+			return;
+		}
+		list_del_init(&put->entry);
+		dso = put->dso;
+		mutex_unlock(dso__data_open_lock());
+
+		free(put);
+		dso__put(dso);
+	}
 }
+#else
+static void dso__put_deferred(void) {}
+#endif
 
 static void close_first_dso(void);
 
@@ -805,6 +866,7 @@ void dso__data_close(struct dso *dso)
 	mutex_lock(dso__data_open_lock());
 	close_dso(dso);
 	mutex_unlock(dso__data_open_lock());
+	dso__put_deferred();
 }
 
 static void try_to_open_dso(struct dso *dso, struct machine *machine)
@@ -865,12 +927,14 @@ bool dso__data_get_fd(struct dso *dso, struct machine *machine, int *fd)
 		return true;
 
 	mutex_unlock(dso__data_open_lock());
+	dso__put_deferred();
 	return false;
 }
 
 void dso__data_put_fd(struct dso *dso __maybe_unused)
 {
 	mutex_unlock(dso__data_open_lock());
+	dso__put_deferred();
 }
 
 bool dso__data_status_seen(struct dso *dso, enum dso_data_status_seen by)
@@ -1058,6 +1122,7 @@ static ssize_t file_read(struct dso *dso, struct machine *machine,
 	ret = pread(dso__data(dso)->fd, data, DSO__DATA_CACHE_SIZE, offset);
 out:
 	mutex_unlock(dso__data_open_lock());
+	dso__put_deferred();
 	return ret;
 }
 
@@ -1188,6 +1253,7 @@ static int file_size(struct dso *dso, struct machine *machine)
 
 out:
 	mutex_unlock(dso__data_open_lock());
+	dso__put_deferred();
 	return ret;
 }
 
@@ -1405,6 +1471,7 @@ uint16_t dso__e_machine_endian(struct dso *dso, struct machine *machine, uint32_
 		*e_flags = 0;
 
 	mutex_unlock(dso__data_open_lock());
+	dso__put_deferred();
 	return e_machine;
 }
 
-- 
2.55.0

Re: [PATCH 1/5] perf jitdump: Byte-swap debug entries via unaligned-safe accessors

[sashiko-bot]

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> debug_entry records are packed with a variable-length name[] field, so
> entries after the first may start at addresses that are not naturally
> aligned for their u64 addr and int lineno/discrim fields.  On strict
> alignment architectures the byte-swap loop in jit_get_next_entry()
> performed misaligned 64-bit loads and stores through struct member
> access, which is undefined behavior.
> 
> Use get_unaligned()/put_unaligned() for the byte-swap of each field.
> 
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: LLM
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [PATCH 2/5] perf genelf: Use unaligned-safe accessors for debug entries

[sashiko-bot]

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

New issues:
- [Medium] Changing `last_line` and `lineno` to `unsigned int` causes negative line number advances to be incorrectly zero-extended to large positive values on 64-bit architectures, corrupting the DWARF line number program.

Pre-existing issues:
- [High] Unchecked realloc failure in buffer_ext_add() leads to exploitable out-of-bounds heap writes.
--

commit 4a5e45a87437dd59a172031050c168bbb7f8ce78
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf genelf: Use unaligned-safe accessors for debug entries

This patch modifies the perf genelf utility to use unaligned-safe accessors
for debug entry address and line number fields. This prevents undefined
behavior on strict-alignment architectures, as debug_entry records pack
a variable-length string field, leaving subsequent entries unaligned.

> diff --git a/tools/perf/util/genelf_debug.c b/tools/perf/util/genelf_debug.c
> index 8588b3e35e008..8244d3c64103b 100644
> --- a/tools/perf/util/genelf_debug.c
> +++ b/tools/perf/util/genelf_debug.c
> @@ -355,13 +359,15 @@ static void emit_lineno_info(struct buffer_ext *be,
>  	unsigned long last_vma = 0;
>  	char const  *cur_filename = NULL;
>  	unsigned long cur_file_idx = 0;
> -	int last_line = 1;
> +	unsigned int last_line = 1;
>  
>  	emit_lne_set_address(be, (void *)code_addr);
>  
>  	for (i = 0; i < nr_entry; i++, ent = debug_entry_next(ent)) {
>  		int need_copy = 0;
>  		ubyte special_opcode;
> +		unsigned int lineno = get_unaligned(&ent->lineno);
> +		uint64_t addr = get_unaligned(&ent->addr);
>  
>  		/*
>  		 * check if filename changed, if so add it

[ ... ]

> @@ -376,24 +382,24 @@ static void emit_lineno_info(struct buffer_ext *be,
>  
>  		special_opcode = get_special_opcode(ent, last_line, last_vma);
>  		if (special_opcode != 0) {
> -			last_line = ent->lineno;
> -			last_vma  = ent->addr;
> +			last_line = lineno;
> +			last_vma  = addr;
>  			emit_opcode(be, special_opcode);
>  		} else {
>  			/*
>  			 * lines differ, emit line delta
>  			 */
> -			if (last_line != ent->lineno) {
> -				emit_advance_lineno(be, ent->lineno - last_line);
> -				last_line = ent->lineno;
> +			if (last_line != lineno) {
> +				emit_advance_lineno(be, lineno - last_line);
> +				last_line = lineno;

[Severity: Medium]
Will this zero-extend instead of sign-extend on backward line jumps?

Because lineno and last_line were changed from int to unsigned int,
their subtraction will wrap around to a large 32-bit unsigned value (e.g.,
0xFFFFFFEC for -20).

When this large unsigned value is passed to emit_advance_lineno, which
expects a 64-bit signed long, it implicitly zero-extends on 64-bit
architectures instead of sign-extending. This results in encoding a massive
forward line jump rather than a backward jump, potentially corrupting DWARF
line number mapping.


I also noticed a potential out-of-bounds write during buffer operations here:

tools/perf/util/genelf_debug.c:add_debug_line() {
    ...
	buffer_ext_add(be, (void *)&default_debug_line_header,
		 sizeof(default_debug_line_header));

	buffer_ext_add(be, &standard_opcode_length,  sizeof(standard_opcode_length));

	// empty directory entry
	buffer_ext_add(be, (void *)"", 1);

	// empty filename directory
	buffer_ext_add(be, (void *)"", 1);

	dbg_header = buffer_ext_addr(be) + old_size;
	dbg_header->prolog_length = (buffer_ext_size(be) - old_size) -
		offsetof(struct debug_line_header, minimum_instruction_length);
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but does this code handle
buffer_ext_add() allocation failures correctly?

If realloc fails inside buffer_ext_add(), it returns -1 without expanding
the buffer. The caller ignores this return value and proceeds to dereference
the computed pointer dbg_header.

If the buffer was completely full when realloc failed, this points exactly
to the end of the heap chunk, leading to unconditionally writing beyond the
allocated bounds.

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

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

[sashiko-bot]

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

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

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

perf jitdump: Free unwinding data even when eh_frame_hdr_size is zero

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

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

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

tools/perf/util/jitdump.c:jit_repipe_code_load() {
    ...
	if (jr->load.p.total_size < sizeof(jr->load) ||
	    jr->load.code_size > jr->load.p.total_size - sizeof(jr->load) ||
	    jr->load.code_size > INT_MAX) {
		pr_warning("jitdump: invalid code_size %" PRIu64 " (total_size=%u) in code_load record\n",
			   (uint64_t)jr->load.code_size, jr->load.p.total_size);
		return -1;
	}
    ...
}

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

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

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

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

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

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

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

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

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

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

Re: [PATCH 4/5] perf jitdump: Size code_move event allocation with idr_size

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Replacing the hardcoded +16 padding with +idr_size introduces a heap buffer overflow in jit_repipe_code_move() when PERF_SAMPLE_TID is missing but PERF_SAMPLE_TIME is present, and the filename length approaches PATH_MAX. This shares the same root cause as a pre-existing vulnerability in jit_repipe_code_load().
--

commit 631d6cebecdf5de246fc9c8ce57d27948626c5ac
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: perf jitdump: Size code_move event allocation with idr_size

This commit alters the event buffer allocation in jit_repipe_code_move()
to dynamically scale with idr_size rather than using a static padding.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c

[ ... ]

> @@ -636,9 +636,10 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
>  	idr_size = jd->machine->id_hdr_size;
>  
>  	/*
> -	 * +16 to account for sample_id_all (hack)
> +	 * Sample ID is written past the end of the mmap2 record; size
> +	 * the allocation to account for it instead of a hardcoded +16.
>  	 */
> -	event = calloc(1, sizeof(*event) + 16);
> +	event = calloc(1, sizeof(*event) + idr_size);

[Severity: Critical]
Does this regression introduce a heap buffer overflow when setting the event
time?

Looking later in jit_repipe_code_move(), we calculate the id pointer based on
header.size and idr_size:

    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 PERF_SAMPLE_TID is missing from the configured sample type but
PERF_SAMPLE_TIME is present, idr_size evaluates to 8, making the allocation
size become sizeof(*event) + 8.

Because id->time is located at an 8-byte offset within the id structure
(after pid and tid), writing to id->time accesses memory 8 bytes past the
start of id.

When the filename string approaches PATH_MAX and header.size nears the base
allocation size, this writes exactly past the end of the calloc heap
allocation. Could this memory corruption be triggered by parsing a
maliciously crafted perf.data file?

This is a pre-existing issue, but does this exact same pattern also cause
an out-of-bounds write in jit_repipe_code_load()?

Looking at jit_repipe_code_load(), it similarly uses the idr_size for its
allocation limit and unconditionally performs the same timestamp assignment:

tools/perf/util/jitdump.c:jit_repipe_code_load() {
    ...
    event = calloc(1, sizeof(*event) + idr_size);
    ...
    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);
    ...
}

Should both load and move operations account for the struct offset of
id->time when sizing the buffer?

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

Re: [PATCH 5/5] perf dso: Defer dropping the open list reference until after the lock

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Failing to allocate a deferred put node leaks the reference handle and causes a memory leak of the DSO object.
--

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

perf dso: Defer dropping the open list reference until after the lock

This patch addresses a deadlock and resource imbalance issue by deferring
the dropping of the DSO open list reference. It transfers the reference to a
deferred node during dso__list_del(), which is later drained by
dso__put_deferred() outside of the dso__data_open_lock.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 42bfe30a3b518..a4b2361bc7420 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
[ ... ]
> @@ -531,18 +531,79 @@ static void dso__list_add(struct dso *dso) EXCLUSIVE_LOCKS_REQUIRED(_dso__data_o
>  	dso__data_open_cnt++;
>  }
>  
> +#ifdef REFCNT_CHECKING
> +/*
> + * A deferred put: carries the reference taken by dso__list_add() for an
> + * entry removed from dso__data_open.  Dedicated nodes are used so that
> + * the dso_data's own open_entry node can be relinked by a concurrent
> + * dso__list_add() without corrupting this list or its reference.
> + */
> +struct dso_data_put {
> +	struct list_head entry;
> +	struct dso *dso;
> +};
> +static LIST_HEAD(dso__data_open_put);
> +#endif
> +
>  static void dso__list_del(struct dso *dso) EXCLUSIVE_LOCKS_REQUIRED(_dso__data_open_lock)
>  {
> -	list_del_init(&dso__data(dso)->open_entry);
>  #ifdef REFCNT_CHECKING
> -	mutex_unlock(dso__data_open_lock());
> -	dso__put(dso__data(dso)->dso);
> -	mutex_lock(dso__data_open_lock());
> +	struct dso_data_put *put;
>  #endif
> +
> +	list_del_init(&dso__data(dso)->open_entry);
>  	WARN_ONCE(dso__data_open_cnt <= 0,
>  		  "DSO data fd counter out of bounds.");
>  	dso__data_open_cnt--;
> +#ifdef REFCNT_CHECKING
> +	/*
> +	 * The reference taken in dso__list_add() cannot be dropped while
> +	 * holding the open lock: dso__put() may call dso__data_close(),
> +	 * which takes dso__data_open_lock itself, deadlocking and leaving
> +	 * the list/counter state inconsistent for concurrent threads.
> +	 * Transfer the reference to a deferred node drained by
> +	 * dso__put_deferred() once the lock is released.
> +	 */
> +	put = zalloc(sizeof(*put));
> +
> +	if (put == NULL)
> +		return;

[Severity: Medium]
Does this early return cause a memory leak of the DSO object if zalloc fails?

During dso__list_del(), returning early without placing the deferred put node
onto dso__data_open_put abandons the reference in dso__data(dso)->dso. If the
DSO is subsequently added to the open list again, dso__list_add() could
overwrite this pointer with a new reference, permanently leaking the prior one
and preventing the DSO reference count from ever reaching 0.

> +
> +	put->dso = dso__data(dso)->dso;
> +	dso__data(dso)->dso = NULL;
> +	list_add_tail(&put->entry, &dso__data_open_put);
> +#endif
> +}

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

Re: [PATCH 5/5] perf dso: Defer dropping the open list reference until after the lock

[Ian Rogers]

On Thu, Sep 3, 2026 at 6:23 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> There was a problem in the code with some resources potentially being
> left unbalanced, and the logic on dso__data_close() becoming confused
> if the fd had been closed already.
>
> The reference taken by dso__list_add() on the open list cannot be
> dropped while holding the open lock: dso__put() may call
> dso__data_close(), which takes dso__data_open_lock() itself,
> deadlocking and leaving the list and its counter inconsistent for
> concurrent threads.
>
> Fix it by changing dso__list_del() to transfer the reference to a
> deferred node, drained by dso__put_deferred() right after every
> unlock of dso__data_open_lock().  Since the counter is now decremented
> under the open lock, do_open()'s close_first_dso() no longer races
> with a stale count.

So I'm not a fan of this change due to its complexity. There reference
counting with dso_data is funny, see:
https://lore.kernel.org/r/20240506180104.485674-5-irogers@google.com
Basically a dso has a dso_data embedded within it. Perhaps the cleaner
fix is to allocate the dso_data, separate from the dso, and have a
reference to the dso from the dso_data. We should be able to scope our
lock usage and avoid deadlock without resorting to a deferral
mechanism.

Thanks,
Ian

> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: LLM
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/dso.c | 75 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 42bfe30a3b518e80..a4b2361bc7420084 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -531,18 +531,79 @@ static void dso__list_add(struct dso *dso) EXCLUSIVE_LOCKS_REQUIRED(_dso__data_o
>         dso__data_open_cnt++;
>  }
>
> +#ifdef REFCNT_CHECKING
> +/*
> + * A deferred put: carries the reference taken by dso__list_add() for an
> + * entry removed from dso__data_open.  Dedicated nodes are used so that
> + * the dso_data's own open_entry node can be relinked by a concurrent
> + * dso__list_add() without corrupting this list or its reference.
> + */
> +struct dso_data_put {
> +       struct list_head entry;
> +       struct dso *dso;
> +};
> +static LIST_HEAD(dso__data_open_put);
> +#endif
> +
>  static void dso__list_del(struct dso *dso) EXCLUSIVE_LOCKS_REQUIRED(_dso__data_open_lock)
>  {
> -       list_del_init(&dso__data(dso)->open_entry);
>  #ifdef REFCNT_CHECKING
> -       mutex_unlock(dso__data_open_lock());
> -       dso__put(dso__data(dso)->dso);
> -       mutex_lock(dso__data_open_lock());
> +       struct dso_data_put *put;
>  #endif
> +
> +       list_del_init(&dso__data(dso)->open_entry);
>         WARN_ONCE(dso__data_open_cnt <= 0,
>                   "DSO data fd counter out of bounds.");
>         dso__data_open_cnt--;
> +#ifdef REFCNT_CHECKING
> +       /*
> +        * The reference taken in dso__list_add() cannot be dropped while
> +        * holding the open lock: dso__put() may call dso__data_close(),
> +        * which takes dso__data_open_lock itself, deadlocking and leaving
> +        * the list/counter state inconsistent for concurrent threads.
> +        * Transfer the reference to a deferred node drained by
> +        * dso__put_deferred() once the lock is released.
> +        */
> +       put = zalloc(sizeof(*put));
> +
> +       if (put == NULL)
> +               return;
> +
> +       put->dso = dso__data(dso)->dso;
> +       dso__data(dso)->dso = NULL;
> +       list_add_tail(&put->entry, &dso__data_open_put);
> +#endif
> +}
> +
> +#ifdef REFCNT_CHECKING
> +/*
> + * Drop the references deferred by dso__list_del().  Must be called
> + * without holding dso__data_open_lock: dso__put() may re-enter it via
> + * dso__data_close().
> + */
> +static void dso__put_deferred(void) LOCKS_EXCLUDED(_dso__data_open_lock)
> +{
> +       for (;;) {
> +               struct dso_data_put *put;
> +               struct dso *dso;
> +
> +               mutex_lock(dso__data_open_lock());
> +               put = list_first_entry_or_null(&dso__data_open_put, struct dso_data_put, entry);
> +               if (put == NULL) {
> +                       mutex_unlock(dso__data_open_lock());
> +                       return;
> +               }
> +               list_del_init(&put->entry);
> +               dso = put->dso;
> +               mutex_unlock(dso__data_open_lock());
> +
> +               free(put);
> +               dso__put(dso);
> +       }
>  }
> +#else
> +static void dso__put_deferred(void) {}
> +#endif
>
>  static void close_first_dso(void);
>
> @@ -805,6 +866,7 @@ void dso__data_close(struct dso *dso)
>         mutex_lock(dso__data_open_lock());
>         close_dso(dso);
>         mutex_unlock(dso__data_open_lock());
> +       dso__put_deferred();
>  }
>
>  static void try_to_open_dso(struct dso *dso, struct machine *machine)
> @@ -865,12 +927,14 @@ bool dso__data_get_fd(struct dso *dso, struct machine *machine, int *fd)
>                 return true;
>
>         mutex_unlock(dso__data_open_lock());
> +       dso__put_deferred();
>         return false;
>  }
>
>  void dso__data_put_fd(struct dso *dso __maybe_unused)
>  {
>         mutex_unlock(dso__data_open_lock());
> +       dso__put_deferred();
>  }
>
>  bool dso__data_status_seen(struct dso *dso, enum dso_data_status_seen by)
> @@ -1058,6 +1122,7 @@ static ssize_t file_read(struct dso *dso, struct machine *machine,
>         ret = pread(dso__data(dso)->fd, data, DSO__DATA_CACHE_SIZE, offset);
>  out:
>         mutex_unlock(dso__data_open_lock());
> +       dso__put_deferred();
>         return ret;
>  }
>
> @@ -1188,6 +1253,7 @@ static int file_size(struct dso *dso, struct machine *machine)
>
>  out:
>         mutex_unlock(dso__data_open_lock());
> +       dso__put_deferred();
>         return ret;
>  }
>
> @@ -1405,6 +1471,7 @@ uint16_t dso__e_machine_endian(struct dso *dso, struct machine *machine, uint32_
>                 *e_flags = 0;
>
>         mutex_unlock(dso__data_open_lock());
> +       dso__put_deferred();
>         return e_machine;
>  }
>
> --
> 2.55.0
>

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

[Ian Rogers]

On Thu, Sep 3, 2026 at 6:23 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> jit_repipe_code_load() only cleared the unwinding state when both
> unwinding_data and eh_frame_hdr_size were set.  When a record carries
> unwinding data but eh_frame_hdr_size is 0, the cleanup condition fails
> and the unwinding state persists in jd, being applied to all subsequent
> JIT_CODE_LOAD and JIT_CODE_MOVE records, duplicating unwinding sections
> in the generated ELF files and inflating their event->mmap2.len.
>
> The record is validated upstream so eh_frame_hdr_size <= unwinding_size
> always holds.  Free the unwinding data based on the data pointer alone.
>
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Assisted-by: LLM
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

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

Thanks,
Ian

> ---
>  tools/perf/util/jitdump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index e0d5cc9a828189a2..efb40d93e33ae664 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -530,7 +530,7 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
>                 jd->nr_debug_entries = 0;
>         }
>
> -       if (jd->unwinding_data && jd->eh_frame_hdr_size) {
> +       if (jd->unwinding_data) {
>                 zfree(&jd->unwinding_data);
>                 jd->eh_frame_hdr_size = 0;
>                 jd->unwinding_mapped_size = 0;
> --
> 2.55.0
>

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

[Arnaldo Carvalho de Melo]

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

jit_repipe_code_load() only cleared the unwinding state when both
unwinding_data and eh_frame_hdr_size were set.  When a record carries
unwinding data but eh_frame_hdr_size is 0, the cleanup condition fails
and the unwinding state persists in jd, being applied to all subsequent
JIT_CODE_LOAD and JIT_CODE_MOVE records, duplicating unwinding sections
in the generated ELF files and inflating their event->mmap2.len.

The record is validated upstream so eh_frame_hdr_size <= unwinding_size
always holds.  Free the unwinding data based on the data pointer alone.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Reviewed-by: Ian Rogers <irogers@google.com>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/jitdump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Arbitrary File Overwrite via Predictable JIT ELF Filename (Symlink Attack).
- [High] Unwinding and debug data leak to subsequent records on error paths.
- [High] Silent suppression of parsing errors in jit_process_dump() loop.
- [Medium] State leak and MMAP2 length truncation in jit_repipe_code_move().
- [Medium] Ineffective use of flockfile() for inter-process synchronization.
--

commit 9551fc5ddf55f406039867f081e66a51333a4aa8
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf jitdump: Free unwinding data even when eh_frame_hdr_size is zero

This patch fixes an issue in jit_repipe_code_load() where unwinding state
was not properly cleared when eh_frame_hdr_size was zero. It updates the
cleanup condition to rely solely on the presence of unwinding_data, ensuring
it is freed correctly and not mistakenly applied to subsequent records.

Looking at jit_emit_elf() in the same 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;
...
}

[Severity: Critical]
This is a pre-existing issue, but does this open() call enable a symlink
attack?

perf inject --jit reads perf.data to find JIT dump directories, which are
specified by the monitored process and can be in world-writable locations
(e.g., /tmp). It constructs a predictable filename jitted-<pid>-<index>.so
in this directory.

The open() is called with O_CREAT|O_TRUNC|O_WRONLY but without O_NOFOLLOW
or O_EXCL.

Can a local attacker create a symlink at this predictable path pointing to
a critical system file (like /etc/shadow)? If perf inject is run by root
or a privileged user on this untrusted perf.data, would the target file be
truncated and overwritten with ELF data?


Looking at jit_repipe_code_load():

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

[Severity: High]
This isn't a bug introduced by this patch, but do jd->unwinding_data and
jd->debug_data leak if the function returns here?

jd->unwinding_data and jd->debug_data are global state pointers holding
dynamically allocated memory for the current JIT record.

If jit_repipe_code_load() hits this error condition, it returns -1
immediately. The allocated memory is not freed, and the size fields are not
reset.

Because jit_process_dump() ignores this error and continues parsing, will
the leaked unwinding and debug data be erroneously applied to the next
successful JIT_CODE_LOAD record, corrupting its ELF sections?


Looking at jit_process_dump():

tools/perf/util/jitdump.c:jit_process_dump() {
...
	while ((jr = jit_get_next_entry(jd))) {
		switch(jr->prefix.id) {
...
		default:
			ret = 0;
			continue;
		}
	}
	return ret;
}

[Severity: High]
This is a pre-existing issue, but does this loop silently suppress parsing
errors?

The while loop iterates over records and assigns the result of processing
functions to ret. There is no error-checking mechanism (e.g., if (ret)
break;) to halt processing.

If a subsequent record is successfully parsed, ret is overwritten with 0.
If an unknown record type is encountered, the default switch case explicitly
resets ret = 0.

Does this cause critical memory allocation failures, invalid record sizes,
or corrupted streams in the middle of a JIT dump to be completely ignored?


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

[Severity: Medium]
This is a pre-existing issue, but does modifying jit_repipe_code_load() to
unconditionally clear jd->unwinding_mapped_size cause a state leak and
MMAP2 length truncation in jit_repipe_code_move()?

jit_repipe_code_move() computes the new mapping length by reading the global
jd->unwinding_mapped_size into usize:

tools/perf/util/jitdump.c:jit_repipe_code_move() {
...
	usize = jd->unwinding_mapped_size;
...
	event->mmap2.len   = usize ? ALIGN_8(jr->move.code_size) + usize
				   : jr->move.code_size;
...
}

Because of the unconditional clear in this patch, unless a JIT agent emits
an out-of-sequence JIT_CODE_UNWINDING_INFO record immediately before the
MOVE record, will usize always be 0?

Does this cause the new MMAP2 record length to incorrectly truncate the
memory mapping, failing to cover the .eh_frame section from the original
JIT allocation?

Furthermore, if an agent does emit an unwinding record before the move,
jit_repipe_code_move() uses the size but fails to clear it. Does this leak
the state to all subsequent MOVE records?

>  		jd->unwinding_size = 0;
>  	}


Looking at jit_open():

tools/perf/util/jitdump.c:jit_open() {
...
	/*
	 * protect from writer modifying the file while we are reading it
	 */
	flockfile(jd->in);
...
}

[Severity: Medium]
This isn't a bug introduced by this patch, but is this use of flockfile()
ineffective for inter-process synchronization?

The comment states this is to protect from a writer modifying the file while
reading it. However, the writer is the JVM or JIT agent executing in a
different process.

Since flockfile() is a POSIX thread-synchronization primitive that only
locks the FILE * structure for thread-safe stdio operations within the
same process, it does not acquire an OS-level file lock.

Does this leave the reader vulnerable to data races and parsing corruption
if the dump file is read concurrently while the JIT agent is appending to it?

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