All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf top: fix overflow in elf_sec__is_text
@ 2021-06-21 22:21 Riccardo Mancini
  2021-06-22 19:46 ` Namhyung Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Riccardo Mancini @ 2021-06-21 22:21 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ian Rogers, Riccardo Mancini, Jiri Slaby,
	Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Remi Bernon, Fabian Hemmer, linux-perf-users,
	linux-kernel

ASan reports a heap-buffer-overflow in elf_sec__is_text when using perf-top.
The bug is caused by the fact that secstrs is built from runtime_ss, while
shdr is built from syms_ss if shdr.sh_type != SHT_NOBITS. Therefore, they
point to two different ELF files.

This patch renames secstrs to secstrs_run and adds secstrs_sym, so that
the correct secstrs is chosen depending on shdr.sh_type.

$ ASAN_OPTIONS=abort_on_error=1:disable_coredump=0:unmap_shadow_on_exit=1 ./perf top
=================================================================
==363148==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61300009add6 at pc 0x00000049875c bp 0x7f4f56446440 sp 0x7f4f56445bf0
READ of size 1 at 0x61300009add6 thread T6
    #0 0x49875b in StrstrCheck(void*, char*, char const*, char const*) (/home/user/linux/tools/perf/perf+0x49875b)
    #1 0x4d13a2 in strstr (/home/user/linux/tools/perf/perf+0x4d13a2)
    #2 0xacae36 in elf_sec__is_text /home/user/linux/tools/perf/util/symbol-elf.c:176:9
    #3 0xac3ec9 in elf_sec__filter /home/user/linux/tools/perf/util/symbol-elf.c:187:9
    #4 0xac2c3d in dso__load_sym /home/user/linux/tools/perf/util/symbol-elf.c:1254:20
    #5 0x883981 in dso__load /home/user/linux/tools/perf/util/symbol.c:1897:9
    #6 0x8e6248 in map__load /home/user/linux/tools/perf/util/map.c:332:7
    #7 0x8e66e5 in map__find_symbol /home/user/linux/tools/perf/util/map.c:366:6
    #8 0x7f8278 in machine__resolve /home/user/linux/tools/perf/util/event.c:707:13
    #9 0x5f3d1a in perf_event__process_sample /home/user/linux/tools/perf/builtin-top.c:773:6
    #10 0x5f30e4 in deliver_event /home/user/linux/tools/perf/builtin-top.c:1197:3
    #11 0x908a72 in do_flush /home/user/linux/tools/perf/util/ordered-events.c:244:9
    #12 0x905fae in __ordered_events__flush /home/user/linux/tools/perf/util/ordered-events.c:323:8
    #13 0x9058db in ordered_events__flush /home/user/linux/tools/perf/util/ordered-events.c:341:9
    #14 0x5f19b1 in process_thread /home/user/linux/tools/perf/builtin-top.c:1109:7
    #15 0x7f4f6a21a298 in start_thread /usr/src/debug/glibc-2.33-16.fc34.x86_64/nptl/pthread_create.c:481:8
    #16 0x7f4f697d0352 in clone ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

0x61300009add6 is located 10 bytes to the right of 332-byte region [0x61300009ac80,0x61300009adcc)
allocated by thread T6 here:
    #0 0x4f3f7f in malloc (/home/user/linux/tools/perf/perf+0x4f3f7f)
    #1 0x7f4f6a0a88d9  (/lib64/libelf.so.1+0xa8d9)

Thread T6 created by T0 here:
    #0 0x464856 in pthread_create (/home/user/linux/tools/perf/perf+0x464856)
    #1 0x5f06e0 in __cmd_top /home/user/linux/tools/perf/builtin-top.c:1309:6
    #2 0x5ef19f in cmd_top /home/user/linux/tools/perf/builtin-top.c:1762:11
    #3 0x7b28c0 in run_builtin /home/user/linux/tools/perf/perf.c:313:11
    #4 0x7b119f in handle_internal_command /home/user/linux/tools/perf/perf.c:365:8
    #5 0x7b2423 in run_argv /home/user/linux/tools/perf/perf.c:409:2
    #6 0x7b0c19 in main /home/user/linux/tools/perf/perf.c:539:3
    #7 0x7f4f696f7b74 in __libc_start_main /usr/src/debug/glibc-2.33-16.fc34.x86_64/csu/../csu/libc-start.c:332:16

SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/user/linux/tools/perf/perf+0x49875b) in StrstrCheck(void*, char*, char const*, char const*)
Shadow bytes around the buggy address:
  0x0c268000b560: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c268000b570: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c268000b580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c268000b590: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c268000b5a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c268000b5b0: 00 00 00 00 00 00 00 00 00 04[fa]fa fa fa fa fa
  0x0c268000b5c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c268000b5d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c268000b5e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c268000b5f0: 07 fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c268000b600: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==363148==ABORTING

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
Suggested-by: Jiri Slaby <jirislaby@kernel.org>
---
 tools/perf/util/symbol-elf.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 4c56aa837434..7a6e38bf87b0 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1081,7 +1081,7 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 	struct maps *kmaps = kmap ? map__kmaps(map) : NULL;
 	struct map *curr_map = map;
 	struct dso *curr_dso = dso;
-	Elf_Data *symstrs, *secstrs;
+	Elf_Data *symstrs, *secstrs, *secstrs_run, *secstrs_sym;
 	uint32_t nr_syms;
 	int err = -1;
 	uint32_t idx;
@@ -1150,8 +1150,16 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 	if (sec_strndx == NULL)
 		goto out_elf_end;
 
-	secstrs = elf_getdata(sec_strndx, NULL);
-	if (secstrs == NULL)
+	secstrs_run = elf_getdata(sec_strndx, NULL);
+	if (secstrs_run == NULL)
+		goto out_elf_end;
+
+	sec_strndx = elf_getscn(elf, ehdr.e_shstrndx);
+	if (sec_strndx == NULL)
+		goto out_elf_end;
+
+	secstrs_sym = elf_getdata(sec_strndx, NULL);
+	if (secstrs_sym == NULL)
 		goto out_elf_end;
 
 	nr_syms = shdr.sh_size / shdr.sh_entsize;
@@ -1237,6 +1245,8 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 
 		gelf_getshdr(sec, &shdr);
 
+		secstrs = secstrs_sym;
+
 		/*
 		 * We have to fallback to runtime when syms' section header has
 		 * NOBITS set. NOBITS results in file offset (sh_offset) not
@@ -1249,6 +1259,7 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 				goto out_elf_end;
 
 			gelf_getshdr(sec, &shdr);
+			secstrs = secstrs_run;
 		}
 
 		if (is_label && !elf_sec__filter(&shdr, secstrs))
-- 
2.23.0


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

* Re: [PATCH] perf top: fix overflow in elf_sec__is_text
  2021-06-21 22:21 [PATCH] perf top: fix overflow in elf_sec__is_text Riccardo Mancini
@ 2021-06-22 19:46 ` Namhyung Kim
  2021-07-05 17:56   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2021-06-22 19:46 UTC (permalink / raw)
  To: Riccardo Mancini
  Cc: Arnaldo Carvalho de Melo, Ian Rogers, Jiri Slaby, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Remi Bernon, Fabian Hemmer, linux-perf-users, linux-kernel

Hi Riccardo,

On Mon, Jun 21, 2021 at 3:22 PM Riccardo Mancini <rickyman7@gmail.com> wrote:
>
> ASan reports a heap-buffer-overflow in elf_sec__is_text when using perf-top.
> The bug is caused by the fact that secstrs is built from runtime_ss, while
> shdr is built from syms_ss if shdr.sh_type != SHT_NOBITS. Therefore, they
> point to two different ELF files.
>
> This patch renames secstrs to secstrs_run and adds secstrs_sym, so that
> the correct secstrs is chosen depending on shdr.sh_type.
>
> $ ASAN_OPTIONS=abort_on_error=1:disable_coredump=0:unmap_shadow_on_exit=1 ./perf top
> =================================================================
> ==363148==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61300009add6 at pc 0x00000049875c bp 0x7f4f56446440 sp 0x7f4f56445bf0
> READ of size 1 at 0x61300009add6 thread T6
>     #0 0x49875b in StrstrCheck(void*, char*, char const*, char const*) (/home/user/linux/tools/perf/perf+0x49875b)
>     #1 0x4d13a2 in strstr (/home/user/linux/tools/perf/perf+0x4d13a2)
>     #2 0xacae36 in elf_sec__is_text /home/user/linux/tools/perf/util/symbol-elf.c:176:9
>     #3 0xac3ec9 in elf_sec__filter /home/user/linux/tools/perf/util/symbol-elf.c:187:9
>     #4 0xac2c3d in dso__load_sym /home/user/linux/tools/perf/util/symbol-elf.c:1254:20
>     #5 0x883981 in dso__load /home/user/linux/tools/perf/util/symbol.c:1897:9
>     #6 0x8e6248 in map__load /home/user/linux/tools/perf/util/map.c:332:7
>     #7 0x8e66e5 in map__find_symbol /home/user/linux/tools/perf/util/map.c:366:6
>     #8 0x7f8278 in machine__resolve /home/user/linux/tools/perf/util/event.c:707:13
>     #9 0x5f3d1a in perf_event__process_sample /home/user/linux/tools/perf/builtin-top.c:773:6
>     #10 0x5f30e4 in deliver_event /home/user/linux/tools/perf/builtin-top.c:1197:3
>     #11 0x908a72 in do_flush /home/user/linux/tools/perf/util/ordered-events.c:244:9
>     #12 0x905fae in __ordered_events__flush /home/user/linux/tools/perf/util/ordered-events.c:323:8
>     #13 0x9058db in ordered_events__flush /home/user/linux/tools/perf/util/ordered-events.c:341:9
>     #14 0x5f19b1 in process_thread /home/user/linux/tools/perf/builtin-top.c:1109:7
>     #15 0x7f4f6a21a298 in start_thread /usr/src/debug/glibc-2.33-16.fc34.x86_64/nptl/pthread_create.c:481:8
>     #16 0x7f4f697d0352 in clone ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
>
> 0x61300009add6 is located 10 bytes to the right of 332-byte region [0x61300009ac80,0x61300009adcc)
> allocated by thread T6 here:
>     #0 0x4f3f7f in malloc (/home/user/linux/tools/perf/perf+0x4f3f7f)
>     #1 0x7f4f6a0a88d9  (/lib64/libelf.so.1+0xa8d9)
>
> Thread T6 created by T0 here:
>     #0 0x464856 in pthread_create (/home/user/linux/tools/perf/perf+0x464856)
>     #1 0x5f06e0 in __cmd_top /home/user/linux/tools/perf/builtin-top.c:1309:6
>     #2 0x5ef19f in cmd_top /home/user/linux/tools/perf/builtin-top.c:1762:11
>     #3 0x7b28c0 in run_builtin /home/user/linux/tools/perf/perf.c:313:11
>     #4 0x7b119f in handle_internal_command /home/user/linux/tools/perf/perf.c:365:8
>     #5 0x7b2423 in run_argv /home/user/linux/tools/perf/perf.c:409:2
>     #6 0x7b0c19 in main /home/user/linux/tools/perf/perf.c:539:3
>     #7 0x7f4f696f7b74 in __libc_start_main /usr/src/debug/glibc-2.33-16.fc34.x86_64/csu/../csu/libc-start.c:332:16
>
> SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/user/linux/tools/perf/perf+0x49875b) in StrstrCheck(void*, char*, char const*, char const*)
> Shadow bytes around the buggy address:
>   0x0c268000b560: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c268000b570: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c268000b580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c268000b590: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x0c268000b5a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> =>0x0c268000b5b0: 00 00 00 00 00 00 00 00 00 04[fa]fa fa fa fa fa
>   0x0c268000b5c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c268000b5d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x0c268000b5e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x0c268000b5f0: 07 fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c268000b600: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> Shadow byte legend (one shadow byte represents 8 application bytes):
>   Addressable:           00
>   Partially addressable: 01 02 03 04 05 06 07
>   Heap left redzone:       fa
>   Freed heap region:       fd
>   Stack left redzone:      f1
>   Stack mid redzone:       f2
>   Stack right redzone:     f3
>   Stack after return:      f5
>   Stack use after scope:   f8
>   Global redzone:          f9
>   Global init order:       f6
>   Poisoned by user:        f7
>   Container overflow:      fc
>   Array cookie:            ac
>   Intra object redzone:    bb
>   ASan internal:           fe
>   Left alloca redzone:     ca
>   Right alloca redzone:    cb
>   Shadow gap:              cc
> ==363148==ABORTING
>
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> Suggested-by: Jiri Slaby <jirislaby@kernel.org>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  tools/perf/util/symbol-elf.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 4c56aa837434..7a6e38bf87b0 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1081,7 +1081,7 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
>         struct maps *kmaps = kmap ? map__kmaps(map) : NULL;
>         struct map *curr_map = map;
>         struct dso *curr_dso = dso;
> -       Elf_Data *symstrs, *secstrs;
> +       Elf_Data *symstrs, *secstrs, *secstrs_run, *secstrs_sym;
>         uint32_t nr_syms;
>         int err = -1;
>         uint32_t idx;
> @@ -1150,8 +1150,16 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
>         if (sec_strndx == NULL)
>                 goto out_elf_end;
>
> -       secstrs = elf_getdata(sec_strndx, NULL);
> -       if (secstrs == NULL)
> +       secstrs_run = elf_getdata(sec_strndx, NULL);
> +       if (secstrs_run == NULL)
> +               goto out_elf_end;
> +
> +       sec_strndx = elf_getscn(elf, ehdr.e_shstrndx);
> +       if (sec_strndx == NULL)
> +               goto out_elf_end;
> +
> +       secstrs_sym = elf_getdata(sec_strndx, NULL);
> +       if (secstrs_sym == NULL)
>                 goto out_elf_end;
>
>         nr_syms = shdr.sh_size / shdr.sh_entsize;
> @@ -1237,6 +1245,8 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
>
>                 gelf_getshdr(sec, &shdr);
>
> +               secstrs = secstrs_sym;
> +
>                 /*
>                  * We have to fallback to runtime when syms' section header has
>                  * NOBITS set. NOBITS results in file offset (sh_offset) not
> @@ -1249,6 +1259,7 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
>                                 goto out_elf_end;
>
>                         gelf_getshdr(sec, &shdr);
> +                       secstrs = secstrs_run;
>                 }
>
>                 if (is_label && !elf_sec__filter(&shdr, secstrs))
> --
> 2.23.0
>

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

* Re: [PATCH] perf top: fix overflow in elf_sec__is_text
  2021-06-22 19:46 ` Namhyung Kim
@ 2021-07-05 17:56   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-05 17:56 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Riccardo Mancini, Ian Rogers, Jiri Slaby, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Remi Bernon, Fabian Hemmer, linux-perf-users, linux-kernel

Em Tue, Jun 22, 2021 at 12:46:38PM -0700, Namhyung Kim escreveu:
> Hi Riccardo,
> 
> On Mon, Jun 21, 2021 at 3:22 PM Riccardo Mancini <rickyman7@gmail.com> wrote:
> >
> > ASan reports a heap-buffer-overflow in elf_sec__is_text when using perf-top.
> > The bug is caused by the fact that secstrs is built from runtime_ss, while
> > shdr is built from syms_ss if shdr.sh_type != SHT_NOBITS. Therefore, they
> > point to two different ELF files.
> >
> > This patch renames secstrs to secstrs_run and adds secstrs_sym, so that
> > the correct secstrs is chosen depending on shdr.sh_type.

Thanks, applied.

- Arnaldo


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

end of thread, other threads:[~2021-07-05 17:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-21 22:21 [PATCH] perf top: fix overflow in elf_sec__is_text Riccardo Mancini
2021-06-22 19:46 ` Namhyung Kim
2021-07-05 17:56   ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.