perf tools: Fix perf_env__find_bpf_prog_info rb tree lookup

PATCH In review Jiri Olsa 5 participants 8 messages Apr 16, 2019 – Apr 19, 2019 perf events

lore

Share & tools

lore

Jiri Olsa

author reviewer

2019-04-16 13:41 UTC #0 lore raw

PATCH 1/1 perf tools: Fix perf_env__find_bpf_prog_info rb tree lookup

We currently don't return NULL in case we don't find the bpf_prog_info_node, fixing that.

Cc Song Liu <[email protected]>

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

Signed-off-by Jiri Olsa <[email protected]>

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

tools/perf/util/env.c +3 -1

index c6351b557bb0..34a363f2e71b 100644
@@ -57,9 +57,11 @@ struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
 		else if (prog_id > node->info_linear->info.id)
 			n = n->rb_right;
 		else
-			break;
+			goto out;
 	}
+	node = NULL;
 
+out:
 	up_read(&env->bpf_progs.lock);
 	return node;
 }

Signature · 1 line

2.17.2

Arnaldo Carvalho de Melo

2019-04-16 14:20 UTC #1 lore raw

Em Tue, Apr 16, 2019 at 03:41:51PM +0200, Jiri Olsa escreveu:

Quoted 2 lines

We currently don't return NULL in case we don't find the bpf_prog_info_node, fixing that.

Can you please try to figure out what was the changeset that introduced this problem so that we can have a Fixes: tag that helps me determine if this should go to perf/urgent (likely) or (perf/core) and to help the stable guys as well?

Thanks,

- Arnaldo

Quoted 26 lines

Cc Song Liu <[email protected]>

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

Signed-off-by Jiri Olsa <[email protected]>

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

tools/perf/util/env.c +3 -1

index c6351b557bb0..34a363f2e71b 100644
@@ -57,9 +57,11 @@ struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
 		else if (prog_id > node->info_linear->info.id)
 			n = n->rb_right;
 		else
-			break;
+			goto out;
 	}
+	node = NULL;
 
+out:
 	up_read(&env->bpf_progs.lock);
 	return node;
 }

Signature · 1 line

2.17.2

Signature · 1 line

- Arnaldo

Jiri Olsa

2019-04-16 14:34 UTC #2 lore raw

Quoted 8 lines · Arnaldo Carvalho de Melo

Em Tue, Apr 16, 2019 at 03:41:51PM +0200, Jiri Olsa escreveu:
Quoted 2 lines
We currently don't return NULL in case we don't find the bpf_prog_info_node, fixing that.
Can you please try to figure out what was the changeset that introduced this problem so that we can have a Fixes: tag that helps me determine if this should go to perf/urgent (likely) or (perf/core) and to help the stable guys as well?

sry, I keep forgetting this..

Fixes e4378f0cb90b ("perf bpf: Save bpf_prog_info in a rbtree in perf_env")

thanks,
jirka

Quoted 35 lines

Thanks,
- Arnaldo
Quoted 26 lines
Cc Song Liu <[email protected]>
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/util/env.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tools/perf/util/env.c +3 -1
index c6351b557bb0..34a363f2e71b 100644
@@ -57,9 +57,11 @@ struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
 		else if (prog_id > node->info_linear->info.id)
 			n = n->rb_right;
 		else
-			break;
+			goto out;
 	}
+	node = NULL;
 
+out:
 	up_read(&env->bpf_progs.lock);
 	return node;
 }
Signature · 1 line
2.17.2
Signature · 1 line
- Arnaldo

Arnaldo Carvalho de Melo

2019-04-16 14:36 UTC #3 lore raw

Em Tue, Apr 16, 2019 at 04:34:52PM +0200, Jiri Olsa escreveu:

Quoted 5 lines · Arnaldo Carvalho de Melo

Quoted 4 lines
Em Tue, Apr 16, 2019 at 03:41:51PM +0200, Jiri Olsa escreveu:
Quoted 2 lines
We currently don't return NULL in case we don't find the bpf_prog_info_node, fixing that.

Quoted 8 lines

Quoted 4 lines
Can you please try to figure out what was the changeset that introduced this problem so that we can have a Fixes: tag that helps me determine if this should go to perf/urgent (likely) or (perf/core) and to help the stable guys as well?
sry, I keep forgetting this..
Fixes e4378f0cb90b ("perf bpf: Save bpf_prog_info in a rbtree in perf_env")

I had just added:

Fixes e4378f0cb90b ("perf bpf: Save bpf_prog_info in a rbtree in perf_env")

matches, good :-)

At firts I thought hey the look checks for NULL, then, hey, different variable... good catch!

Thanks,

Quoted 38 lines

thanks,
jirka
Quoted 35 lines
Thanks,
- Arnaldo
Quoted 26 lines
Cc Song Liu <[email protected]>
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/util/env.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tools/perf/util/env.c +3 -1
index c6351b557bb0..34a363f2e71b 100644
@@ -57,9 +57,11 @@ struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
 		else if (prog_id > node->info_linear->info.id)
 			n = n->rb_right;
 		else
-			break;
+			goto out;
 	}
+	node = NULL;
 
+out:
 	up_read(&env->bpf_progs.lock);
 	return node;
 }
Signature · 1 line
2.17.2
Signature · 1 line
- Arnaldo

Signature · 1 line

- Arnaldo

Arnaldo Carvalho de Melo

2019-04-16 14:37 UTC #4 lore raw

Em Tue, Apr 16, 2019 at 11:35:54AM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 15 lines

Em Tue, Apr 16, 2019 at 04:34:52PM +0200, Jiri Olsa escreveu:
Quoted 5 lines · Arnaldo Carvalho de Melo
Quoted 4 lines
Em Tue, Apr 16, 2019 at 03:41:51PM +0200, Jiri Olsa escreveu:
Quoted 2 lines
We currently don't return NULL in case we don't find the bpf_prog_info_node, fixing that.
Quoted 8 lines
Quoted 4 lines
Can you please try to figure out what was the changeset that introduced this problem so that we can have a Fixes: tag that helps me determine if this should go to perf/urgent (likely) or (perf/core) and to help the stable guys as well?
sry, I keep forgetting this..
Fixes e4378f0cb90b ("perf bpf: Save bpf_prog_info in a rbtree in perf_env")

Quoted 8 lines

I had just added:
Fixes e4378f0cb90b ("perf bpf: Save bpf_prog_info in a rbtree in perf_env")
matches, good :-)
At firts I thought hey the look checks for NULL, then, hey, different variable... good catch!

Ah, I also changed the summary from:

"perf tools: Fix perf_env__find_bpf_prog_info rb tree"

to:

perf bpf: Return NULL when RB tree lookup fails in perf_env__find_bpf_prog_info()

Thanks,

- Arnaldo

Song Liu

2019-04-16 20:25 UTC #5 lore raw

Quoted 33 lines · Arnaldo Carvalho de Melo

Em Tue, Apr 16, 2019 at 11:35:54AM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 15 lines
Em Tue, Apr 16, 2019 at 04:34:52PM +0200, Jiri Olsa escreveu:
Quoted 5 lines · Arnaldo Carvalho de Melo
Quoted 4 lines
Em Tue, Apr 16, 2019 at 03:41:51PM +0200, Jiri Olsa escreveu:
Quoted 2 lines
We currently don't return NULL in case we don't find the bpf_prog_info_node, fixing that.
Quoted 8 lines
Quoted 4 lines
Can you please try to figure out what was the changeset that introduced this problem so that we can have a Fixes: tag that helps me determine if this should go to perf/urgent (likely) or (perf/core) and to help the stable guys as well?
sry, I keep forgetting this..
Fixes e4378f0cb90b ("perf bpf: Save bpf_prog_info in a rbtree in perf_env")
Quoted 8 lines
I had just added:
Fixes e4378f0cb90b ("perf bpf: Save bpf_prog_info in a rbtree in perf_env")
matches, good :-)
At firts I thought hey the look checks for NULL, then, hey, different variable... good catch!
Ah, I also changed the summary from:
"perf tools: Fix perf_env__find_bpf_prog_info rb tree"

Quoted 7 lines

to:
perf bpf: Return NULL when RB tree lookup fails in perf_env__find_bpf_prog_info()
Thanks,
- Arnaldo

Thanks Jiri and Arnaldo!

I think we will need similar fix for perf_env__find_btf().

Song

Jiri Olsa

2019-04-17 07:38 UTC #6 lore raw

Quoted 37 lines · Song Liu

Quoted 37 lines · Arnaldo Carvalho de Melo
Em Tue, Apr 16, 2019 at 11:35:54AM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 15 lines
Em Tue, Apr 16, 2019 at 04:34:52PM +0200, Jiri Olsa escreveu:
Quoted 5 lines · Arnaldo Carvalho de Melo
Quoted 4 lines
Em Tue, Apr 16, 2019 at 03:41:51PM +0200, Jiri Olsa escreveu:
Quoted 2 lines
We currently don't return NULL in case we don't find the bpf_prog_info_node, fixing that.
Quoted 8 lines
Quoted 4 lines
Can you please try to figure out what was the changeset that introduced this problem so that we can have a Fixes: tag that helps me determine if this should go to perf/urgent (likely) or (perf/core) and to help the stable guys as well?
sry, I keep forgetting this..
Fixes e4378f0cb90b ("perf bpf: Save bpf_prog_info in a rbtree in perf_env")
Quoted 8 lines
I had just added:
Fixes e4378f0cb90b ("perf bpf: Save bpf_prog_info in a rbtree in perf_env")
matches, good :-)
At firts I thought hey the look checks for NULL, then, hey, different variable... good catch!
Ah, I also changed the summary from:
"perf tools: Fix perf_env__find_bpf_prog_info rb tree"
to:
perf bpf: Return NULL when RB tree lookup fails in perf_env__find_bpf_prog_info()

Quoted 7 lines

Quoted 3 lines
Thanks,
- Arnaldo
Thanks Jiri and Arnaldo!
I think we will need similar fix for perf_env__find_btf().

right, I'll post it

thanks,
jirka

tip-bot for Jiri Olsa

merge bot

2019-04-19 19:06 UTC #7 lore raw

[tip:perf/urgent] perf bpf: Return NULL when RB tree lookup fails in perf_env__find_bpf_prog_info()

In reply to Jiri Olsa

perf bpf: Return NULL when RB tree lookup fails in perf_env__find_bpf_prog_info()

Commit-ID:  aa52660231410b13d237299e691c43e346e3a73f
Gitweb:     https://git.kernel.org/tip/aa52660231410b13d237299e691c43e346e3a73f
Author:     Jiri Olsa <[email protected]>
AuthorDate: Tue, 16 Apr 2019 15:41:51 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 17 Apr 2019 14:30:06 -0300

perf bpf: Return NULL when RB tree lookup fails in perf_env__find_bpf_prog_info()

We currently don't return NULL in case we don't find the bpf_prog_info_node, fixing that.

Signed-off-by Jiri Olsa <[email protected]>

Acked-by Song Liu <[email protected]>

Cc Alexander Shishkin <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Fixes e4378f0cb90b ("perf bpf: Save bpf_prog_info in a rbtree in perf_env")

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

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

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

tools/perf/util/env.c +3 -1

index c6351b557bb0..34a363f2e71b 100644
@@ -57,9 +57,11 @@ struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
 		else if (prog_id > node->info_linear->info.id)
 			n = n->rb_right;
 		else
-			break;
+			goto out;
 	}
+	node = NULL;
 
+out:
 	up_read(&env->bpf_progs.lock);
 	return node;
 }