2017-03-22 13:07:52

by Tommi Rantala

[permalink] [raw]
Subject: [PATCH 0/6] perf string handling fixes

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


2017-03-22 13:08:37

by Tommi Rantala

[permalink] [raw]
Subject: [PATCH 3/6] 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 <[email protected]>
---
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

2017-03-22 13:08:52

by Tommi Rantala

[permalink] [raw]
Subject: [PATCH 5/6] 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 <[email protected]>
---
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

2017-03-22 13:09:04

by Tommi Rantala

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

Signed-off-by: Tommi Rantala <[email protected]>
---
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

2017-03-22 13:09:21

by Tommi Rantala

[permalink] [raw]
Subject: [PATCH 4/6] 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 <[email protected]>
---
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

2017-03-22 13:09:41

by Tommi Rantala

[permalink] [raw]
Subject: [PATCH 2/6] 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 <[email protected]>
---
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

2017-03-22 13:09:55

by Tommi Rantala

[permalink] [raw]
Subject: [PATCH 1/6] 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 <[email protected]>
---
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

2017-03-27 18:37:31

by Arnaldo Carvalho de Melo

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

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

2017-03-27 18:39:10

by Arnaldo Carvalho de Melo

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

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

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

Commit-ID: b7126ef78612a3d4a37aadf39125cff048cebb9b
Gitweb: http://git.kernel.org/tip/b7126ef78612a3d4a37aadf39125cff048cebb9b
Author: Tommi Rantala <[email protected]>
AuthorDate: Wed, 22 Mar 2017 15:06:22 +0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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;


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

Commit-ID: 0e6ba11511aef91ba8e2528ddc681d88922d7b0b
Gitweb: http://git.kernel.org/tip/0e6ba11511aef91ba8e2528ddc681d88922d7b0b
Author: Tommi Rantala <[email protected]>
AuthorDate: Wed, 22 Mar 2017 15:06:21 +0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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;

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

Commit-ID: 2ccc220238680642be87a2d010ce07f1c40edafb
Gitweb: http://git.kernel.org/tip/2ccc220238680642be87a2d010ce07f1c40edafb
Author: Tommi Rantala <[email protected]>
AuthorDate: Wed, 22 Mar 2017 15:06:19 +0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Tommi Rantala <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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:

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

Commit-ID: 5a2342111c68e623e27ee7ea3d0492d8dad6bda0
Gitweb: http://git.kernel.org/tip/5a2342111c68e623e27ee7ea3d0492d8dad6bda0
Author: Tommi Rantala <[email protected]>
AuthorDate: Wed, 22 Mar 2017 15:06:20 +0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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)) {

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

Commit-ID: d4b364df5f6540e8d6a38008ce2693ba73a8508a
Gitweb: http://git.kernel.org/tip/d4b364df5f6540e8d6a38008ce2693ba73a8508a
Author: Tommi Rantala <[email protected]>
AuthorDate: Wed, 22 Mar 2017 15:06:23 +0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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);

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

Commit-ID: 55f77128e7652e537d6c226d5b56821cdb5c22de
Gitweb: http://git.kernel.org/tip/55f77128e7652e537d6c226d5b56821cdb5c22de
Author: Tommi Rantala <[email protected]>
AuthorDate: Wed, 22 Mar 2017 15:06:24 +0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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;