perf tools: Check for null when copying nsinfo.

PATCH Superseded Benno Evers 4 participants 4 messages Aug 9, 2018 – Aug 10, 2018 perf events

Share & tools

Benno Evers

author

2018-08-09 21:54 UTC #0 lore raw

PATCH 1/1 perf tools: Check for null when copying nsinfo.

The argument to nsinfo__copy() was assumed to be valid, but some code paths exist that will lead to NULL being passed.

In particular, running 'perf script -D' on a perf.data file containing an PERF_RECORD_MMAP event associating the '[vdso]' dso with pid 0 earlier in the event stream will lead to a segfault.

Since all calling code is already checking for a non-null return value, just return NULL for this case as well.

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

tools/perf/util/namespaces.c +3 -0

index 5be021701f34..cf8bd123cf73 100644
@@ -139,6 +139,9 @@ struct nsinfo *nsinfo__copy(struct nsinfo *nsi)
 {
 	struct nsinfo *nnsi;
 
+	if (nsi == NULL)
+		return NULL;
+
 	nnsi = calloc(1, sizeof(*nnsi));
 	if (nnsi != NULL) {
 		nnsi->pid = nsi->pid;

Signature · 1 line

2.17.1

Namhyung Kim

maintainer Acked-by on v1 1/1

2018-08-10 04:55 UTC #1 lore raw

Hello,

Quoted 9 lines · Benno Evers

The argument to nsinfo__copy() was assumed to be valid, but some code paths exist that will lead to NULL being passed.
In particular, running 'perf script -D' on a perf.data file containing an PERF_RECORD_MMAP event associating the '[vdso]' dso with pid 0 earlier in the event stream will lead to a segfault.
Since all calling code is already checking for a non-null return value, just return NULL for this case as well.

Acked-by Namhyung Kim <[email protected]>

It seems you missed the Signed-off-by though.

Thanks,
Namhyung

Quoted 21 lines

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

tools/perf/util/namespaces.c +3 -0

index 5be021701f34..cf8bd123cf73 100644
@@ -139,6 +139,9 @@ struct nsinfo *nsinfo__copy(struct nsinfo *nsi)
 {
 	struct nsinfo *nnsi;
 
+	if (nsi == NULL)
+		return NULL;
+
 	nnsi = calloc(1, sizeof(*nnsi));
 	if (nnsi != NULL) {
 		nnsi->pid = nsi->pid;

Signature · 1 line

2.17.1

Arnaldo Carvalho de Melo

maintainer Acked-by Namhyung Kim

2018-08-10 13:18 UTC #2 lore raw

Em Fri, Aug 10, 2018 at 01:55:37PM +0900, Namhyung Kim escreveu:

Quoted 10 lines

Hello,
Quoted 7 lines · Benno Evers
The argument to nsinfo__copy() was assumed to be valid, but some code paths exist that will lead to NULL being passed.
In particular, running 'perf script -D' on a perf.data file containing an PERF_RECORD_MMAP event associating the '[vdso]' dso with pid 0 earlier in the event stream will lead to a segfault.

Quoted 6 lines

Quoted 2 lines
Since all calling code is already checking for a non-null return value, just return NULL for this case as well.
Acked-by Namhyung Kim <[email protected]>
It seems you missed the Signed-off-by though.

Yeah, please collect Namhyung's Acked-by and provide yours, i.e. your patch should end with the following lines:

Acked-by Namhyung Kim <[email protected]>

Signed-off-by Benno Evers <[email protected]>

My git-am scripts will collect the Cc from the e-mail message and add the Link: with the Message-id.

Thanks,

- Arnaldo

Quoted 25 lines

Thanks,
Namhyung

Quoted 21 lines

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

tools/perf/util/namespaces.c +3 -0

index 5be021701f34..cf8bd123cf73 100644
@@ -139,6 +139,9 @@ struct nsinfo *nsinfo__copy(struct nsinfo *nsi)
 {
 	struct nsinfo *nnsi;
 
+	if (nsi == NULL)
+		return NULL;
+
 	nnsi = calloc(1, sizeof(*nnsi));
 	if (nnsi != NULL) {
 		nnsi->pid = nsi->pid;

Signature · 1 line

2.17.1

Benno Evers

2018-08-10 13:38 UTC #3 lore raw

Haha, sorry, I was following some "how-to-contribute" guide out there, but it seems like I missed a step :D I sent out a new version with the missing sign-off.

Quoted 55 lines · Arnaldo Carvalho de Melo

Em Fri, Aug 10, 2018 at 01:55:37PM +0900, Namhyung Kim escreveu:
Quoted 10 lines
Hello,
Quoted 7 lines · Benno Evers
The argument to nsinfo__copy() was assumed to be valid, but some code paths exist that will lead to NULL being passed.
In particular, running 'perf script -D' on a perf.data file containing an PERF_RECORD_MMAP event associating the '[vdso]' dso with pid 0 earlier in the event stream will lead to a segfault.
Quoted 5 lines
Quoted 2 lines
Since all calling code is already checking for a non-null return value, just return NULL for this case as well.
Acked-by Namhyung Kim <[email protected]>
It seems you missed the Signed-off-by though.
Yeah, please collect Namhyung's Acked-by and provide yours, i.e. your patch should end with the following lines:
Acked-by Namhyung Kim <[email protected]>
Signed-off-by Benno Evers <[email protected]>
My git-am scripts will collect the Cc from the e-mail message and add the Link: with the Message-id.
Thanks,
- Arnaldo
Quoted 25 lines
Thanks,
Namhyung
Quoted 21 lines
---
  tools/perf/util/namespaces.c | 3 +++
  1 file changed, 3 insertions(+)
tools/perf/util/namespaces.c +3 -0
index 5be021701f34..cf8bd123cf73 100644
@@ -139,6 +139,9 @@ struct nsinfo *nsinfo__copy(struct nsinfo *nsi)
  {
  	struct nsinfo *nnsi;
 
+	if (nsi == NULL)
+		return NULL;
+
  	nnsi = calloc(1, sizeof(*nnsi));
  	if (nnsi != NULL) {
  		nnsi->pid = nsi->pid;
Signature · 1 line
2.17.1