Linux Perf Users
 help / color / mirror / Atom feed
* [PATCH] perf jitdump: Fix a build error with ASAN
@ 2026-06-05  6:49 Namhyung Kim
  2026-06-05  7:03 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2026-06-05  6:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
  Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
	linux-perf-users

I found this error when I pass EXTRA_CFLAGS=-fsanitize=address on Fedora
44 with GCC 16.  Fix it by copying one less byte.

    CC      util/jitdump.o
  util/jitdump.c: In function ‘jit_process’:
  util/jitdump.c:237:9: error: ‘__builtin_strncpy’ specified bound 4096 equals destination size
                               [-Werror=stringop-truncation]
    237 |         strncpy(jd->dir, name, PATH_MAX);
        |         ^
  cc1: all warnings being treated as errors
  make[4]: *** [tools/build/Makefile.build:95: util/jitdump.o] Error 1
  make[4]: *** Waiting for unfinished jobs....
  make[3]: *** [tools/build/Makefile.build:158: util] Error 2
  make[2]: *** [Makefile.perf:578: perf-util-in.o] Error 2
  make[1]: *** [Makefile.perf:288: sub-make] Error 2
  make: *** [Makefile:76: all] Error 2

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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 18fd84a82153c2ab..83005b30b9bf3fd7 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -234,7 +234,7 @@ jit_open(struct jit_buf_desc *jd, const char *name)
 	/*
 	 * keep dirname for generating files and mmap records
 	 */
-	strncpy(jd->dir, name, PATH_MAX);
+	strncpy(jd->dir, name, PATH_MAX - 1);
 	jd->dir[PATH_MAX - 1] = '\0';
 	dirname(jd->dir);
 	free(buf);
-- 
2.54.0


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

* Re: [PATCH] perf jitdump: Fix a build error with ASAN
  2026-06-05  6:49 [PATCH] perf jitdump: Fix a build error with ASAN Namhyung Kim
@ 2026-06-05  7:03 ` sashiko-bot
  2026-06-05 23:22   ` Namhyung Kim
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-06-05  7:03 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-perf-users

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

Pre-existing issues:
- [High] Unlocking an unlocked stream on the error path causes undefined behavior.
- [High] Incorrect logic for skipping extended jitdump headers leads to parsing failure.
- [High] Ignoring the return value of `dirname()` breaks directory extraction and causes ENOTDIR failures.
- [High] If the generated JIT ELF path exceeds PATH_MAX, snprintf truncation causes size underflow resulting in a heap buffer overflow.
- [High] Cross-endian byte swapping of JIT_CODE_DEBUG_INFO records corrupts data by ignoring the flexible array member.
- [High] Lack of validation on `code_size` enables arbitrary out-of-bounds read and memory leakage.
- [Medium] Memory leak of `debug_data` and `unwinding_data` pointers.
- [Medium] Memory leak of `event` in `jit_repipe_code_move`.
- [Medium] Ineffective inter-process synchronization using `flockfile`.
- [Medium] Integer underflow leading to massive malloc in `jit_repipe_debug_info`.
--

commit f314399f4edf9366c3ee4d529f45cb2d7e7d454d
Author: Namhyung Kim <namhyung@kernel.org>

perf jitdump: Fix a build error with ASAN

This patch resolves a stringop-truncation build error when compiling perf
with ASAN on GCC 16. It reduces the strncpy bound by one byte to avoid
truncation warnings.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index e0ce8b9047298..16390e7f9062d 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -234,7 +234,7 @@ jit_open(struct jit_buf_desc *jd, const char *name)
>  	/*
>  	 * keep dirname for generating files and mmap records
>  	 */
> -	strncpy(jd->dir, name, PATH_MAX);
> +	strncpy(jd->dir, name, PATH_MAX - 1);
>  	jd->dir[PATH_MAX - 1] = '\0';
>  	dirname(jd->dir);

[Severity: High]
This isn't a bug introduced by this patch, but is it correct to ignore the
return value of dirname()?

If jd->dir contains no slashes, glibc dirname() returns a pointer to "." but
leaves the original string unmodified. Since the return value is discarded,
jd->dir retains the full filename instead of becoming ".". This could cause
later snprintf calls to use the target file as a directory component, leading
to ENOTDIR failures.

>  	free(buf);

[Severity: High]
This is a pre-existing issue, but does the error handling path incorrectly
unlock an unlocked stream?

If malloc() fails early in jit_open(), it jumps to the error label:

tools/perf/util/jitdump.c:jit_open() {
    ...
	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);
	fclose(jd->in);
	return retval;
}

Since flockfile() is only called after malloc() succeeds, calling funlockfile()
here results in undefined behavior on an unlocked stream.

[Severity: High]
This is a pre-existing issue, but does this logic break extended header reads?

In jit_open(), when handling extended headers, bsz is updated to bs right
before calculating the read size:

tools/perf/util/jitdump.c:jit_open() {
    ...
	if (bs > bsz) {
		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);
		if (ret != 1)
			goto error;
	}
    ...
}

Because bsz = bs, the read size bs - bsz is always 0. fread() will return 0,
causing the ret != 1 check to fail and unconditionally jump to the error path.

[Severity: High]
This is a pre-existing issue, but could snprintf() truncation cause an
out-of-bounds pointer calculation?

In jit_repipe_code_load(), snprintf() calculates a path:

tools/perf/util/jitdump.c:jit_repipe_code_load() {
    ...
	filename = event->mmap2.filename;
	size = snprintf(filename, PATH_MAX, "%s/jitted-%d-%" PRIu64 ".so",
			jd->dir,
			nspid,
			count);

	size++; /* for \0 */

	size = PERF_ALIGN(size, sizeof(u64));
    ...
	event->mmap2.header.size = (sizeof(event->mmap2) -
			(sizeof(event->mmap2.filename) - size) + idr_size);
    ...
	id = (void *)((unsigned long)event + event->mmap.header.size - idr_size);
	if (jd->sample_type & PERF_SAMPLE_TID) {
		id->pid  = pid;
    ...
}

If the string exceeds PATH_MAX, snprintf() returns the untruncated length. The
expression sizeof(event->mmap2.filename) - size will then underflow because size
is greater than PATH_MAX, resulting in a massively inflated header.size. The
pointer id will point outside the allocated bounds, leading to heap corruption
when id->pid is assigned.

[Severity: High]
This isn't a bug introduced by this patch, but does the byte-swapping logic
corrupt data by ignoring a flexible array member?

In jit_get_next_entry():

tools/perf/util/jitdump.c:jit_get_next_entry() {
    ...
	case JIT_CODE_DEBUG_INFO:
		if (jd->needs_bswap) {
			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);
			}
		}
		break;
    ...
}

struct debug_entry contains a flexible array member (const char name[]). Using
array indexing (jr->info.entries[n]) assumes fixed-size elements. This will
calculate incorrect offsets that land inside the variable-length strings of
earlier entries, overwriting them with byte-swapped data.

[Severity: Medium]
This is a pre-existing issue, but do jd->debug_data and jd->unwinding_data leak?

In jit_repipe_debug_info(), memory is allocated and assigned:

tools/perf/util/jitdump.c:jit_repipe_debug_info() {
    ...
	data = malloc(sz);
	if (!data)
		return -1;

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

	jd->debug_data       = data;
    ...
}

While these are freed during jit_repipe_code_load(), if a JIT dump terminates
early, lacks trailing load records, or contains consecutive debug records, the
previously allocated buffers are overwritten or never freed. Neither jit_close()
nor jit_process() cleans up jd->debug_data or jd->unwinding_data.

[Severity: Medium]
This isn't a bug introduced by this patch, but does jit_repipe_code_move() leak
its event allocation?

tools/perf/util/jitdump.c:jit_repipe_code_move() {
    ...
	event = calloc(1, sizeof(*event) + 16);
	if (!event)
		return -1;
    ...
out:
	perf_sample__exit(&sample);
	return ret;
}

The event buffer is dynamically allocated but there is no free(event) in the
cleanup block, leaking the memory for every JIT_CODE_MOVE record processed.

[Severity: Medium]
This is a pre-existing issue, but is flockfile() being used incorrectly for
inter-process synchronization?

In jit_open():

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

flockfile() is a POSIX thread mutex that provides no OS-level file locking.
It cannot prevent an external process from modifying the file concurrently,
making this synchronization completely ineffective against external writers.

[Severity: Medium]
This isn't a bug introduced by this patch, but could this allocation size calculation
underflow?

In jit_repipe_debug_info():

tools/perf/util/jitdump.c:jit_repipe_debug_info() {
    ...
	sz  = jr->prefix.total_size - sizeof(jr->info);
	data = malloc(sz);
    ...
}

If total_size is smaller than sizeof(jr->info), the unsigned subtraction will
underflow to a massive value, leading to a guaranteed malloc() failure or
instability if memory is overcommitted.

[Severity: High]
This is a pre-existing issue, but does lack of validation on code_size allow
out-of-bounds reads?

In jit_repipe_code_load():

tools/perf/util/jitdump.c:jit_repipe_code_load() {
    ...
	csize = jr->load.code_size;
    ...
	code  = (unsigned long)jr + jr->load.p.total_size - csize;
    ...
	ret = jit_emit_elf(jd, filename, sym, addr, (const void *)uaddr, csize, ...
    ...
}

If an artificially large csize is parsed from the file without being bounds
checked against total_size, the code pointer calculation will underflow and
point into earlier heap memory. jit_emit_elf() will then dump csize bytes
from this out-of-bounds memory into the ELF file.

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

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

* Re: [PATCH] perf jitdump: Fix a build error with ASAN
  2026-06-05  7:03 ` sashiko-bot
@ 2026-06-05 23:22   ` Namhyung Kim
  0 siblings, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2026-06-05 23:22 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-perf-users

On Fri, Jun 05, 2026 at 07:03:49AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Unlocking an unlocked stream on the error path causes undefined behavior.
> - [High] Incorrect logic for skipping extended jitdump headers leads to parsing failure.
> - [High] Ignoring the return value of `dirname()` breaks directory extraction and causes ENOTDIR failures.
> - [High] If the generated JIT ELF path exceeds PATH_MAX, snprintf truncation causes size underflow resulting in a heap buffer overflow.
> - [High] Cross-endian byte swapping of JIT_CODE_DEBUG_INFO records corrupts data by ignoring the flexible array member.
> - [High] Lack of validation on `code_size` enables arbitrary out-of-bounds read and memory leakage.
> - [Medium] Memory leak of `debug_data` and `unwinding_data` pointers.
> - [Medium] Memory leak of `event` in `jit_repipe_code_move`.
> - [Medium] Ineffective inter-process synchronization using `flockfile`.
> - [Medium] Integer underflow leading to massive malloc in `jit_repipe_debug_info`.

I don't plan to touch any of these issues, at least in the near future.

Thanks,
Namhyung

> --
> 
> commit f314399f4edf9366c3ee4d529f45cb2d7e7d454d
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf jitdump: Fix a build error with ASAN
> 
> This patch resolves a stringop-truncation build error when compiling perf
> with ASAN on GCC 16. It reduces the strncpy bound by one byte to avoid
> truncation warnings.
> 
> > diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> > index e0ce8b9047298..16390e7f9062d 100644
> > --- a/tools/perf/util/jitdump.c
> > +++ b/tools/perf/util/jitdump.c
> > @@ -234,7 +234,7 @@ jit_open(struct jit_buf_desc *jd, const char *name)
> >  	/*
> >  	 * keep dirname for generating files and mmap records
> >  	 */
> > -	strncpy(jd->dir, name, PATH_MAX);
> > +	strncpy(jd->dir, name, PATH_MAX - 1);
> >  	jd->dir[PATH_MAX - 1] = '\0';
> >  	dirname(jd->dir);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but is it correct to ignore the
> return value of dirname()?
> 
> If jd->dir contains no slashes, glibc dirname() returns a pointer to "." but
> leaves the original string unmodified. Since the return value is discarded,
> jd->dir retains the full filename instead of becoming ".". This could cause
> later snprintf calls to use the target file as a directory component, leading
> to ENOTDIR failures.
> 
> >  	free(buf);
> 
> [Severity: High]
> This is a pre-existing issue, but does the error handling path incorrectly
> unlock an unlocked stream?
> 
> If malloc() fails early in jit_open(), it jumps to the error label:
> 
> tools/perf/util/jitdump.c:jit_open() {
>     ...
> 	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);
> 	fclose(jd->in);
> 	return retval;
> }
> 
> Since flockfile() is only called after malloc() succeeds, calling funlockfile()
> here results in undefined behavior on an unlocked stream.
> 
> [Severity: High]
> This is a pre-existing issue, but does this logic break extended header reads?
> 
> In jit_open(), when handling extended headers, bsz is updated to bs right
> before calculating the read size:
> 
> tools/perf/util/jitdump.c:jit_open() {
>     ...
> 	if (bs > bsz) {
> 		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);
> 		if (ret != 1)
> 			goto error;
> 	}
>     ...
> }
> 
> Because bsz = bs, the read size bs - bsz is always 0. fread() will return 0,
> causing the ret != 1 check to fail and unconditionally jump to the error path.
> 
> [Severity: High]
> This is a pre-existing issue, but could snprintf() truncation cause an
> out-of-bounds pointer calculation?
> 
> In jit_repipe_code_load(), snprintf() calculates a path:
> 
> tools/perf/util/jitdump.c:jit_repipe_code_load() {
>     ...
> 	filename = event->mmap2.filename;
> 	size = snprintf(filename, PATH_MAX, "%s/jitted-%d-%" PRIu64 ".so",
> 			jd->dir,
> 			nspid,
> 			count);
> 
> 	size++; /* for \0 */
> 
> 	size = PERF_ALIGN(size, sizeof(u64));
>     ...
> 	event->mmap2.header.size = (sizeof(event->mmap2) -
> 			(sizeof(event->mmap2.filename) - size) + idr_size);
>     ...
> 	id = (void *)((unsigned long)event + event->mmap.header.size - idr_size);
> 	if (jd->sample_type & PERF_SAMPLE_TID) {
> 		id->pid  = pid;
>     ...
> }
> 
> If the string exceeds PATH_MAX, snprintf() returns the untruncated length. The
> expression sizeof(event->mmap2.filename) - size will then underflow because size
> is greater than PATH_MAX, resulting in a massively inflated header.size. The
> pointer id will point outside the allocated bounds, leading to heap corruption
> when id->pid is assigned.
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does the byte-swapping logic
> corrupt data by ignoring a flexible array member?
> 
> In jit_get_next_entry():
> 
> tools/perf/util/jitdump.c:jit_get_next_entry() {
>     ...
> 	case JIT_CODE_DEBUG_INFO:
> 		if (jd->needs_bswap) {
> 			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);
> 			}
> 		}
> 		break;
>     ...
> }
> 
> struct debug_entry contains a flexible array member (const char name[]). Using
> array indexing (jr->info.entries[n]) assumes fixed-size elements. This will
> calculate incorrect offsets that land inside the variable-length strings of
> earlier entries, overwriting them with byte-swapped data.
> 
> [Severity: Medium]
> This is a pre-existing issue, but do jd->debug_data and jd->unwinding_data leak?
> 
> In jit_repipe_debug_info(), memory is allocated and assigned:
> 
> tools/perf/util/jitdump.c:jit_repipe_debug_info() {
>     ...
> 	data = malloc(sz);
> 	if (!data)
> 		return -1;
> 
> 	memcpy(data, &jr->info.entries, sz);
> 
> 	jd->debug_data       = data;
>     ...
> }
> 
> While these are freed during jit_repipe_code_load(), if a JIT dump terminates
> early, lacks trailing load records, or contains consecutive debug records, the
> previously allocated buffers are overwritten or never freed. Neither jit_close()
> nor jit_process() cleans up jd->debug_data or jd->unwinding_data.
> 
> [Severity: Medium]
> This isn't a bug introduced by this patch, but does jit_repipe_code_move() leak
> its event allocation?
> 
> tools/perf/util/jitdump.c:jit_repipe_code_move() {
>     ...
> 	event = calloc(1, sizeof(*event) + 16);
> 	if (!event)
> 		return -1;
>     ...
> out:
> 	perf_sample__exit(&sample);
> 	return ret;
> }
> 
> The event buffer is dynamically allocated but there is no free(event) in the
> cleanup block, leaking the memory for every JIT_CODE_MOVE record processed.
> 
> [Severity: Medium]
> This is a pre-existing issue, but is flockfile() being used incorrectly for
> inter-process synchronization?
> 
> In jit_open():
> 
> tools/perf/util/jitdump.c:jit_open() {
>     ...
> 	/*
> 	 * protect from writer modifying the file while we are reading it
> 	 */
> 	flockfile(jd->in);
>     ...
> }
> 
> flockfile() is a POSIX thread mutex that provides no OS-level file locking.
> It cannot prevent an external process from modifying the file concurrently,
> making this synchronization completely ineffective against external writers.
> 
> [Severity: Medium]
> This isn't a bug introduced by this patch, but could this allocation size calculation
> underflow?
> 
> In jit_repipe_debug_info():
> 
> tools/perf/util/jitdump.c:jit_repipe_debug_info() {
>     ...
> 	sz  = jr->prefix.total_size - sizeof(jr->info);
> 	data = malloc(sz);
>     ...
> }
> 
> If total_size is smaller than sizeof(jr->info), the unsigned subtraction will
> underflow to a massive value, leading to a guaranteed malloc() failure or
> instability if memory is overcommitted.
> 
> [Severity: High]
> This is a pre-existing issue, but does lack of validation on code_size allow
> out-of-bounds reads?
> 
> In jit_repipe_code_load():
> 
> tools/perf/util/jitdump.c:jit_repipe_code_load() {
>     ...
> 	csize = jr->load.code_size;
>     ...
> 	code  = (unsigned long)jr + jr->load.p.total_size - csize;
>     ...
> 	ret = jit_emit_elf(jd, filename, sym, addr, (const void *)uaddr, csize, ...
>     ...
> }
> 
> If an artificially large csize is parsed from the file without being bounds
> checked against total_size, the code pointer calculation will underflow and
> point into earlier heap memory. jit_emit_elf() will then dump csize bytes
> from this out-of-bounds memory into the ELF file.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260605064932.34316-1-namhyung@kernel.org?part=1

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

end of thread, other threads:[~2026-06-05 23:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-05  6:49 [PATCH] perf jitdump: Fix a build error with ASAN Namhyung Kim
2026-06-05  7:03 ` sashiko-bot
2026-06-05 23:22   ` Namhyung Kim

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