perf mmap: Fix accessing unmapped mmap in perf_mmap__read_done()

PATCH In review [email protected] 3 participants 6 messages Mar 26, 2018 – Mar 29, 2018 perf events

lore

Share & tools

lore

[email protected]

author

2018-03-26 13:43 UTC #0 lore raw

PATCH 1/1 perf mmap: Fix accessing unmapped mmap in perf_mmap__read_done()

From: Kan Liang <[email protected]>

There is segmentation fault when running perf trace. For example,
[root@jouet e]# perf trace -e *chdir -o /tmp/bla perf report --ignore-vmlinux -i ../perf.data

The perf_mmap__consume() could unmap the mmap. It needs to check the refcnt in perf_mmap__read_done().

Fixes ee023de05f35 ("perf mmap: Introduce perf_mmap__read_done()")

Reported-by Arnaldo Carvalho de Melo <[email protected]>

Signed-off-by Kan Liang <[email protected]>

---
 tools/perf/util/mmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

tools/perf/util/mmap.c +6 -0

index 38ca3ff..f6cfc52 100644
@@ -317,5 +317,11 @@ int perf_mmap__push(struct perf_mmap *md, void *to,
  */
 void perf_mmap__read_done(struct perf_mmap *map)
 {
+	/*
+	 * Check if event was unmapped due to a POLLHUP/POLLERR.
+	 */
+	if (!refcount_read(&map->refcnt))
+		return;
+
 	map->prev = perf_mmap__read_head(map);
 }

Signature · 1 line

2.4.11

Arnaldo Carvalho de Melo

maintainer

2018-03-26 14:26 UTC #1 lore raw

Em Mon, Mar 26, 2018 at 09:42:09AM -0400, [email protected] escreveu:

Quoted 6 lines

From: Kan Liang <[email protected]>
There is segmentation fault when running perf trace. For example,
[root@jouet e]# perf trace -e *chdir -o /tmp/bla perf report --ignore-vmlinux -i ../perf.data

Quoted 6 lines

The perf_mmap__consume() could unmap the mmap. It needs to check the refcnt in perf_mmap__read_done().
Fixes ee023de05f35 ("perf mmap: Introduce perf_mmap__read_done()")
Reported-by Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by Kan Liang <[email protected]>

Seems insuficient:

[root@jouet ~]# perf trace -e *chdir -o /tmp/bla perf report --ignore-vmlinux
perf: Segmentation fault
Obtained 9 stack frames.
perf(sighandler_dump_stack+0x3e) [0x4fe31e] /lib64/libc.so.6(+0x346df) [0x7fc0c3ced6df] perf(perf_mmap__read_init+0x26) [0x4ccd06]
perf(cmd_trace+0x1bf6) [0x47bb76]
perf() [0x4a824e]
perf() [0x4a8550]
perf(main+0x331) [0x42bef1] /lib64/libc.so.6(__libc_start_main+0xe9) [0x7fc0c3cd9f29] perf(_start+0x29) [0x42c319]
Segmentation fault (core dumped)
[root@jouet ~]#

- Arnaldo

Arnaldo Carvalho de Melo

maintainer

2018-03-26 14:41 UTC #2 lore raw

Em Mon, Mar 26, 2018 at 11:26:04AM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 16 lines

Em Mon, Mar 26, 2018 at 09:42:09AM -0400, [email protected] escreveu:
Quoted 6 lines
From: Kan Liang <[email protected]>
There is segmentation fault when running perf trace. For example,
[root@jouet e]# perf trace -e *chdir -o /tmp/bla perf report --ignore-vmlinux -i ../perf.data
Quoted 6 lines
The perf_mmap__consume() could unmap the mmap. It needs to check the refcnt in perf_mmap__read_done().
Fixes ee023de05f35 ("perf mmap: Introduce perf_mmap__read_done()")
Reported-by Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by Kan Liang <[email protected]>
Seems insuficient:

Quoted 14 lines

[root@jouet ~]# perf trace -e *chdir -o /tmp/bla perf report --ignore-vmlinux
perf: Segmentation fault
Obtained 9 stack frames.
perf(sighandler_dump_stack+0x3e) [0x4fe31e] /lib64/libc.so.6(+0x346df) [0x7fc0c3ced6df] perf(perf_mmap__read_init+0x26) [0x4ccd06]
perf(cmd_trace+0x1bf6) [0x47bb76]
perf() [0x4a824e]
perf() [0x4a8550]
perf(main+0x331) [0x42bef1] /lib64/libc.so.6(__libc_start_main+0xe9) [0x7fc0c3cd9f29] perf(_start+0x29) [0x42c319]
Segmentation fault (core dumped)
[root@jouet ~]#

I'm adding this as a separate patch, with yours and this one, the segfault is gone.

- Arnaldo

tools/perf/util/mmap.c +12 -1

index f6cfc52ff1fe..fc832676a798 100644
@@ -234,7 +234,7 @@ static int overwrite_rb_find_range(void *buf, int mask, u64 *start, u64 *end)
 /*
  * Report the start and end of the available data in ringbuffer
  */
-int perf_mmap__read_init(struct perf_mmap *md)
+static int __perf_mmap__read_init(struct perf_mmap *md)
 {
 	u64 head = perf_mmap__read_head(md);
 	u64 old = md->prev;

@@ -268,6 +268,17 @@ int perf_mmap__read_init(struct perf_mmap *md)
 	return 0;
 }
 
+int perf_mmap__read_init(struct perf_mmap *map)
+{
+	/*
+	 * Check if event was unmapped due to a POLLHUP/POLLERR.
+	 */
+	if (!refcount_read(&map->refcnt))
+		return -ENOENT;
+
+	return __perf_mmap__read_init(map);
+}
+
 int perf_mmap__push(struct perf_mmap *md, void *to,
 		    int push(void *to, void *buf, size_t size))
 {

Liang, Kan

author

2018-03-26 14:51 UTC #3 lore raw

Quoted 32 lines · Arnaldo Carvalho de Melo

Em Mon, Mar 26, 2018 at 11:26:04AM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 16 lines
Em Mon, Mar 26, 2018 at 09:42:09AM -0400, [email protected] escreveu:
Quoted 6 lines
From: Kan Liang <[email protected]>
There is segmentation fault when running perf trace. For example,
[root@jouet e]# perf trace -e *chdir -o /tmp/bla perf report --ignore-vmlinux -i ../perf.data
Quoted 6 lines
The perf_mmap__consume() could unmap the mmap. It needs to check the refcnt in perf_mmap__read_done().
Fixes ee023de05f35 ("perf mmap: Introduce perf_mmap__read_done()")
Reported-by Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by Kan Liang <[email protected]>
Seems insuficient:
Quoted 14 lines
[root@jouet ~]# perf trace -e *chdir -o /tmp/bla perf report --ignore-vmlinux
perf: Segmentation fault
Obtained 9 stack frames.
perf(sighandler_dump_stack+0x3e) [0x4fe31e] /lib64/libc.so.6(+0x346df) [0x7fc0c3ced6df] perf(perf_mmap__read_init+0x26) [0x4ccd06]
perf(cmd_trace+0x1bf6) [0x47bb76]
perf() [0x4a824e]
perf() [0x4a8550]
perf(main+0x331) [0x42bef1] /lib64/libc.so.6(__libc_start_main+0xe9) [0x7fc0c3cd9f29] perf(_start+0x29) [0x42c319]
Segmentation fault (core dumped)
[root@jouet ~]#

Quoted 3 lines

I'm adding this as a separate patch, with yours and this one, the segfault is gone.

Thanks. Yes, we should check the mmap before using it for all new interfaces.

Thanks,
Kan

Quoted 34 lines

- Arnaldo

tools/perf/util/mmap.c +12 -1

index f6cfc52ff1fe..fc832676a798 100644
@@ -234,7 +234,7 @@ static int overwrite_rb_find_range(void *buf, int mask, u64 *start, u64 *end)
  /*
   * Report the start and end of the available data in ringbuffer
   */
-int perf_mmap__read_init(struct perf_mmap *md)
+static int __perf_mmap__read_init(struct perf_mmap *md)
  {
  	u64 head = perf_mmap__read_head(md);
  	u64 old = md->prev;

@@ -268,6 +268,17 @@ int perf_mmap__read_init(struct perf_mmap *md)
  	return 0;
  }
 
+int perf_mmap__read_init(struct perf_mmap *map)
+{
+	/*
+	 * Check if event was unmapped due to a POLLHUP/POLLERR.
+	 */
+	if (!refcount_read(&map->refcnt))
+		return -ENOENT;
+
+	return __perf_mmap__read_init(map);
+}
+
  int perf_mmap__push(struct perf_mmap *md, void *to,
  		    int push(void *to, void *buf, size_t size))
  {

tip-bot for Arnaldo Carvalho de Melo

merge bot

2018-03-29 13:59 UTC #4 lore raw

[tip:perf/core] perf mmap: Be consistent when checking for an unmaped ring buffer

In reply to Arnaldo Carvalho de Melo

perf mmap: Be consistent when checking for an unmaped ring buffer

Commit-ID:  895e3b06fc2ce438adc62cb13d31ea001dcfda16
Gitweb:     https://git.kernel.org/tip/895e3b06fc2ce438adc62cb13d31ea001dcfda16
Author:     Arnaldo Carvalho de Melo <[email protected]>
AuthorDate: Mon, 26 Mar 2018 11:42:15 -0300
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 27 Mar 2018 13:13:38 -0300

perf mmap: Be consistent when checking for an unmaped ring buffer

The previous patch is insufficient to cure the reported 'perf trace' segfault, as it only cures the perf_mmap__read_done() case, moving the segfault to perf_mmap__read_init() functio, fix it by doing the same refcount check.

Cc Adrian Hunter <[email protected]>

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc David Ahern <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Kan Liang <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Wang Nan <[email protected]>

Fixes 8872481bd048 ("perf mmap: Introduce perf_mmap__read_init()")

Link https://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/mmap.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

tools/perf/util/mmap.c +12 -1

index f6cfc52ff1fe..fc832676a798 100644
@@ -234,7 +234,7 @@ static int overwrite_rb_find_range(void *buf, int mask, u64 *start, u64 *end)
 /*
  * Report the start and end of the available data in ringbuffer
  */
-int perf_mmap__read_init(struct perf_mmap *md)
+static int __perf_mmap__read_init(struct perf_mmap *md)
 {
 	u64 head = perf_mmap__read_head(md);
 	u64 old = md->prev;

@@ -268,6 +268,17 @@ int perf_mmap__read_init(struct perf_mmap *md)
 	return 0;
 }
 
+int perf_mmap__read_init(struct perf_mmap *map)
+{
+	/*
+	 * Check if event was unmapped due to a POLLHUP/POLLERR.
+	 */
+	if (!refcount_read(&map->refcnt))
+		return -ENOENT;
+
+	return __perf_mmap__read_init(map);
+}
+
 int perf_mmap__push(struct perf_mmap *md, void *to,
 		    int push(void *to, void *buf, size_t size))
 {

tip-bot for Kan Liang

merge bot

2018-03-29 13:58 UTC #5 lore raw

In reply to [email protected]

perf mmap: Fix accessing unmapped mmap in perf_mmap__read_done()

Commit-ID:  f58385f629c87a9e210108b39c1f4950d0363ad2
Gitweb:     https://git.kernel.org/tip/f58385f629c87a9e210108b39c1f4950d0363ad2
Author:     Kan Liang <[email protected]>
AuthorDate: Mon, 26 Mar 2018 09:42:09 -0400
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 27 Mar 2018 13:13:38 -0300

perf mmap: Fix accessing unmapped mmap in perf_mmap__read_done()

There is a segmentation fault when running 'perf trace'. For example:

  [root@jouet e]# perf trace -e *chdir -o /tmp/bla perf report --ignore-vmlinux -i ../perf.data

The perf_mmap__consume() could unmap the mmap. It needs to check the refcnt in perf_mmap__read_done().

Reported-by Arnaldo Carvalho de Melo <[email protected]>

Signed-off-by Kan Liang <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Wang Nan <[email protected]>

Fixes ee023de05f35 ("perf mmap: Introduce perf_mmap__read_done()")

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/mmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

tools/perf/util/mmap.c +6 -0

index 38ca3ffb9d61..f6cfc52ff1fe 100644
@@ -317,5 +317,11 @@ out:
  */
 void perf_mmap__read_done(struct perf_mmap *map)
 {
+	/*
+	 * Check if event was unmapped due to a POLLHUP/POLLERR.
+	 */
+	if (!refcount_read(&map->refcnt))
+		return;
+
 	map->prev = perf_mmap__read_head(map);
 }