linux-kernel.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).