* [PATCH v1 1/6] perf symbol: Fix use-after-free in filename__read_build_id
2025-05-27 18:06 [PATCH v1 0/6] Various asan and test fixes Ian Rogers
@ 2025-05-27 18:06 ` Ian Rogers
2025-05-28 12:48 ` Arnaldo Carvalho de Melo
2025-05-27 18:06 ` [PATCH v1 2/6] perf test demangle-java: Don't segv if demangling fails Ian Rogers
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2025-05-27 18:06 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Jiapeng Chong, James Clark, Howard Chu, Weilin Wang,
Stephen Brennan, Andi Kleen, Dmitry Vyukov, linux-perf-users,
linux-kernel
The same buf is used for the program headers and reading notes. As the
notes memory may be reallocated then this corrupts the memory pointed
to by the phdr. Using the same buffer is in any case a logic
error. Rather than deal with the duplicated code, introduce an elf32
boolean and a union for either the elf32 or elf64 headers that are in
use. Let the program headers have their own memory and grow the buffer
for notes as necessary.
Before `perf list -j` compiled with asan would crash with:
```
==4176189==ERROR: AddressSanitizer: heap-use-after-free on address 0x5160000070b8 at pc 0x555d3b15075b bp 0x7ffebb5a8090 sp 0x7ffebb5a8088
READ of size 8 at 0x5160000070b8 thread T0
#0 0x555d3b15075a in filename__read_build_id tools/perf/util/symbol-minimal.c:212:25
#1 0x555d3ae43aff in filename__sprintf_build_id tools/perf/util/build-id.c:110:8
...
0x5160000070b8 is located 312 bytes inside of 560-byte region [0x516000006f80,0x5160000071b0)
freed by thread T0 here:
#0 0x555d3ab21840 in realloc (perf+0x264840) (BuildId: 12dff2f6629f738e5012abdf0e90055518e70b5e)
#1 0x555d3b1506e7 in filename__read_build_id tools/perf/util/symbol-minimal.c:206:11
...
previously allocated by thread T0 here:
#0 0x555d3ab21423 in malloc (perf+0x264423) (BuildId: 12dff2f6629f738e5012abdf0e90055518e70b5e)
#1 0x555d3b1503a2 in filename__read_build_id tools/perf/util/symbol-minimal.c:182:9
...
```
Note: this bug is long standing and not introduced by the other asan
fix in commit fa9c4977fbfb ("perf symbol-minimal: Fix double free in
filename__read_build_id").
Fixes: b691f64360ecec49 ("perf symbols: Implement poor man's ELF parser")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/symbol-minimal.c | 168 +++++++++++++------------------
1 file changed, 70 insertions(+), 98 deletions(-)
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index d8da3da01fe6..c9b7a1ca5e52 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -90,11 +90,23 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
{
FILE *fp;
int ret = -1;
- bool need_swap = false;
+ bool need_swap = false, elf32;
u8 e_ident[EI_NIDENT];
- size_t buf_size;
- void *buf;
int i;
+ union {
+ struct {
+ Elf32_Ehdr ehdr32;
+ Elf32_Phdr *phdr32;
+ };
+ struct {
+ Elf64_Ehdr ehdr64;
+ Elf64_Phdr *phdr64;
+ };
+ } hdrs;
+ void *phdr;
+ size_t phdr_size;
+ void *buf = NULL;
+ size_t buf_size = 0;
fp = fopen(filename, "r");
if (fp == NULL)
@@ -108,119 +120,79 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
goto out;
need_swap = check_need_swap(e_ident[EI_DATA]);
+ elf32 = e_ident[EI_CLASS] == ELFCLASS32;
- /* for simplicity */
- fseek(fp, 0, SEEK_SET);
-
- if (e_ident[EI_CLASS] == ELFCLASS32) {
- Elf32_Ehdr ehdr;
- Elf32_Phdr *phdr;
-
- if (fread(&ehdr, sizeof(ehdr), 1, fp) != 1)
- goto out;
+ if (fread(elf32 ? (void *)&hdrs.ehdr32 : (void *)&hdrs.ehdr64,
+ elf32 ? sizeof(hdrs.ehdr32) : sizeof(hdrs.ehdr32),
+ 1, fp) != 1)
+ goto out;
- if (need_swap) {
- ehdr.e_phoff = bswap_32(ehdr.e_phoff);
- ehdr.e_phentsize = bswap_16(ehdr.e_phentsize);
- ehdr.e_phnum = bswap_16(ehdr.e_phnum);
+ if (need_swap) {
+ if (elf32) {
+ hdrs.ehdr32.e_phoff = bswap_32(hdrs.ehdr32.e_phoff);
+ hdrs.ehdr32.e_phentsize = bswap_16(hdrs.ehdr32.e_phentsize);
+ hdrs.ehdr32.e_phnum = bswap_16(hdrs.ehdr32.e_phnum);
+ } else {
+ hdrs.ehdr64.e_phoff = bswap_64(hdrs.ehdr64.e_phoff);
+ hdrs.ehdr64.e_phentsize = bswap_16(hdrs.ehdr64.e_phentsize);
+ hdrs.ehdr64.e_phnum = bswap_16(hdrs.ehdr64.e_phnum);
}
+ }
+ phdr_size = elf32 ? hdrs.ehdr32.e_phentsize * hdrs.ehdr32.e_phnum
+ : hdrs.ehdr64.e_phentsize * hdrs.ehdr64.e_phnum;
+ phdr = malloc(phdr_size);
+ if (phdr == NULL)
+ goto out;
- buf_size = ehdr.e_phentsize * ehdr.e_phnum;
- buf = malloc(buf_size);
- if (buf == NULL)
- goto out;
-
- fseek(fp, ehdr.e_phoff, SEEK_SET);
- if (fread(buf, buf_size, 1, fp) != 1)
- goto out_free;
-
- for (i = 0, phdr = buf; i < ehdr.e_phnum; i++, phdr++) {
- void *tmp;
- long offset;
-
- if (need_swap) {
- phdr->p_type = bswap_32(phdr->p_type);
- phdr->p_offset = bswap_32(phdr->p_offset);
- phdr->p_filesz = bswap_32(phdr->p_filesz);
- }
-
- if (phdr->p_type != PT_NOTE)
- continue;
-
- offset = phdr->p_offset;
- if (phdr->p_filesz > buf_size) {
- buf_size = phdr->p_filesz;
- tmp = realloc(buf, buf_size);
- if (tmp == NULL)
- goto out_free;
- buf = tmp;
- }
- fseek(fp, offset, SEEK_SET);
- if (fread(buf, phdr->p_filesz, 1, fp) != 1)
- goto out_free;
+ fseek(fp, elf32 ? hdrs.ehdr32.e_phoff : hdrs.ehdr64.e_phoff, SEEK_SET);
+ if (fread(phdr, phdr_size, 1, fp) != 1)
+ goto out_free;
- ret = read_build_id(buf, phdr->p_filesz, bid, need_swap);
- if (ret == 0) {
- ret = bid->size;
- break;
- }
- }
- } else {
- Elf64_Ehdr ehdr;
- Elf64_Phdr *phdr;
+ if (elf32)
+ hdrs.phdr32 = phdr;
+ else
+ hdrs.phdr64 = phdr;
- if (fread(&ehdr, sizeof(ehdr), 1, fp) != 1)
- goto out;
+ for (i = 0; i < elf32 ? hdrs.ehdr32.e_phnum : hdrs.ehdr64.e_phnum; i++) {
+ size_t p_filesz;
if (need_swap) {
- ehdr.e_phoff = bswap_64(ehdr.e_phoff);
- ehdr.e_phentsize = bswap_16(ehdr.e_phentsize);
- ehdr.e_phnum = bswap_16(ehdr.e_phnum);
+ if (elf32) {
+ hdrs.phdr32[i].p_type = bswap_32(hdrs.phdr32[i].p_type);
+ hdrs.phdr32[i].p_offset = bswap_32(hdrs.phdr32[i].p_offset);
+ hdrs.phdr32[i].p_filesz = bswap_32(hdrs.phdr32[i].p_offset);
+ } else {
+ hdrs.phdr64[i].p_type = bswap_32(hdrs.phdr64[i].p_type);
+ hdrs.phdr64[i].p_offset = bswap_64(hdrs.phdr64[i].p_offset);
+ hdrs.phdr64[i].p_filesz = bswap_64(hdrs.phdr64[i].p_filesz);
+ }
}
+ if ((elf32 ? hdrs.phdr32[i].p_type : hdrs.phdr64[i].p_type) != PT_NOTE)
+ continue;
- buf_size = ehdr.e_phentsize * ehdr.e_phnum;
- buf = malloc(buf_size);
- if (buf == NULL)
- goto out;
-
- fseek(fp, ehdr.e_phoff, SEEK_SET);
- if (fread(buf, buf_size, 1, fp) != 1)
- goto out_free;
-
- for (i = 0, phdr = buf; i < ehdr.e_phnum; i++, phdr++) {
+ p_filesz = elf32 ? hdrs.phdr32[i].p_filesz : hdrs.phdr64[i].p_filesz;
+ if (p_filesz > buf_size) {
void *tmp;
- long offset;
-
- if (need_swap) {
- phdr->p_type = bswap_32(phdr->p_type);
- phdr->p_offset = bswap_64(phdr->p_offset);
- phdr->p_filesz = bswap_64(phdr->p_filesz);
- }
-
- if (phdr->p_type != PT_NOTE)
- continue;
- offset = phdr->p_offset;
- if (phdr->p_filesz > buf_size) {
- buf_size = phdr->p_filesz;
- tmp = realloc(buf, buf_size);
- if (tmp == NULL)
- goto out_free;
- buf = tmp;
- }
- fseek(fp, offset, SEEK_SET);
- if (fread(buf, phdr->p_filesz, 1, fp) != 1)
+ buf_size = p_filesz;
+ tmp = realloc(buf, buf_size);
+ if (tmp == NULL)
goto out_free;
+ buf = tmp;
+ }
+ fseek(fp, elf32 ? hdrs.phdr32[i].p_offset : hdrs.phdr64[i].p_offset, SEEK_SET);
+ if (fread(buf, p_filesz, 1, fp) != 1)
+ goto out_free;
- ret = read_build_id(buf, phdr->p_filesz, bid, need_swap);
- if (ret == 0) {
- ret = bid->size;
- break;
- }
+ ret = read_build_id(buf, p_filesz, bid, need_swap);
+ if (ret == 0) {
+ ret = bid->size;
+ break;
}
}
out_free:
free(buf);
+ free(phdr);
out:
fclose(fp);
return ret;
--
2.49.0.1204.g71687c7c1d-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v1 1/6] perf symbol: Fix use-after-free in filename__read_build_id
2025-05-27 18:06 ` [PATCH v1 1/6] perf symbol: Fix use-after-free in filename__read_build_id Ian Rogers
@ 2025-05-28 12:48 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-28 12:48 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jiapeng Chong, James Clark,
Howard Chu, Weilin Wang, Stephen Brennan, Andi Kleen,
Dmitry Vyukov, linux-perf-users, linux-kernel
On Tue, May 27, 2025 at 11:06:58AM -0700, Ian Rogers wrote:
> The same buf is used for the program headers and reading notes. As the
> notes memory may be reallocated then this corrupts the memory pointed
> to by the phdr. Using the same buffer is in any case a logic
> error. Rather than deal with the duplicated code, introduce an elf32
> boolean and a union for either the elf32 or elf64 headers that are in
> use. Let the program headers have their own memory and grow the buffer
> for notes as necessary.
Applied.
- Arnaldo
> Before `perf list -j` compiled with asan would crash with:
> ```
> ==4176189==ERROR: AddressSanitizer: heap-use-after-free on address 0x5160000070b8 at pc 0x555d3b15075b bp 0x7ffebb5a8090 sp 0x7ffebb5a8088
> READ of size 8 at 0x5160000070b8 thread T0
> #0 0x555d3b15075a in filename__read_build_id tools/perf/util/symbol-minimal.c:212:25
> #1 0x555d3ae43aff in filename__sprintf_build_id tools/perf/util/build-id.c:110:8
> ...
>
> 0x5160000070b8 is located 312 bytes inside of 560-byte region [0x516000006f80,0x5160000071b0)
> freed by thread T0 here:
> #0 0x555d3ab21840 in realloc (perf+0x264840) (BuildId: 12dff2f6629f738e5012abdf0e90055518e70b5e)
> #1 0x555d3b1506e7 in filename__read_build_id tools/perf/util/symbol-minimal.c:206:11
> ...
>
> previously allocated by thread T0 here:
> #0 0x555d3ab21423 in malloc (perf+0x264423) (BuildId: 12dff2f6629f738e5012abdf0e90055518e70b5e)
> #1 0x555d3b1503a2 in filename__read_build_id tools/perf/util/symbol-minimal.c:182:9
> ...
> ```
>
> Note: this bug is long standing and not introduced by the other asan
> fix in commit fa9c4977fbfb ("perf symbol-minimal: Fix double free in
> filename__read_build_id").
>
> Fixes: b691f64360ecec49 ("perf symbols: Implement poor man's ELF parser")
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/symbol-minimal.c | 168 +++++++++++++------------------
> 1 file changed, 70 insertions(+), 98 deletions(-)
>
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index d8da3da01fe6..c9b7a1ca5e52 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -90,11 +90,23 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
> {
> FILE *fp;
> int ret = -1;
> - bool need_swap = false;
> + bool need_swap = false, elf32;
> u8 e_ident[EI_NIDENT];
> - size_t buf_size;
> - void *buf;
> int i;
> + union {
> + struct {
> + Elf32_Ehdr ehdr32;
> + Elf32_Phdr *phdr32;
> + };
> + struct {
> + Elf64_Ehdr ehdr64;
> + Elf64_Phdr *phdr64;
> + };
> + } hdrs;
> + void *phdr;
> + size_t phdr_size;
> + void *buf = NULL;
> + size_t buf_size = 0;
>
> fp = fopen(filename, "r");
> if (fp == NULL)
> @@ -108,119 +120,79 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
> goto out;
>
> need_swap = check_need_swap(e_ident[EI_DATA]);
> + elf32 = e_ident[EI_CLASS] == ELFCLASS32;
>
> - /* for simplicity */
> - fseek(fp, 0, SEEK_SET);
> -
> - if (e_ident[EI_CLASS] == ELFCLASS32) {
> - Elf32_Ehdr ehdr;
> - Elf32_Phdr *phdr;
> -
> - if (fread(&ehdr, sizeof(ehdr), 1, fp) != 1)
> - goto out;
> + if (fread(elf32 ? (void *)&hdrs.ehdr32 : (void *)&hdrs.ehdr64,
> + elf32 ? sizeof(hdrs.ehdr32) : sizeof(hdrs.ehdr32),
> + 1, fp) != 1)
> + goto out;
>
> - if (need_swap) {
> - ehdr.e_phoff = bswap_32(ehdr.e_phoff);
> - ehdr.e_phentsize = bswap_16(ehdr.e_phentsize);
> - ehdr.e_phnum = bswap_16(ehdr.e_phnum);
> + if (need_swap) {
> + if (elf32) {
> + hdrs.ehdr32.e_phoff = bswap_32(hdrs.ehdr32.e_phoff);
> + hdrs.ehdr32.e_phentsize = bswap_16(hdrs.ehdr32.e_phentsize);
> + hdrs.ehdr32.e_phnum = bswap_16(hdrs.ehdr32.e_phnum);
> + } else {
> + hdrs.ehdr64.e_phoff = bswap_64(hdrs.ehdr64.e_phoff);
> + hdrs.ehdr64.e_phentsize = bswap_16(hdrs.ehdr64.e_phentsize);
> + hdrs.ehdr64.e_phnum = bswap_16(hdrs.ehdr64.e_phnum);
> }
> + }
> + phdr_size = elf32 ? hdrs.ehdr32.e_phentsize * hdrs.ehdr32.e_phnum
> + : hdrs.ehdr64.e_phentsize * hdrs.ehdr64.e_phnum;
> + phdr = malloc(phdr_size);
> + if (phdr == NULL)
> + goto out;
>
> - buf_size = ehdr.e_phentsize * ehdr.e_phnum;
> - buf = malloc(buf_size);
> - if (buf == NULL)
> - goto out;
> -
> - fseek(fp, ehdr.e_phoff, SEEK_SET);
> - if (fread(buf, buf_size, 1, fp) != 1)
> - goto out_free;
> -
> - for (i = 0, phdr = buf; i < ehdr.e_phnum; i++, phdr++) {
> - void *tmp;
> - long offset;
> -
> - if (need_swap) {
> - phdr->p_type = bswap_32(phdr->p_type);
> - phdr->p_offset = bswap_32(phdr->p_offset);
> - phdr->p_filesz = bswap_32(phdr->p_filesz);
> - }
> -
> - if (phdr->p_type != PT_NOTE)
> - continue;
> -
> - offset = phdr->p_offset;
> - if (phdr->p_filesz > buf_size) {
> - buf_size = phdr->p_filesz;
> - tmp = realloc(buf, buf_size);
> - if (tmp == NULL)
> - goto out_free;
> - buf = tmp;
> - }
> - fseek(fp, offset, SEEK_SET);
> - if (fread(buf, phdr->p_filesz, 1, fp) != 1)
> - goto out_free;
> + fseek(fp, elf32 ? hdrs.ehdr32.e_phoff : hdrs.ehdr64.e_phoff, SEEK_SET);
> + if (fread(phdr, phdr_size, 1, fp) != 1)
> + goto out_free;
>
> - ret = read_build_id(buf, phdr->p_filesz, bid, need_swap);
> - if (ret == 0) {
> - ret = bid->size;
> - break;
> - }
> - }
> - } else {
> - Elf64_Ehdr ehdr;
> - Elf64_Phdr *phdr;
> + if (elf32)
> + hdrs.phdr32 = phdr;
> + else
> + hdrs.phdr64 = phdr;
>
> - if (fread(&ehdr, sizeof(ehdr), 1, fp) != 1)
> - goto out;
> + for (i = 0; i < elf32 ? hdrs.ehdr32.e_phnum : hdrs.ehdr64.e_phnum; i++) {
> + size_t p_filesz;
>
> if (need_swap) {
> - ehdr.e_phoff = bswap_64(ehdr.e_phoff);
> - ehdr.e_phentsize = bswap_16(ehdr.e_phentsize);
> - ehdr.e_phnum = bswap_16(ehdr.e_phnum);
> + if (elf32) {
> + hdrs.phdr32[i].p_type = bswap_32(hdrs.phdr32[i].p_type);
> + hdrs.phdr32[i].p_offset = bswap_32(hdrs.phdr32[i].p_offset);
> + hdrs.phdr32[i].p_filesz = bswap_32(hdrs.phdr32[i].p_offset);
> + } else {
> + hdrs.phdr64[i].p_type = bswap_32(hdrs.phdr64[i].p_type);
> + hdrs.phdr64[i].p_offset = bswap_64(hdrs.phdr64[i].p_offset);
> + hdrs.phdr64[i].p_filesz = bswap_64(hdrs.phdr64[i].p_filesz);
> + }
> }
> + if ((elf32 ? hdrs.phdr32[i].p_type : hdrs.phdr64[i].p_type) != PT_NOTE)
> + continue;
>
> - buf_size = ehdr.e_phentsize * ehdr.e_phnum;
> - buf = malloc(buf_size);
> - if (buf == NULL)
> - goto out;
> -
> - fseek(fp, ehdr.e_phoff, SEEK_SET);
> - if (fread(buf, buf_size, 1, fp) != 1)
> - goto out_free;
> -
> - for (i = 0, phdr = buf; i < ehdr.e_phnum; i++, phdr++) {
> + p_filesz = elf32 ? hdrs.phdr32[i].p_filesz : hdrs.phdr64[i].p_filesz;
> + if (p_filesz > buf_size) {
> void *tmp;
> - long offset;
> -
> - if (need_swap) {
> - phdr->p_type = bswap_32(phdr->p_type);
> - phdr->p_offset = bswap_64(phdr->p_offset);
> - phdr->p_filesz = bswap_64(phdr->p_filesz);
> - }
> -
> - if (phdr->p_type != PT_NOTE)
> - continue;
>
> - offset = phdr->p_offset;
> - if (phdr->p_filesz > buf_size) {
> - buf_size = phdr->p_filesz;
> - tmp = realloc(buf, buf_size);
> - if (tmp == NULL)
> - goto out_free;
> - buf = tmp;
> - }
> - fseek(fp, offset, SEEK_SET);
> - if (fread(buf, phdr->p_filesz, 1, fp) != 1)
> + buf_size = p_filesz;
> + tmp = realloc(buf, buf_size);
> + if (tmp == NULL)
> goto out_free;
> + buf = tmp;
> + }
> + fseek(fp, elf32 ? hdrs.phdr32[i].p_offset : hdrs.phdr64[i].p_offset, SEEK_SET);
> + if (fread(buf, p_filesz, 1, fp) != 1)
> + goto out_free;
>
> - ret = read_build_id(buf, phdr->p_filesz, bid, need_swap);
> - if (ret == 0) {
> - ret = bid->size;
> - break;
> - }
> + ret = read_build_id(buf, p_filesz, bid, need_swap);
> + if (ret == 0) {
> + ret = bid->size;
> + break;
> }
> }
> out_free:
> free(buf);
> + free(phdr);
> out:
> fclose(fp);
> return ret;
> --
> 2.49.0.1204.g71687c7c1d-goog
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v1 2/6] perf test demangle-java: Don't segv if demangling fails
2025-05-27 18:06 [PATCH v1 0/6] Various asan and test fixes Ian Rogers
2025-05-27 18:06 ` [PATCH v1 1/6] perf symbol: Fix use-after-free in filename__read_build_id Ian Rogers
@ 2025-05-27 18:06 ` Ian Rogers
2025-05-28 12:49 ` Arnaldo Carvalho de Melo
2025-05-27 18:07 ` [PATCH v1 3/6] perf symbol: Move demangling code out of symbol-elf.c Ian Rogers
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2025-05-27 18:06 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Jiapeng Chong, James Clark, Howard Chu, Weilin Wang,
Stephen Brennan, Andi Kleen, Dmitry Vyukov, linux-perf-users,
linux-kernel
The buffer returned by dso__demangle_sym may be NULL, don't segv in
strcmp if this happens. Currently this happens for NO_LIBELF=1 builds.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/demangle-java-test.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/perf/tests/demangle-java-test.c b/tools/perf/tests/demangle-java-test.c
index ebaf60cdfa99..0fb3e5a4a0ed 100644
--- a/tools/perf/tests/demangle-java-test.c
+++ b/tools/perf/tests/demangle-java-test.c
@@ -30,6 +30,11 @@ static int test__demangle_java(struct test_suite *test __maybe_unused, int subte
for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
buf = dso__demangle_sym(/*dso=*/NULL, /*kmodule=*/0, test_cases[i].mangled);
+ if (!buf) {
+ pr_debug("FAILED to demangle: \"%s\"\n \"%s\"\n", test_cases[i].mangled,
+ test_cases[i].demangled);
+ continue;
+ }
if (strcmp(buf, test_cases[i].demangled)) {
pr_debug("FAILED: %s: %s != %s\n", test_cases[i].mangled,
buf, test_cases[i].demangled);
--
2.49.0.1204.g71687c7c1d-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v1 2/6] perf test demangle-java: Don't segv if demangling fails
2025-05-27 18:06 ` [PATCH v1 2/6] perf test demangle-java: Don't segv if demangling fails Ian Rogers
@ 2025-05-28 12:49 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-28 12:49 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jiapeng Chong, James Clark,
Howard Chu, Weilin Wang, Stephen Brennan, Andi Kleen,
Dmitry Vyukov, linux-perf-users, linux-kernel
On Tue, May 27, 2025 at 11:06:59AM -0700, Ian Rogers wrote:
> The buffer returned by dso__demangle_sym may be NULL, don't segv in
> strcmp if this happens. Currently this happens for NO_LIBELF=1 builds.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/tests/demangle-java-test.c | 5 +++++
> 1 file changed, 5 insertions(+)
Thanks, applied to perf-tools-next,
- Arnaldo
> diff --git a/tools/perf/tests/demangle-java-test.c b/tools/perf/tests/demangle-java-test.c
> index ebaf60cdfa99..0fb3e5a4a0ed 100644
> --- a/tools/perf/tests/demangle-java-test.c
> +++ b/tools/perf/tests/demangle-java-test.c
> @@ -30,6 +30,11 @@ static int test__demangle_java(struct test_suite *test __maybe_unused, int subte
>
> for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
> buf = dso__demangle_sym(/*dso=*/NULL, /*kmodule=*/0, test_cases[i].mangled);
> + if (!buf) {
> + pr_debug("FAILED to demangle: \"%s\"\n \"%s\"\n", test_cases[i].mangled,
> + test_cases[i].demangled);
> + continue;
> + }
> if (strcmp(buf, test_cases[i].demangled)) {
> pr_debug("FAILED: %s: %s != %s\n", test_cases[i].mangled,
> buf, test_cases[i].demangled);
> --
> 2.49.0.1204.g71687c7c1d-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v1 3/6] perf symbol: Move demangling code out of symbol-elf.c
2025-05-27 18:06 [PATCH v1 0/6] Various asan and test fixes Ian Rogers
2025-05-27 18:06 ` [PATCH v1 1/6] perf symbol: Fix use-after-free in filename__read_build_id Ian Rogers
2025-05-27 18:06 ` [PATCH v1 2/6] perf test demangle-java: Don't segv if demangling fails Ian Rogers
@ 2025-05-27 18:07 ` Ian Rogers
2025-05-28 12:47 ` Arnaldo Carvalho de Melo
2025-05-27 18:07 ` [PATCH v1 4/6] perf intel-tpebs: Avoid race when evlist is being deleted Ian Rogers
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2025-05-27 18:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Jiapeng Chong, James Clark, Howard Chu, Weilin Wang,
Stephen Brennan, Andi Kleen, Dmitry Vyukov, linux-perf-users,
linux-kernel
symbol-elf.c is used when building with libelf, symbol-minimal is used
otherwise. There is no reason the demangling code with no dependencies
on libelf is part of symbol-elf.c so move to symbol.c. This allows
demangling tests to pass with NO_LIBELF=1.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/demangle-cxx.h | 2 +
tools/perf/util/symbol-elf.c | 86 -------------------------------
tools/perf/util/symbol-minimal.c | 7 ---
tools/perf/util/symbol.c | 87 ++++++++++++++++++++++++++++++++
4 files changed, 89 insertions(+), 93 deletions(-)
diff --git a/tools/perf/util/demangle-cxx.h b/tools/perf/util/demangle-cxx.h
index 26b5b66c0b4e..9359937a881a 100644
--- a/tools/perf/util/demangle-cxx.h
+++ b/tools/perf/util/demangle-cxx.h
@@ -2,6 +2,8 @@
#ifndef __PERF_DEMANGLE_CXX
#define __PERF_DEMANGLE_CXX 1
+#include <stdbool.h>
+
#ifdef __cplusplus
extern "C" {
#endif
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 8734e8b6cf84..60f37f149a87 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -13,17 +13,12 @@
#include "maps.h"
#include "symbol.h"
#include "symsrc.h"
-#include "demangle-cxx.h"
-#include "demangle-ocaml.h"
-#include "demangle-java.h"
-#include "demangle-rust-v0.h"
#include "machine.h"
#include "vdso.h"
#include "debug.h"
#include "util/copyfile.h"
#include <linux/ctype.h>
#include <linux/kernel.h>
-#include <linux/log2.h>
#include <linux/zalloc.h>
#include <linux/string.h>
#include <symbol/kallsyms.h>
@@ -280,82 +275,6 @@ static int elf_read_program_header(Elf *elf, u64 vaddr, GElf_Phdr *phdr)
return -1;
}
-static bool want_demangle(bool is_kernel_sym)
-{
- return is_kernel_sym ? symbol_conf.demangle_kernel : symbol_conf.demangle;
-}
-
-/*
- * Demangle C++ function signature, typically replaced by demangle-cxx.cpp
- * version.
- */
-#ifndef HAVE_CXA_DEMANGLE_SUPPORT
-char *cxx_demangle_sym(const char *str __maybe_unused, bool params __maybe_unused,
- bool modifiers __maybe_unused)
-{
-#ifdef HAVE_LIBBFD_SUPPORT
- int flags = (params ? DMGL_PARAMS : 0) | (modifiers ? DMGL_ANSI : 0);
-
- return bfd_demangle(NULL, str, flags);
-#elif defined(HAVE_CPLUS_DEMANGLE_SUPPORT)
- int flags = (params ? DMGL_PARAMS : 0) | (modifiers ? DMGL_ANSI : 0);
-
- return cplus_demangle(str, flags);
-#else
- return NULL;
-#endif
-}
-#endif /* !HAVE_CXA_DEMANGLE_SUPPORT */
-
-static char *demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
-{
- struct demangle rust_demangle = {
- .style = DemangleStyleUnknown,
- };
- char *demangled = NULL;
-
- /*
- * We need to figure out if the object was created from C++ sources
- * DWARF DW_compile_unit has this, but we don't always have access
- * to it...
- */
- if (!want_demangle((dso && dso__kernel(dso)) || kmodule))
- return demangled;
-
- rust_demangle_demangle(elf_name, &rust_demangle);
- if (rust_demangle_is_known(&rust_demangle)) {
- /* A rust mangled name. */
- if (rust_demangle.mangled_len == 0)
- return demangled;
-
- for (size_t buf_len = roundup_pow_of_two(rust_demangle.mangled_len * 2);
- buf_len < 1024 * 1024; buf_len += 32) {
- char *tmp = realloc(demangled, buf_len);
-
- if (!tmp) {
- /* Failure to grow output buffer, return what is there. */
- return demangled;
- }
- demangled = tmp;
- if (rust_demangle_display_demangle(&rust_demangle, demangled, buf_len,
- /*alternate=*/true) == OverflowOk)
- return demangled;
- }
- /* Buffer exceeded sensible bounds, return what is there. */
- return demangled;
- }
-
- demangled = cxx_demangle_sym(elf_name, verbose > 0, verbose > 0);
- if (demangled)
- return demangled;
-
- demangled = ocaml_demangle_sym(elf_name);
- if (demangled)
- return demangled;
-
- return java_demangle_sym(elf_name, JAVA_DEMANGLE_NORET);
-}
-
struct rel_info {
u32 nr_entries;
u32 *sorted;
@@ -868,11 +787,6 @@ int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss)
return 0;
}
-char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
-{
- return demangle_sym(dso, kmodule, elf_name);
-}
-
/*
* Align offset to 4 bytes as needed for note name and descriptor data.
*/
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index c9b7a1ca5e52..908f509bbb78 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -355,13 +355,6 @@ void symbol__elf_init(void)
{
}
-char *dso__demangle_sym(struct dso *dso __maybe_unused,
- int kmodule __maybe_unused,
- const char *elf_name __maybe_unused)
-{
- return NULL;
-}
-
bool filename__has_section(const char *filename __maybe_unused, const char *sec __maybe_unused)
{
return false;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index fe801880afea..9d712416f0b6 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -19,6 +19,11 @@
#include "build-id.h"
#include "cap.h"
#include "cpumap.h"
+#include "debug.h"
+#include "demangle-cxx.h"
+#include "demangle-java.h"
+#include "demangle-ocaml.h"
+#include "demangle-rust-v0.h"
#include "dso.h"
#include "util.h" // lsdir()
#include "debug.h"
@@ -36,6 +41,7 @@
#include "header.h"
#include "path.h"
#include <linux/ctype.h>
+#include <linux/log2.h>
#include <linux/zalloc.h>
#include <elf.h>
@@ -2648,3 +2654,84 @@ int symbol__validate_sym_arguments(void)
}
return 0;
}
+
+static bool want_demangle(bool is_kernel_sym)
+{
+ return is_kernel_sym ? symbol_conf.demangle_kernel : symbol_conf.demangle;
+}
+
+/*
+ * Demangle C++ function signature, typically replaced by demangle-cxx.cpp
+ * version.
+ */
+#ifndef HAVE_CXA_DEMANGLE_SUPPORT
+char *cxx_demangle_sym(const char *str __maybe_unused, bool params __maybe_unused,
+ bool modifiers __maybe_unused)
+{
+#ifdef HAVE_LIBBFD_SUPPORT
+ int flags = (params ? DMGL_PARAMS : 0) | (modifiers ? DMGL_ANSI : 0);
+
+ return bfd_demangle(NULL, str, flags);
+#elif defined(HAVE_CPLUS_DEMANGLE_SUPPORT)
+ int flags = (params ? DMGL_PARAMS : 0) | (modifiers ? DMGL_ANSI : 0);
+
+ return cplus_demangle(str, flags);
+#else
+ return NULL;
+#endif
+}
+#endif /* !HAVE_CXA_DEMANGLE_SUPPORT */
+
+static char *demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
+{
+ struct demangle rust_demangle = {
+ .style = DemangleStyleUnknown,
+ };
+ char *demangled = NULL;
+
+ /*
+ * We need to figure out if the object was created from C++ sources
+ * DWARF DW_compile_unit has this, but we don't always have access
+ * to it...
+ */
+ if (!want_demangle((dso && dso__kernel(dso)) || kmodule))
+ return demangled;
+
+ rust_demangle_demangle(elf_name, &rust_demangle);
+ if (rust_demangle_is_known(&rust_demangle)) {
+ /* A rust mangled name. */
+ if (rust_demangle.mangled_len == 0)
+ return demangled;
+
+ for (size_t buf_len = roundup_pow_of_two(rust_demangle.mangled_len * 2);
+ buf_len < 1024 * 1024; buf_len += 32) {
+ char *tmp = realloc(demangled, buf_len);
+
+ if (!tmp) {
+ /* Failure to grow output buffer, return what is there. */
+ return demangled;
+ }
+ demangled = tmp;
+ if (rust_demangle_display_demangle(&rust_demangle, demangled, buf_len,
+ /*alternate=*/true) == OverflowOk)
+ return demangled;
+ }
+ /* Buffer exceeded sensible bounds, return what is there. */
+ return demangled;
+ }
+
+ demangled = cxx_demangle_sym(elf_name, verbose > 0, verbose > 0);
+ if (demangled)
+ return demangled;
+
+ demangled = ocaml_demangle_sym(elf_name);
+ if (demangled)
+ return demangled;
+
+ return java_demangle_sym(elf_name, JAVA_DEMANGLE_NORET);
+}
+
+char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
+{
+ return demangle_sym(dso, kmodule, elf_name);
+}
--
2.49.0.1204.g71687c7c1d-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v1 3/6] perf symbol: Move demangling code out of symbol-elf.c
2025-05-27 18:07 ` [PATCH v1 3/6] perf symbol: Move demangling code out of symbol-elf.c Ian Rogers
@ 2025-05-28 12:47 ` Arnaldo Carvalho de Melo
2025-05-28 18:03 ` Ian Rogers
0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-28 12:47 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jiapeng Chong, James Clark,
Howard Chu, Weilin Wang, Stephen Brennan, Andi Kleen,
Dmitry Vyukov, linux-perf-users, linux-kernel
On Tue, May 27, 2025 at 11:07:00AM -0700, Ian Rogers wrote:
> symbol-elf.c is used when building with libelf, symbol-minimal is used
> otherwise. There is no reason the demangling code with no dependencies
> on libelf is part of symbol-elf.c so move to symbol.c. This allows
> demangling tests to pass with NO_LIBELF=1.
At this point:
⬢ [acme@toolbx perf-tools-next]$ alias m='rm -rf ~/libexec/perf-core/ ; make -k O=/tmp/build/$(basename $PWD)/ -C tools/perf install-bin && perf test python && cat /tmp/build/$(basename $PWD)/feature/test-all.make.output'
⬢ [acme@toolbx perf-tools-next]$ m
rm: cannot remove '/home/acme/libexec/perf-core/scripts/python/Perf-Trace-Util/lib/Perf/Trace/__pycache__/Core.cpython-313.pyc': Permission denied
make: Entering directory '/home/acme/git/perf-tools-next/tools/perf'
BUILD: Doing 'make -j32' parallel build
Warning: Kernel ABI header differences:
diff -u tools/arch/arm64/include/asm/cputype.h arch/arm64/include/asm/cputype.h
Auto-detecting system features:
... libdw: [ on ]
... glibc: [ on ]
... libelf: [ on ]
... libnuma: [ on ]
... numa_num_possible_cpus: [ on ]
... libperl: [ on ]
... libpython: [ on ]
... libcrypto: [ on ]
... libcapstone: [ on ]
... llvm-perf: [ on ]
... zlib: [ on ]
... lzma: [ on ]
... get_cpuid: [ on ]
... bpf: [ on ]
... libaio: [ on ]
... libzstd: [ on ]
INSTALL libsubcmd_headers
INSTALL libperf_headers
INSTALL libapi_headers
INSTALL libsymbol_headers
INSTALL libbpf_headers
AR /tmp/build/perf-tools-next/libpmu-events.a
CC /tmp/build/perf-tools-next/util/symbol-elf.o
util/symbol-elf.c: In function ‘get_plt_got_name’:
util/symbol-elf.c:563:21: error: implicit declaration of function ‘demangle_sym’; did you mean ‘dso__demangle_sym’? [-Wimplicit-function-declaration]
563 | demangled = demangle_sym(di->dso, 0, sym_name);
| ^~~~~~~~~~~~
| dso__demangle_sym
util/symbol-elf.c:563:19: error: assignment to ‘char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
563 | demangled = demangle_sym(di->dso, 0, sym_name);
| ^
util/symbol-elf.c: In function ‘dso__synthesize_plt_symbols’:
util/symbol-elf.c:761:27: error: assignment to ‘char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
761 | demangled = demangle_sym(dso, 0, elf_name);
| ^
util/symbol-elf.c: In function ‘dso__load_sym_internal’:
util/symbol-elf.c:1778:27: error: assignment to ‘char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
1778 | demangled = demangle_sym(dso, kmodule, elf_name);
| ^
make[4]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:85: /tmp/build/perf-tools-next/util/symbol-elf.o] Error 1
make[3]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:142: util] Error 2
make[2]: *** [Makefile.perf:798: /tmp/build/perf-tools-next/perf-util-in.o] Error 2
make[1]: *** [Makefile.perf:290: sub-make] Error 2
make: *** [Makefile:119: install-bin] Error 2
make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
⬢ [acme@toolbx perf-tools-next]$
- Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v1 3/6] perf symbol: Move demangling code out of symbol-elf.c
2025-05-28 12:47 ` Arnaldo Carvalho de Melo
@ 2025-05-28 18:03 ` Ian Rogers
0 siblings, 0 replies; 13+ messages in thread
From: Ian Rogers @ 2025-05-28 18:03 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jiapeng Chong, James Clark,
Howard Chu, Weilin Wang, Stephen Brennan, Andi Kleen,
Dmitry Vyukov, linux-perf-users, linux-kernel
On Wed, May 28, 2025 at 5:47 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Tue, May 27, 2025 at 11:07:00AM -0700, Ian Rogers wrote:
> > symbol-elf.c is used when building with libelf, symbol-minimal is used
> > otherwise. There is no reason the demangling code with no dependencies
> > on libelf is part of symbol-elf.c so move to symbol.c. This allows
> > demangling tests to pass with NO_LIBELF=1.
>
> At this point:
>
> ⬢ [acme@toolbx perf-tools-next]$ alias m='rm -rf ~/libexec/perf-core/ ; make -k O=/tmp/build/$(basename $PWD)/ -C tools/perf install-bin && perf test python && cat /tmp/build/$(basename $PWD)/feature/test-all.make.output'
> ⬢ [acme@toolbx perf-tools-next]$ m
> rm: cannot remove '/home/acme/libexec/perf-core/scripts/python/Perf-Trace-Util/lib/Perf/Trace/__pycache__/Core.cpython-313.pyc': Permission denied
> make: Entering directory '/home/acme/git/perf-tools-next/tools/perf'
> BUILD: Doing 'make -j32' parallel build
> Warning: Kernel ABI header differences:
> diff -u tools/arch/arm64/include/asm/cputype.h arch/arm64/include/asm/cputype.h
>
> Auto-detecting system features:
> ... libdw: [ on ]
> ... glibc: [ on ]
> ... libelf: [ on ]
> ... libnuma: [ on ]
> ... numa_num_possible_cpus: [ on ]
> ... libperl: [ on ]
> ... libpython: [ on ]
> ... libcrypto: [ on ]
> ... libcapstone: [ on ]
> ... llvm-perf: [ on ]
> ... zlib: [ on ]
> ... lzma: [ on ]
> ... get_cpuid: [ on ]
> ... bpf: [ on ]
> ... libaio: [ on ]
> ... libzstd: [ on ]
>
> INSTALL libsubcmd_headers
> INSTALL libperf_headers
> INSTALL libapi_headers
> INSTALL libsymbol_headers
> INSTALL libbpf_headers
> AR /tmp/build/perf-tools-next/libpmu-events.a
> CC /tmp/build/perf-tools-next/util/symbol-elf.o
> util/symbol-elf.c: In function ‘get_plt_got_name’:
> util/symbol-elf.c:563:21: error: implicit declaration of function ‘demangle_sym’; did you mean ‘dso__demangle_sym’? [-Wimplicit-function-declaration]
> 563 | demangled = demangle_sym(di->dso, 0, sym_name);
> | ^~~~~~~~~~~~
> | dso__demangle_sym
> util/symbol-elf.c:563:19: error: assignment to ‘char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
> 563 | demangled = demangle_sym(di->dso, 0, sym_name);
> | ^
> util/symbol-elf.c: In function ‘dso__synthesize_plt_symbols’:
> util/symbol-elf.c:761:27: error: assignment to ‘char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
> 761 | demangled = demangle_sym(dso, 0, elf_name);
> | ^
> util/symbol-elf.c: In function ‘dso__load_sym_internal’:
> util/symbol-elf.c:1778:27: error: assignment to ‘char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
> 1778 | demangled = demangle_sym(dso, kmodule, elf_name);
> | ^
> make[4]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:85: /tmp/build/perf-tools-next/util/symbol-elf.o] Error 1
> make[3]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:142: util] Error 2
> make[2]: *** [Makefile.perf:798: /tmp/build/perf-tools-next/perf-util-in.o] Error 2
> make[1]: *** [Makefile.perf:290: sub-make] Error 2
> make: *** [Makefile:119: install-bin] Error 2
> make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
> ⬢ [acme@toolbx perf-tools-next]$
Sorry, was so focussed on getting the sanitizers clean I'd missed the
non-sanitizer build. Will fix in v3.
Thanks,
Ian
> - Arnaldo
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v1 4/6] perf intel-tpebs: Avoid race when evlist is being deleted
2025-05-27 18:06 [PATCH v1 0/6] Various asan and test fixes Ian Rogers
` (2 preceding siblings ...)
2025-05-27 18:07 ` [PATCH v1 3/6] perf symbol: Move demangling code out of symbol-elf.c Ian Rogers
@ 2025-05-27 18:07 ` Ian Rogers
2025-05-28 12:51 ` Arnaldo Carvalho de Melo
2025-05-27 18:07 ` [PATCH v1 5/6] perf test intel-pt: Skip jitdump test if no libelf Ian Rogers
2025-05-27 18:07 ` [PATCH v1 6/6] perf test trace_summary: Skip --bpf-summary tests if no libbpf Ian Rogers
5 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2025-05-27 18:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Jiapeng Chong, James Clark, Howard Chu, Weilin Wang,
Stephen Brennan, Andi Kleen, Dmitry Vyukov, linux-perf-users,
linux-kernel
Reading through the evsel->evlist may seg fault if a sample arrives
when the evlist is being deleted. Detect this case and ignore samples
arriving when the evlist is being deleted.
Fixes: bcfab08db7fb ("perf intel-tpebs: Filter non-workload samples")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/intel-tpebs.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c
index 4ad4bc118ea5..3b92ebf5c112 100644
--- a/tools/perf/util/intel-tpebs.c
+++ b/tools/perf/util/intel-tpebs.c
@@ -162,9 +162,17 @@ static bool is_child_pid(pid_t parent, pid_t child)
static bool should_ignore_sample(const struct perf_sample *sample, const struct tpebs_retire_lat *t)
{
- pid_t workload_pid = t->evsel->evlist->workload.pid;
- pid_t sample_pid = sample->pid;
+ pid_t workload_pid, sample_pid = sample->pid;
+ /*
+ * During evlist__purge the evlist will be removed prior to the
+ * evsel__exit calling evsel__tpebs_close and taking the
+ * tpebs_mtx. Avoid a segfault by ignoring samples in this case.
+ */
+ if (t->evsel->evlist == NULL)
+ return true;
+
+ workload_pid = t->evsel->evlist->workload.pid;
if (workload_pid < 0 || workload_pid == sample_pid)
return false;
--
2.49.0.1204.g71687c7c1d-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v1 4/6] perf intel-tpebs: Avoid race when evlist is being deleted
2025-05-27 18:07 ` [PATCH v1 4/6] perf intel-tpebs: Avoid race when evlist is being deleted Ian Rogers
@ 2025-05-28 12:51 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-28 12:51 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jiapeng Chong, James Clark,
Howard Chu, Weilin Wang, Stephen Brennan, Andi Kleen,
Dmitry Vyukov, linux-perf-users, linux-kernel
On Tue, May 27, 2025 at 11:07:01AM -0700, Ian Rogers wrote:
> Reading through the evsel->evlist may seg fault if a sample arrives
> when the evlist is being deleted. Detect this case and ignore samples
> arriving when the evlist is being deleted.
Thanks, applied to perf-tools-next,
- Arnaldo
> Fixes: bcfab08db7fb ("perf intel-tpebs: Filter non-workload samples")
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/intel-tpebs.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c
> index 4ad4bc118ea5..3b92ebf5c112 100644
> --- a/tools/perf/util/intel-tpebs.c
> +++ b/tools/perf/util/intel-tpebs.c
> @@ -162,9 +162,17 @@ static bool is_child_pid(pid_t parent, pid_t child)
>
> static bool should_ignore_sample(const struct perf_sample *sample, const struct tpebs_retire_lat *t)
> {
> - pid_t workload_pid = t->evsel->evlist->workload.pid;
> - pid_t sample_pid = sample->pid;
> + pid_t workload_pid, sample_pid = sample->pid;
>
> + /*
> + * During evlist__purge the evlist will be removed prior to the
> + * evsel__exit calling evsel__tpebs_close and taking the
> + * tpebs_mtx. Avoid a segfault by ignoring samples in this case.
> + */
> + if (t->evsel->evlist == NULL)
> + return true;
> +
> + workload_pid = t->evsel->evlist->workload.pid;
> if (workload_pid < 0 || workload_pid == sample_pid)
> return false;
>
> --
> 2.49.0.1204.g71687c7c1d-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v1 5/6] perf test intel-pt: Skip jitdump test if no libelf
2025-05-27 18:06 [PATCH v1 0/6] Various asan and test fixes Ian Rogers
` (3 preceding siblings ...)
2025-05-27 18:07 ` [PATCH v1 4/6] perf intel-tpebs: Avoid race when evlist is being deleted Ian Rogers
@ 2025-05-27 18:07 ` Ian Rogers
2025-05-28 12:52 ` Arnaldo Carvalho de Melo
2025-05-27 18:07 ` [PATCH v1 6/6] perf test trace_summary: Skip --bpf-summary tests if no libbpf Ian Rogers
5 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2025-05-27 18:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Jiapeng Chong, James Clark, Howard Chu, Weilin Wang,
Stephen Brennan, Andi Kleen, Dmitry Vyukov, linux-perf-users,
linux-kernel
jitdump support is only present if building with libelf. Skip the
intel-pt jitdump test if perf isn't compiled with libelf support.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/shell/test_intel_pt.sh | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index f3a9a040bacc..32a9b8dcb200 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -288,6 +288,11 @@ test_jitdump()
jitdump_incl_dir="${script_dir}/../../util"
jitdump_h="${jitdump_incl_dir}/jitdump.h"
+ if ! perf check feature -q libelf ; then
+ echo "SKIP: libelf is needed for jitdump"
+ return 2
+ fi
+
if [ ! -e "${jitdump_h}" ] ; then
echo "SKIP: Include file jitdump.h not found"
return 2
--
2.49.0.1204.g71687c7c1d-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v1 5/6] perf test intel-pt: Skip jitdump test if no libelf
2025-05-27 18:07 ` [PATCH v1 5/6] perf test intel-pt: Skip jitdump test if no libelf Ian Rogers
@ 2025-05-28 12:52 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-28 12:52 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jiapeng Chong, James Clark,
Howard Chu, Weilin Wang, Stephen Brennan, Andi Kleen,
Dmitry Vyukov, linux-perf-users, linux-kernel
On Tue, May 27, 2025 at 11:07:02AM -0700, Ian Rogers wrote:
> jitdump support is only present if building with libelf. Skip the
> intel-pt jitdump test if perf isn't compiled with libelf support.
Thanks, applied to perf-tools-next,
- Arnaldo
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/tests/shell/test_intel_pt.sh | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
> index f3a9a040bacc..32a9b8dcb200 100755
> --- a/tools/perf/tests/shell/test_intel_pt.sh
> +++ b/tools/perf/tests/shell/test_intel_pt.sh
> @@ -288,6 +288,11 @@ test_jitdump()
> jitdump_incl_dir="${script_dir}/../../util"
> jitdump_h="${jitdump_incl_dir}/jitdump.h"
>
> + if ! perf check feature -q libelf ; then
> + echo "SKIP: libelf is needed for jitdump"
> + return 2
> + fi
> +
> if [ ! -e "${jitdump_h}" ] ; then
> echo "SKIP: Include file jitdump.h not found"
> return 2
> --
> 2.49.0.1204.g71687c7c1d-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v1 6/6] perf test trace_summary: Skip --bpf-summary tests if no libbpf
2025-05-27 18:06 [PATCH v1 0/6] Various asan and test fixes Ian Rogers
` (4 preceding siblings ...)
2025-05-27 18:07 ` [PATCH v1 5/6] perf test intel-pt: Skip jitdump test if no libelf Ian Rogers
@ 2025-05-27 18:07 ` Ian Rogers
5 siblings, 0 replies; 13+ messages in thread
From: Ian Rogers @ 2025-05-27 18:07 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Jiapeng Chong, James Clark, Howard Chu, Weilin Wang,
Stephen Brennan, Andi Kleen, Dmitry Vyukov, linux-perf-users,
linux-kernel
If perf is built without libbpf (e.g. NO_LIBBPF=1) then the
--bpf-summary perf trace tests will fail. Skip the tests as this is
expected behavior.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/shell/trace_summary.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/perf/tests/shell/trace_summary.sh b/tools/perf/tests/shell/trace_summary.sh
index 49766524dc21..f9bb7f9388be 100755
--- a/tools/perf/tests/shell/trace_summary.sh
+++ b/tools/perf/tests/shell/trace_summary.sh
@@ -53,6 +53,12 @@ test_perf_trace "-as --summary-mode=thread --no-bpf-summary"
# summary only for system wide - total summary mode
test_perf_trace "-as --summary-mode=total --no-bpf-summary"
+if ! perf check feature -q bpf; then
+ echo "Skip --bpf-summary tests as perf built without libbpf"
+ rm -f ${OUTPUT}
+ exit 2
+fi
+
# summary only for system wide - per-thread summary with BPF
test_perf_trace "-as --summary-mode=thread --bpf-summary"
--
2.49.0.1204.g71687c7c1d-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread