[PATCH 0/6] perf string handling fixes

[PATCH 0/6] perf string handling fixes

[Tommi Rantala]

Hi,

Some small perf fixes, mostly caught with valgrind.

The last patch is a simplification: it is easier to open /proc/self/exe
than /proc/$pid/exe.

Tommi Rantala (6):
  perf buildid: do not update SDT cache with null filename
  perf buildid: do not assume that readlink() returns a null terminated
    string
  perf tests: do not assume that readlink() returns a null terminated
    string
  perf utils: use sizeof(buf)-1 in readlink() call
  perf utils: null terminate buf in read_ftrace_printk()
  perf utils: readlink /proc/self/exe to find the perf binary

 tools/perf/tests/sdt.c             | 2 +-
 tools/perf/util/build-id.c         | 8 ++++++--
 tools/perf/util/header.c           | 8 ++------
 tools/perf/util/trace-event-read.c | 4 +++-
 4 files changed, 12 insertions(+), 10 deletions(-)

-- 
2.9.3

[PATCH 1/6] perf buildid: do not update SDT cache with null filename

[Tommi Rantala]

Valgrind was complaining:

  ==2633== Syscall param open(filename) points to unaddressable byte(s)
  ==2633==    at 0x5281CC0: __open_nocancel (syscall-template.S:84)
  ==2633==    by 0x537D38: open (fcntl2.h:53)
  ==2633==    by 0x537D38: get_sdt_note_list (symbol-elf.c:2017)
  ==2633==    by 0x5396FD: probe_cache__scan_sdt (probe-file.c:700)
  ==2633==    by 0x49EA2C: build_id_cache__add_sdt_cache (build-id.c:625)
  ==2633==    by 0x49EA2C: build_id_cache__add_s (build-id.c:697)
  ==2633==    by 0x49EE72: build_id_cache__add_b (build-id.c:717)
  ==2633==    by 0x49EE72: dso__cache_build_id (build-id.c:782)
  ==2633==    by 0x49F190: __dsos__cache_build_ids (build-id.c:793)
  ==2633==    by 0x49F190: machine__cache_build_ids (build-id.c:801)
  ==2633==    by 0x49F190: perf_session__cache_build_ids (build-id.c:815)
  ==2633==    by 0x4CD4F2: write_build_id (header.c:165)
  ==2633==    by 0x4D26F7: do_write_feat (header.c:2296)
  ==2633==    by 0x4D26F7: perf_header__adds_write (header.c:2335)
  ==2633==    by 0x4D26F7: perf_session__write_header (header.c:2414)
  ==2633==    by 0x43B324: __cmd_record (builtin-record.c:1154)
  ==2633==    by 0x43B324: cmd_record (builtin-record.c:1839)
  ==2633==    by 0x455A07: __cmd_record (builtin-kmem.c:1868)
  ==2633==    by 0x455A07: cmd_kmem (builtin-kmem.c:1944)
  ==2633==    by 0x497150: run_builtin (perf.c:359)
  ==2633==    by 0x428CE0: handle_internal_command (perf.c:421)
  ==2633==    by 0x428CE0: run_argv (perf.c:467)
  ==2633==    by 0x428CE0: main (perf.c:614)
  ==2633==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
---
 tools/perf/util/build-id.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index e528c40..234859f 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -690,7 +690,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 		err = 0;
 
 	/* Update SDT cache : error is just warned */
-	if (build_id_cache__add_sdt_cache(sbuild_id, realname) < 0)
+	if (realname && build_id_cache__add_sdt_cache(sbuild_id, realname) < 0)
 		pr_debug4("Failed to update/scan SDT cache for %s\n", realname);
 
 out_free:
-- 
2.9.3

[PATCH 2/6] perf buildid: do not assume that readlink() returns a null terminated string

[Tommi Rantala]

Valgrind was complaining:

  $ valgrind ./perf list >/dev/null
  ==11643== Memcheck, a memory error detector
  ==11643== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
  ==11643== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
  ==11643== Command: ./perf list
  ==11643==
  ==11643== Conditional jump or move depends on uninitialised value(s)
  ==11643==    at 0x4C30620: rindex (vg_replace_strmem.c:199)
  ==11643==    by 0x49DAA9: build_id_cache__origname (build-id.c:198)
  ==11643==    by 0x49E1C7: build_id_cache__valid_id (build-id.c:222)
  ==11643==    by 0x49E1C7: build_id_cache__list_all (build-id.c:507)
  ==11643==    by 0x4B9C8F: print_sdt_events (parse-events.c:2067)
  ==11643==    by 0x4BB0B3: print_events (parse-events.c:2313)
  ==11643==    by 0x439501: cmd_list (builtin-list.c:53)
  ==11643==    by 0x497150: run_builtin (perf.c:359)
  ==11643==    by 0x428CE0: handle_internal_command (perf.c:421)
  ==11643==    by 0x428CE0: run_argv (perf.c:467)
  ==11643==    by 0x428CE0: main (perf.c:614)
  [...]

Additionally, a zero length result from readlink() is not very interesting.

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
---
 tools/perf/util/build-id.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 234859f..9ad77b0 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -182,13 +182,17 @@ char *build_id_cache__origname(const char *sbuild_id)
 	char buf[PATH_MAX];
 	char *ret = NULL, *p;
 	size_t offs = 5;	/* == strlen("../..") */
+	ssize_t len;
 
 	linkname = build_id_cache__linkname(sbuild_id, NULL, 0);
 	if (!linkname)
 		return NULL;
 
-	if (readlink(linkname, buf, PATH_MAX) < 0)
+	len = readlink(linkname, buf, sizeof(buf)-1);
+	if (len <= 0)
 		goto out;
+	buf[len] = '\0';
+
 	/* The link should be "../..<origpath>/<sbuild_id>" */
 	p = strrchr(buf, '/');	/* Cut off the "/<sbuild_id>" */
 	if (p && (p > buf + offs)) {
-- 
2.9.3

[PATCH 3/6] perf tests: do not assume that readlink() returns a null terminated string

[Tommi Rantala]

Ensure that the string in buf is null terminated.

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
---
 tools/perf/tests/sdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/sdt.c b/tools/perf/tests/sdt.c
index f59d210..121949a 100644
--- a/tools/perf/tests/sdt.c
+++ b/tools/perf/tests/sdt.c
@@ -43,7 +43,7 @@ static char *get_self_path(void)
 {
 	char *buf = calloc(PATH_MAX, sizeof(char));
 
-	if (buf && readlink("/proc/self/exe", buf, PATH_MAX) < 0) {
+	if (buf && readlink("/proc/self/exe", buf, PATH_MAX-1) < 0) {
 		pr_debug("Failed to get correct path of perf\n");
 		free(buf);
 		return NULL;
-- 
2.9.3

[PATCH 4/6] perf utils: use sizeof(buf)-1 in readlink() call

[Tommi Rantala]

Ensure that we have space for the null byte in buf.

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
---
 tools/perf/util/header.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 05714d5..ab10e9d 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -378,7 +378,7 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
 	 * actual atual path to perf binary
 	 */
 	sprintf(proc, "/proc/%d/exe", getpid());
-	ret = readlink(proc, buf, sizeof(buf));
+	ret = readlink(proc, buf, sizeof(buf)-1);
 	if (ret <= 0)
 		return -1;
 
-- 
2.9.3

[PATCH 5/6] perf utils: null terminate buf in read_ftrace_printk()

[Tommi Rantala]

Ensure that the string that we read from the data file is null terminated.

Valgrind was complaining:

  ==31357== Invalid read of size 1
  ==31357==    at 0x4EC8C1: __strtok_r_1c (string2.h:200)
  ==31357==    by 0x4EC8C1: parse_ftrace_printk (trace-event-parse.c:161)
  ==31357==    by 0x4F82A8: read_ftrace_printk (trace-event-read.c:204)
  ==31357==    by 0x4F82A8: trace_report (trace-event-read.c:468)
  ==31357==    by 0x4CD552: process_tracing_data (header.c:1576)
  ==31357==    by 0x4D3397: perf_file_section__process (header.c:2705)
  ==31357==    by 0x4D3397: perf_header__process_sections (header.c:2488)
  ==31357==    by 0x4D3397: perf_session__read_header (header.c:2925)
  ==31357==    by 0x4E71E2: perf_session__open (session.c:32)
  ==31357==    by 0x4E71E2: perf_session__new (session.c:139)
  ==31357==    by 0x429F5D: cmd_annotate (builtin-annotate.c:472)
  ==31357==    by 0x497150: run_builtin (perf.c:359)
  ==31357==    by 0x428CE0: handle_internal_command (perf.c:421)
  ==31357==    by 0x428CE0: run_argv (perf.c:467)
  ==31357==    by 0x428CE0: main (perf.c:614)
  ==31357==  Address 0x8ac0efb is 0 bytes after a block of size 1,963 alloc'd
  ==31357==    at 0x4C2DB9D: malloc (vg_replace_malloc.c:299)
  ==31357==    by 0x4F827B: read_ftrace_printk (trace-event-read.c:195)
  ==31357==    by 0x4F827B: trace_report (trace-event-read.c:468)
  ==31357==    by 0x4CD552: process_tracing_data (header.c:1576)
  ==31357==    by 0x4D3397: perf_file_section__process (header.c:2705)
  ==31357==    by 0x4D3397: perf_header__process_sections (header.c:2488)
  ==31357==    by 0x4D3397: perf_session__read_header (header.c:2925)
  ==31357==    by 0x4E71E2: perf_session__open (session.c:32)
  ==31357==    by 0x4E71E2: perf_session__new (session.c:139)
  ==31357==    by 0x429F5D: cmd_annotate (builtin-annotate.c:472)
  ==31357==    by 0x497150: run_builtin (perf.c:359)
  ==31357==    by 0x428CE0: handle_internal_command (perf.c:421)
  ==31357==    by 0x428CE0: run_argv (perf.c:467)
  ==31357==    by 0x428CE0: main (perf.c:614)

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
---
 tools/perf/util/trace-event-read.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index 2742015..04605c0 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -192,7 +192,7 @@ static int read_ftrace_printk(struct pevent *pevent)
 	if (!size)
 		return 0;
 
-	buf = malloc(size);
+	buf = malloc(size+1);
 	if (buf == NULL)
 		return -1;
 
@@ -201,6 +201,8 @@ static int read_ftrace_printk(struct pevent *pevent)
 		return -1;
 	}
 
+	buf[size] = '\0';
+
 	parse_ftrace_printk(pevent, buf, size);
 
 	free(buf);
-- 
2.9.3

[PATCH 6/6] perf utils: readlink /proc/self/exe to find the perf binary

[Tommi Rantala]

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
---
 tools/perf/util/header.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index ab10e9d..c6243af 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -370,15 +370,11 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
 			 struct perf_evlist *evlist __maybe_unused)
 {
 	char buf[MAXPATHLEN];
-	char proc[32];
 	u32 n;
 	int i, ret;
 
-	/*
-	 * actual atual path to perf binary
-	 */
-	sprintf(proc, "/proc/%d/exe", getpid());
-	ret = readlink(proc, buf, sizeof(buf)-1);
+	/* actual path to perf binary */
+	ret = readlink("/proc/self/exe", buf, sizeof(buf)-1);
 	if (ret <= 0)
 		return -1;
 
-- 
2.9.3

Re: [PATCH 5/6] perf utils: null terminate buf in read_ftrace_printk()

[Arnaldo Carvalho de Melo]

Em Wed, Mar 22, 2017 at 03:06:23PM +0200, Tommi Rantala escreveu:
> Ensure that the string that we read from the data file is null terminated.
> 
> Valgrind was complaining:

Thanks, applied.
 
- Arnaldo

Re: [PATCH 0/6] perf string handling fixes

[Arnaldo Carvalho de Melo]

Em Wed, Mar 22, 2017 at 03:06:18PM +0200, Tommi Rantala escreveu:
> Hi,
> 
> Some small perf fixes, mostly caught with valgrind.
> 
> The last patch is a simplification: it is easier to open /proc/self/exe
> than /proc/$pid/exe.

Thanks, applied the series.

- Arnaldo
 
> Tommi Rantala (6):
>   perf buildid: do not update SDT cache with null filename
>   perf buildid: do not assume that readlink() returns a null terminated
>     string
>   perf tests: do not assume that readlink() returns a null terminated
>     string
>   perf utils: use sizeof(buf)-1 in readlink() call
>   perf utils: null terminate buf in read_ftrace_printk()
>   perf utils: readlink /proc/self/exe to find the perf binary
> 
>  tools/perf/tests/sdt.c             | 2 +-
>  tools/perf/util/build-id.c         | 8 ++++++--
>  tools/perf/util/header.c           | 8 ++------
>  tools/perf/util/trace-event-read.c | 4 +++-
>  4 files changed, 12 insertions(+), 10 deletions(-)
> 
> -- 
> 2.9.3

[tip:perf/core] perf buildid: Do not update SDT cache with null filename

[tip-bot for Tommi Rantala]

Commit-ID:  2ccc220238680642be87a2d010ce07f1c40edafb
Gitweb:     http://git.kernel.org/tip/2ccc220238680642be87a2d010ce07f1c40edafb
Author:     Tommi Rantala <tommi.t.rantala@nokia.com>
AuthorDate: Wed, 22 Mar 2017 15:06:19 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 27 Mar 2017 15:33:36 -0300

perf buildid: Do not update SDT cache with null filename

Valgrind was complaining:

  ==2633== Syscall param open(filename) points to unaddressable byte(s)
  ==2633==    at 0x5281CC0: __open_nocancel (syscall-template.S:84)
  ==2633==    by 0x537D38: open (fcntl2.h:53)
  ==2633==    by 0x537D38: get_sdt_note_list (symbol-elf.c:2017)
  ==2633==    by 0x5396FD: probe_cache__scan_sdt (probe-file.c:700)
  ==2633==    by 0x49EA2C: build_id_cache__add_sdt_cache (build-id.c:625)
  ==2633==    by 0x49EA2C: build_id_cache__add_s (build-id.c:697)
  ==2633==    by 0x49EE72: build_id_cache__add_b (build-id.c:717)
  ==2633==    by 0x49EE72: dso__cache_build_id (build-id.c:782)
  ==2633==    by 0x49F190: __dsos__cache_build_ids (build-id.c:793)
  ==2633==    by 0x49F190: machine__cache_build_ids (build-id.c:801)
  ==2633==    by 0x49F190: perf_session__cache_build_ids (build-id.c:815)
  ==2633==    by 0x4CD4F2: write_build_id (header.c:165)
  ==2633==    by 0x4D26F7: do_write_feat (header.c:2296)
  ==2633==    by 0x4D26F7: perf_header__adds_write (header.c:2335)
  ==2633==    by 0x4D26F7: perf_session__write_header (header.c:2414)
  ==2633==    by 0x43B324: __cmd_record (builtin-record.c:1154)
  ==2633==    by 0x43B324: cmd_record (builtin-record.c:1839)
  ==2633==    by 0x455A07: __cmd_record (builtin-kmem.c:1868)
  ==2633==    by 0x455A07: cmd_kmem (builtin-kmem.c:1944)
  ==2633==    by 0x497150: run_builtin (perf.c:359)
  ==2633==    by 0x428CE0: handle_internal_command (perf.c:421)
  ==2633==    by 0x428CE0: run_argv (perf.c:467)
  ==2633==    by 0x428CE0: main (perf.c:614)
  ==2633==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tommi Rantala <tommi.t.rantala@nokia.com>
Link: http://lkml.kernel.org/r/20170322130624.21881-2-tommi.t.rantala@nokia.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/build-id.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index e528c40..234859f 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -690,7 +690,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 		err = 0;
 
 	/* Update SDT cache : error is just warned */
-	if (build_id_cache__add_sdt_cache(sbuild_id, realname) < 0)
+	if (realname && build_id_cache__add_sdt_cache(sbuild_id, realname) < 0)
 		pr_debug4("Failed to update/scan SDT cache for %s\n", realname);
 
 out_free:

[tip:perf/core] perf buildid: Do not assume that readlink() returns a null terminated string

[tip-bot for Tommi Rantala]

Commit-ID:  5a2342111c68e623e27ee7ea3d0492d8dad6bda0
Gitweb:     http://git.kernel.org/tip/5a2342111c68e623e27ee7ea3d0492d8dad6bda0
Author:     Tommi Rantala <tommi.t.rantala@nokia.com>
AuthorDate: Wed, 22 Mar 2017 15:06:20 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 27 Mar 2017 15:35:06 -0300

perf buildid: Do not assume that readlink() returns a null terminated string

Valgrind was complaining:

  $ valgrind ./perf list >/dev/null
  ==11643== Memcheck, a memory error detector
  ==11643== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
  ==11643== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
  ==11643== Command: ./perf list
  ==11643==
  ==11643== Conditional jump or move depends on uninitialised value(s)
  ==11643==    at 0x4C30620: rindex (vg_replace_strmem.c:199)
  ==11643==    by 0x49DAA9: build_id_cache__origname (build-id.c:198)
  ==11643==    by 0x49E1C7: build_id_cache__valid_id (build-id.c:222)
  ==11643==    by 0x49E1C7: build_id_cache__list_all (build-id.c:507)
  ==11643==    by 0x4B9C8F: print_sdt_events (parse-events.c:2067)
  ==11643==    by 0x4BB0B3: print_events (parse-events.c:2313)
  ==11643==    by 0x439501: cmd_list (builtin-list.c:53)
  ==11643==    by 0x497150: run_builtin (perf.c:359)
  ==11643==    by 0x428CE0: handle_internal_command (perf.c:421)
  ==11643==    by 0x428CE0: run_argv (perf.c:467)
  ==11643==    by 0x428CE0: main (perf.c:614)
  [...]

Additionally, a zero length result from readlink() is not very interesting.

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20170322130624.21881-3-tommi.t.rantala@nokia.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/build-id.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 234859f..33af675 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -182,13 +182,17 @@ char *build_id_cache__origname(const char *sbuild_id)
 	char buf[PATH_MAX];
 	char *ret = NULL, *p;
 	size_t offs = 5;	/* == strlen("../..") */
+	ssize_t len;
 
 	linkname = build_id_cache__linkname(sbuild_id, NULL, 0);
 	if (!linkname)
 		return NULL;
 
-	if (readlink(linkname, buf, PATH_MAX) < 0)
+	len = readlink(linkname, buf, sizeof(buf) - 1);
+	if (len <= 0)
 		goto out;
+	buf[len] = '\0';
+
 	/* The link should be "../..<origpath>/<sbuild_id>" */
 	p = strrchr(buf, '/');	/* Cut off the "/<sbuild_id>" */
 	if (p && (p > buf + offs)) {

[tip:perf/core] perf tests: Do not assume that readlink() returns a null terminated string

[tip-bot for Tommi Rantala]

Commit-ID:  0e6ba11511aef91ba8e2528ddc681d88922d7b0b
Gitweb:     http://git.kernel.org/tip/0e6ba11511aef91ba8e2528ddc681d88922d7b0b
Author:     Tommi Rantala <tommi.t.rantala@nokia.com>
AuthorDate: Wed, 22 Mar 2017 15:06:21 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 27 Mar 2017 15:35:56 -0300

perf tests: Do not assume that readlink() returns a null terminated string

Ensure that the string in buf is null terminated.

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20170322130624.21881-4-tommi.t.rantala@nokia.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/sdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/sdt.c b/tools/perf/tests/sdt.c
index f59d210..26e5b7a 100644
--- a/tools/perf/tests/sdt.c
+++ b/tools/perf/tests/sdt.c
@@ -43,7 +43,7 @@ static char *get_self_path(void)
 {
 	char *buf = calloc(PATH_MAX, sizeof(char));
 
-	if (buf && readlink("/proc/self/exe", buf, PATH_MAX) < 0) {
+	if (buf && readlink("/proc/self/exe", buf, PATH_MAX - 1) < 0) {
 		pr_debug("Failed to get correct path of perf\n");
 		free(buf);
 		return NULL;

[tip:perf/core] perf utils: use sizeof(buf) - 1 in readlink() call

[tip-bot for Tommi Rantala]

Commit-ID:  b7126ef78612a3d4a37aadf39125cff048cebb9b
Gitweb:     http://git.kernel.org/tip/b7126ef78612a3d4a37aadf39125cff048cebb9b
Author:     Tommi Rantala <tommi.t.rantala@nokia.com>
AuthorDate: Wed, 22 Mar 2017 15:06:22 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 27 Mar 2017 15:36:27 -0300

perf utils: use sizeof(buf) - 1 in readlink() call

Ensure that we have space for the null byte in buf.

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20170322130624.21881-5-tommi.t.rantala@nokia.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 05714d5..cf22962 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -378,7 +378,7 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
 	 * actual atual path to perf binary
 	 */
 	sprintf(proc, "/proc/%d/exe", getpid());
-	ret = readlink(proc, buf, sizeof(buf));
+	ret = readlink(proc, buf, sizeof(buf) - 1);
 	if (ret <= 0)
 		return -1;
 

[tip:perf/core] perf utils: Null terminate buf in read_ftrace_printk()

[tip-bot for Tommi Rantala]

Commit-ID:  d4b364df5f6540e8d6a38008ce2693ba73a8508a
Gitweb:     http://git.kernel.org/tip/d4b364df5f6540e8d6a38008ce2693ba73a8508a
Author:     Tommi Rantala <tommi.t.rantala@nokia.com>
AuthorDate: Wed, 22 Mar 2017 15:06:23 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 27 Mar 2017 15:37:35 -0300

perf utils: Null terminate buf in read_ftrace_printk()

Ensure that the string that we read from the data file is null terminated.

Valgrind was complaining:

  ==31357== Invalid read of size 1
  ==31357==    at 0x4EC8C1: __strtok_r_1c (string2.h:200)
  ==31357==    by 0x4EC8C1: parse_ftrace_printk (trace-event-parse.c:161)
  ==31357==    by 0x4F82A8: read_ftrace_printk (trace-event-read.c:204)
  ==31357==    by 0x4F82A8: trace_report (trace-event-read.c:468)
  ==31357==    by 0x4CD552: process_tracing_data (header.c:1576)
  ==31357==    by 0x4D3397: perf_file_section__process (header.c:2705)
  ==31357==    by 0x4D3397: perf_header__process_sections (header.c:2488)
  ==31357==    by 0x4D3397: perf_session__read_header (header.c:2925)
  ==31357==    by 0x4E71E2: perf_session__open (session.c:32)
  ==31357==    by 0x4E71E2: perf_session__new (session.c:139)
  ==31357==    by 0x429F5D: cmd_annotate (builtin-annotate.c:472)
  ==31357==    by 0x497150: run_builtin (perf.c:359)
  ==31357==    by 0x428CE0: handle_internal_command (perf.c:421)
  ==31357==    by 0x428CE0: run_argv (perf.c:467)
  ==31357==    by 0x428CE0: main (perf.c:614)
  ==31357==  Address 0x8ac0efb is 0 bytes after a block of size 1,963 alloc'd
  ==31357==    at 0x4C2DB9D: malloc (vg_replace_malloc.c:299)
  ==31357==    by 0x4F827B: read_ftrace_printk (trace-event-read.c:195)
  ==31357==    by 0x4F827B: trace_report (trace-event-read.c:468)
  ==31357==    by 0x4CD552: process_tracing_data (header.c:1576)
  ==31357==    by 0x4D3397: perf_file_section__process (header.c:2705)
  ==31357==    by 0x4D3397: perf_header__process_sections (header.c:2488)
  ==31357==    by 0x4D3397: perf_session__read_header (header.c:2925)
  ==31357==    by 0x4E71E2: perf_session__open (session.c:32)
  ==31357==    by 0x4E71E2: perf_session__new (session.c:139)
  ==31357==    by 0x429F5D: cmd_annotate (builtin-annotate.c:472)
  ==31357==    by 0x497150: run_builtin (perf.c:359)
  ==31357==    by 0x428CE0: handle_internal_command (perf.c:421)
  ==31357==    by 0x428CE0: run_argv (perf.c:467)
  ==31357==    by 0x428CE0: main (perf.c:614)

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20170322130624.21881-6-tommi.t.rantala@nokia.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/trace-event-read.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index 2742015..8a9a677 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -192,7 +192,7 @@ static int read_ftrace_printk(struct pevent *pevent)
 	if (!size)
 		return 0;
 
-	buf = malloc(size);
+	buf = malloc(size + 1);
 	if (buf == NULL)
 		return -1;
 
@@ -201,6 +201,8 @@ static int read_ftrace_printk(struct pevent *pevent)
 		return -1;
 	}
 
+	buf[size] = '\0';
+
 	parse_ftrace_printk(pevent, buf, size);
 
 	free(buf);

[tip:perf/core] perf utils: Readlink /proc/self/exe to find the perf binary

[tip-bot for Tommi Rantala]

Commit-ID:  55f77128e7652e537d6c226d5b56821cdb5c22de
Gitweb:     http://git.kernel.org/tip/55f77128e7652e537d6c226d5b56821cdb5c22de
Author:     Tommi Rantala <tommi.t.rantala@nokia.com>
AuthorDate: Wed, 22 Mar 2017 15:06:24 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 27 Mar 2017 15:37:54 -0300

perf utils: Readlink /proc/self/exe to find the perf binary

Simplification: it is easier to open /proc/self/exe than /proc/$pid/exe.

Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20170322130624.21881-7-tommi.t.rantala@nokia.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index cf22962..ef09f26 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -370,15 +370,11 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
 			 struct perf_evlist *evlist __maybe_unused)
 {
 	char buf[MAXPATHLEN];
-	char proc[32];
 	u32 n;
 	int i, ret;
 
-	/*
-	 * actual atual path to perf binary
-	 */
-	sprintf(proc, "/proc/%d/exe", getpid());
-	ret = readlink(proc, buf, sizeof(buf) - 1);
+	/* actual path to perf binary */
+	ret = readlink("/proc/self/exe", buf, sizeof(buf) - 1);
 	if (ret <= 0)
 		return -1;