linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf tools: Fix crash in build_id_cache__kallsyms_path()
@ 2016-06-07  3:54 Wang Nan
  2016-06-08  8:44 ` [tip:perf/core] " tip-bot for Wang Nan
  2016-06-11  1:28 ` [PATCH] " Masami Hiramatsu
  0 siblings, 2 replies; 3+ messages in thread
From: Wang Nan @ 2016-06-07  3:54 UTC (permalink / raw)
  To: acme, mhiramat
  Cc: pi3orama, lizefan, linux-kernel, Wang Nan, Arnaldo Carvalho de Melo

build_id_cache__kallsyms_path() accept string buffer but also alloc buffer
by itself using asnprintf. Unfortunately, the only user of it passes it
a stack-allocated buffer. Freeing it causes crash like this:

 $ perf script
 *** Error in `/home/wangnan/perf': free(): invalid pointer: 0x00007fffffff9630 ***
 ======= Backtrace: =========
 lib64/libc.so.6(+0x6eeef)[0x7ffff5dbaeef]
 lib64/libc.so.6(+0x78cae)[0x7ffff5dc4cae]
 lib64/libc.so.6(+0x79987)[0x7ffff5dc5987]
 /home/w00229757/perf(build_id_cache__kallsyms_path+0x6b)[0x49681b]
 /home/w00229757/perf[0x4bdd40]
 /home/w00229757/perf(dso__load+0xa3a)[0x4c048a]
 /home/w00229757/perf(map__load+0x6f)[0x4d561f]
 /home/w00229757/perf(thread__find_addr_map+0x235)[0x49e935]
 /home/w00229757/perf(machine__resolve+0x7d)[0x49ec6d]
 /home/w00229757/perf[0x4555a8]
 /home/w00229757/perf[0x4d9507]
 /home/w00229757/perf[0x4d9e80]
 /home/w00229757/perf(ordered_events__flush+0x354)[0x4dd444]
 /home/w00229757/perf(perf_session__process_events+0x3d0)[0x4dc140]
 /home/w00229757/perf(cmd_script+0x12b0)[0x4592e0]
 /home/w00229757/perf[0x4911f1]
 /home/w00229757/perf(main+0x68f)[0x4352ef]
 /lib64/libc.so.6(__libc_start_main+0xf5)[0x7ffff5d6dbd5]
 /home/w00229757/perf[0x435415]
 ======= Memory map: ========

This patch simplify build_id_cache__kallsyms_path(), don't even consider
allocate string buffer, so never free anything. The caller of it should
management memory allocation.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Fixes: 01412261d994 ("perf buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid")
---
 tools/perf/util/build-id.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 67f986c..20aef90 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -147,20 +147,17 @@ static int asnprintf(char **strp, size_t size, const char *fmt, ...)
 char *build_id_cache__kallsyms_path(const char *sbuild_id, char *bf,
 				    size_t size)
 {
-	bool is_alloc = !!bf;
 	bool retry_old = true;
 
-	asnprintf(&bf, size, "%s/%s/%s/kallsyms",
-		  buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
+	snprintf(bf, size, "%s/%s/%s/kallsyms",
+		 buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
 retry:
 	if (!access(bf, F_OK))
 		return bf;
-	if (is_alloc)
-		free(bf);
 	if (retry_old) {
 		/* Try old style kallsyms cache */
-		asnprintf(&bf, size, "%s/%s/%s",
-			  buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
+		snprintf(bf, size, "%s/%s/%s",
+			 buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
 		retry_old = false;
 		goto retry;
 	}
-- 
1.8.3.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [tip:perf/core] perf tools: Fix crash in build_id_cache__kallsyms_path()
  2016-06-07  3:54 [PATCH] perf tools: Fix crash in build_id_cache__kallsyms_path() Wang Nan
@ 2016-06-08  8:44 ` tip-bot for Wang Nan
  2016-06-11  1:28 ` [PATCH] " Masami Hiramatsu
  1 sibling, 0 replies; 3+ messages in thread
From: tip-bot for Wang Nan @ 2016-06-08  8:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mhiramat, wangnan0, linux-kernel, lizefan, tglx, hpa, acme, mingo

Commit-ID:  c58c49ac630979a285d574b3f72a528209515fb3
Gitweb:     http://git.kernel.org/tip/c58c49ac630979a285d574b3f72a528209515fb3
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Tue, 7 Jun 2016 03:54:38 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 7 Jun 2016 10:49:41 -0300

perf tools: Fix crash in build_id_cache__kallsyms_path()

build_id_cache__kallsyms_path() accepts a string buffer but also allocs
a buffer using asnprintf. Unfortunately, the its only user passes it a
stack-allocated buffer. Freeing it causes crashes like this:

  $ perf script
  *** Error in `/home/wangnan/perf': free(): invalid pointer: 0x00007fffffff9630 ***
  ======= Backtrace: =========
  lib64/libc.so.6(+0x6eeef)[0x7ffff5dbaeef]
  lib64/libc.so.6(+0x78cae)[0x7ffff5dc4cae]
  lib64/libc.so.6(+0x79987)[0x7ffff5dc5987]
  /home/w00229757/perf(build_id_cache__kallsyms_path+0x6b)[0x49681b]
  /home/w00229757/perf[0x4bdd40]
  /home/w00229757/perf(dso__load+0xa3a)[0x4c048a]
  /home/w00229757/perf(map__load+0x6f)[0x4d561f]
  /home/w00229757/perf(thread__find_addr_map+0x235)[0x49e935]
  /home/w00229757/perf(machine__resolve+0x7d)[0x49ec6d]
  /home/w00229757/perf[0x4555a8]
  /home/w00229757/perf[0x4d9507]
  /home/w00229757/perf[0x4d9e80]
  /home/w00229757/perf(ordered_events__flush+0x354)[0x4dd444]
  /home/w00229757/perf(perf_session__process_events+0x3d0)[0x4dc140]
  /home/w00229757/perf(cmd_script+0x12b0)[0x4592e0]
  /home/w00229757/perf[0x4911f1]
  /home/w00229757/perf(main+0x68f)[0x4352ef]
  /lib64/libc.so.6(__libc_start_main+0xf5)[0x7ffff5d6dbd5]
  /home/w00229757/perf[0x435415]
  ======= Memory map: ========

This patch simplifies build_id_cache__kallsyms_path(), not even
considering allocating a string buffer, so never frees anything. Its
caller should manage memory allocation.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Fixes: 01412261d994 ("perf buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid")
Link: http://lkml.kernel.org/r/1465271678-7392-1-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/build-id.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 67f986c..20aef90 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -147,20 +147,17 @@ static int asnprintf(char **strp, size_t size, const char *fmt, ...)
 char *build_id_cache__kallsyms_path(const char *sbuild_id, char *bf,
 				    size_t size)
 {
-	bool is_alloc = !!bf;
 	bool retry_old = true;
 
-	asnprintf(&bf, size, "%s/%s/%s/kallsyms",
-		  buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
+	snprintf(bf, size, "%s/%s/%s/kallsyms",
+		 buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
 retry:
 	if (!access(bf, F_OK))
 		return bf;
-	if (is_alloc)
-		free(bf);
 	if (retry_old) {
 		/* Try old style kallsyms cache */
-		asnprintf(&bf, size, "%s/%s/%s",
-			  buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
+		snprintf(bf, size, "%s/%s/%s",
+			 buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
 		retry_old = false;
 		goto retry;
 	}

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] perf tools: Fix crash in build_id_cache__kallsyms_path()
  2016-06-07  3:54 [PATCH] perf tools: Fix crash in build_id_cache__kallsyms_path() Wang Nan
  2016-06-08  8:44 ` [tip:perf/core] " tip-bot for Wang Nan
@ 2016-06-11  1:28 ` Masami Hiramatsu
  1 sibling, 0 replies; 3+ messages in thread
From: Masami Hiramatsu @ 2016-06-11  1:28 UTC (permalink / raw)
  To: Wang Nan; +Cc: acme, pi3orama, lizefan, linux-kernel, Arnaldo Carvalho de Melo

On Tue, 7 Jun 2016 03:54:38 +0000
Wang Nan <wangnan0@huawei.com> wrote:

> build_id_cache__kallsyms_path() accept string buffer but also alloc buffer
> by itself using asnprintf. Unfortunately, the only user of it passes it
> a stack-allocated buffer. Freeing it causes crash like this:
> 
>  $ perf script
>  *** Error in `/home/wangnan/perf': free(): invalid pointer: 0x00007fffffff9630 ***
>  ======= Backtrace: =========
>  lib64/libc.so.6(+0x6eeef)[0x7ffff5dbaeef]
>  lib64/libc.so.6(+0x78cae)[0x7ffff5dc4cae]
>  lib64/libc.so.6(+0x79987)[0x7ffff5dc5987]
>  /home/w00229757/perf(build_id_cache__kallsyms_path+0x6b)[0x49681b]
>  /home/w00229757/perf[0x4bdd40]
>  /home/w00229757/perf(dso__load+0xa3a)[0x4c048a]
>  /home/w00229757/perf(map__load+0x6f)[0x4d561f]
>  /home/w00229757/perf(thread__find_addr_map+0x235)[0x49e935]
>  /home/w00229757/perf(machine__resolve+0x7d)[0x49ec6d]
>  /home/w00229757/perf[0x4555a8]
>  /home/w00229757/perf[0x4d9507]
>  /home/w00229757/perf[0x4d9e80]
>  /home/w00229757/perf(ordered_events__flush+0x354)[0x4dd444]
>  /home/w00229757/perf(perf_session__process_events+0x3d0)[0x4dc140]
>  /home/w00229757/perf(cmd_script+0x12b0)[0x4592e0]
>  /home/w00229757/perf[0x4911f1]
>  /home/w00229757/perf(main+0x68f)[0x4352ef]
>  /lib64/libc.so.6(__libc_start_main+0xf5)[0x7ffff5d6dbd5]
>  /home/w00229757/perf[0x435415]
>  ======= Memory map: ========
> 
> This patch simplify build_id_cache__kallsyms_path(), don't even consider
> allocate string buffer, so never free anything. The caller of it should
> management memory allocation.
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Fixes: 01412261d994 ("perf buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid")
> ---
>  tools/perf/util/build-id.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 67f986c..20aef90 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -147,20 +147,17 @@ static int asnprintf(char **strp, size_t size, const char *fmt, ...)
>  char *build_id_cache__kallsyms_path(const char *sbuild_id, char *bf,
>  				    size_t size)
>  {
> -	bool is_alloc = !!bf;

Oops, I think this is the reason why it failed.
is_alloc should be !bf (means allocation needed)

>  	bool retry_old = true;
>  
> -	asnprintf(&bf, size, "%s/%s/%s/kallsyms",
> -		  buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
> +	snprintf(bf, size, "%s/%s/%s/kallsyms",
> +		 buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
>  retry:
>  	if (!access(bf, F_OK))
>  		return bf;
> -	if (is_alloc)
> -		free(bf);

Then, it works well.

Thanks,

>  	if (retry_old) {
>  		/* Try old style kallsyms cache */
> -		asnprintf(&bf, size, "%s/%s/%s",
> -			  buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
> +		snprintf(bf, size, "%s/%s/%s",
> +			 buildid_dir, DSO__NAME_KALLSYMS, sbuild_id);
>  		retry_old = false;
>  		goto retry;
>  	}
> -- 
> 1.8.3.4
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-06-11  1:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-07  3:54 [PATCH] perf tools: Fix crash in build_id_cache__kallsyms_path() Wang Nan
2016-06-08  8:44 ` [tip:perf/core] " tip-bot for Wang Nan
2016-06-11  1:28 ` [PATCH] " Masami Hiramatsu

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).