Skip to contentSkip to conversation

perf bpf: Avoid memory leak from perf_env__insert_btf

PATCH Applied Ian Rogers 2 participants 4 messages Nov 6, 2021

Subsystems (4)
Networking Network drivers BPF perf events

v1 of 2 Applied Latest is v2 →

lore
Share & tools
lore

Ian Rogers

author reviewer
#0 lore raw

PATCH 1/2 perf bpf: Avoid memory leak from perf_env__insert_btf

perf_env__insert_btf doesn't insert if a duplicate btf id is encountered and this causes a memory leak. Modify the function to return a success/error value and then free the memory if insertion didn't happen.

Fixes 3792cb2ff43b ("perf bpf: Save BTF in a rbtree in perf_env")
Signed-off-by Ian Rogers <[email protected]>
---
 tools/perf/util/bpf-event.c | 5 ++++-
 tools/perf/util/env.c       | 5 ++++-
 tools/perf/util/env.h       | 2 +-
 3 files changed, 9 insertions(+), 3 deletions(-)
tools/perf/util/bpf-event.c +4 -1
index 1a7112a87736..0783b464777a 100644
@@ -120,7 +120,10 @@ static int perf_env__fetch_btf(struct perf_env *env,
 	node->data_size = data_size;
 	memcpy(node->data, data, data_size);
 
-	perf_env__insert_btf(env, node);
+	if (!perf_env__insert_btf(env, node)) {
+		/* Insertion failed because of a duplicate. */
+		free(node);
+	}
 	return 0;
 }
tools/perf/util/env.c +4 -1
index cf773f0dec38..5b24eb010336 100644
@@ -74,12 +74,13 @@ struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
 	return node;
 }
 
-void perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node)
+bool perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node)
 {
 	struct rb_node *parent = NULL;
 	__u32 btf_id = btf_node->id;
 	struct btf_node *node;
 	struct rb_node **p;
+	bool ret = true;
 
 	down_write(&env->bpf_progs.lock);
 	p = &env->bpf_progs.btfs.rb_node;

@@ -93,6 +94,7 @@ void perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node)
 			p = &(*p)->rb_right;
 		} else {
 			pr_debug("duplicated btf %u\n", btf_id);
+			ret = false;
 			goto out;
 		}
 	}

@@ -102,6 +104,7 @@ void perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node)
 	env->bpf_progs.btfs_cnt++;
 out:
 	up_write(&env->bpf_progs.lock);
+	return ret;
 }
 
 struct btf_node *perf_env__find_btf(struct perf_env *env, __u32 btf_id)
tools/perf/util/env.h +1 -1
index 1383876f72b3..163e5ec503a2 100644
@@ -167,7 +167,7 @@ void perf_env__insert_bpf_prog_info(struct perf_env *env,
 				    struct bpf_prog_info_node *info_node);
 struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
 							__u32 prog_id);
-void perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node);
+bool perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node);
 struct btf_node *perf_env__find_btf(struct perf_env *env, __u32 btf_id);
 
 int perf_env__numa_node(struct perf_env *env, int cpu);
Signature · 1 line
2.34.0.rc0.344.g81b53c2807-goog
author reviewer
#1 lore raw

PATCH 2/2 perf bpf: Add missing free to bpf_event__print_bpf_prog_info

If btf__new is called then there needs to be a corresponding btf__free.

Fixes f8dfeae009ef ("perf bpf: Show more BPF program info in print_bpf_prog_info()")
Signed-off-by Ian Rogers <[email protected]>
---
 tools/perf/util/bpf-event.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tools/perf/util/bpf-event.c +3 -1
index 0783b464777a..1f813d8bb946 100644
@@ -579,7 +579,7 @@ void bpf_event__print_bpf_prog_info(struct bpf_prog_info *info,
 		synthesize_bpf_prog_name(name, KSYM_NAME_LEN, info, btf, 0);
 		fprintf(fp, "# bpf_prog_info %u: %s addr 0x%llx size %u\n",
 			info->id, name, prog_addrs[0], prog_lens[0]);
-		return;
+		goto out;
 	}
 
 	fprintf(fp, "# bpf_prog_info %u:\n", info->id);

@@ -589,4 +589,6 @@ void bpf_event__print_bpf_prog_info(struct bpf_prog_info *info,
 		fprintf(fp, "# \tsub_prog %u: %s addr 0x%llx size %u\n",
 			i, name, prog_addrs[i], prog_lens[i]);
 	}
+out:
+	btf__free(btf);
 }
Signature · 1 line
2.34.0.rc0.344.g81b53c2807-goog
maintainer Applied
#2 lore raw

Re: [PATCH 2/2] perf bpf: Add missing free to bpf_event__print_bpf_prog_info

Em Fri, Nov 05, 2021 at 10:37:33PM -0700, Ian Rogers escreveu:

Quoted 1 line

If btf__new is called then there needs to be a corresponding btf__free.

Thanks, applied.

- Arnaldo

Quoted 28 lines
Fixes f8dfeae009ef ("perf bpf: Show more BPF program info in print_bpf_prog_info()")
Signed-off-by Ian Rogers <[email protected]>
---
 tools/perf/util/bpf-event.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tools/perf/util/bpf-event.c +3 -1
index 0783b464777a..1f813d8bb946 100644
@@ -579,7 +579,7 @@ void bpf_event__print_bpf_prog_info(struct bpf_prog_info *info,
 		synthesize_bpf_prog_name(name, KSYM_NAME_LEN, info, btf, 0);
 		fprintf(fp, "# bpf_prog_info %u: %s addr 0x%llx size %u\n",
 			info->id, name, prog_addrs[0], prog_lens[0]);
-		return;
+		goto out;
 	}
 
 	fprintf(fp, "# bpf_prog_info %u:\n", info->id);

@@ -589,4 +589,6 @@ void bpf_event__print_bpf_prog_info(struct bpf_prog_info *info,
 		fprintf(fp, "# \tsub_prog %u: %s addr 0x%llx size %u\n",
 			i, name, prog_addrs[i], prog_lens[i]);
 	}
+out:
+	btf__free(btf);
 }
Signature · 1 line
2.34.0.rc0.344.g81b53c2807-goog
Signature · 1 line
- Arnaldo
maintainer
#3 lore raw
In reply to Ian Rogers

Em Fri, Nov 05, 2021 at 10:37:32PM -0700, Ian Rogers escreveu:

Quoted 18 lines

perf_env__insert_btf doesn't insert if a duplicate btf id is encountered and this causes a memory leak. Modify the function to return a success/error value and then free the memory if insertion didn't happen.

Fixes 3792cb2ff43b ("perf bpf: Save BTF in a rbtree in perf_env")
Signed-off-by Ian Rogers <[email protected]>
---
 tools/perf/util/bpf-event.c | 5 ++++-
 tools/perf/util/env.c       | 5 ++++-
 tools/perf/util/env.h       | 2 +-
 3 files changed, 9 insertions(+), 3 deletions(-)
tools/perf/util/bpf-event.c +0 -0
index 1a7112a87736..0783b464777a 100644

tools/perf/util/bpf-event.c static int perf_env__fetch_btf(struct perf_env *env, · line 127

Quoted 9 lines
tools/perf/util/bpf-event.c +4 -1
@@ -120,7 +120,10 @@ static int perf_env__fetch_btf(struct perf_env *env,
 	node->data_size = data_size;
 	memcpy(node->data, data, data_size);
 
-	perf_env__insert_btf(env, node);
+	if (!perf_env__insert_btf(env, node)) {
+		/* Insertion failed because of a duplicate. */
+		free(node);
+	}
 	return 0;

Shouldn't this error be propagated? Song?

- Arnaldo

Quoted 52 lines
tools/perf/util/bpf-event.c +0 -0
 }
tools/perf/util/env.c +4 -1
index cf773f0dec38..5b24eb010336 100644
@@ -74,12 +74,13 @@ struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
 	return node;
 }
 
-void perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node)
+bool perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node)
 {
 	struct rb_node *parent = NULL;
 	__u32 btf_id = btf_node->id;
 	struct btf_node *node;
 	struct rb_node **p;
+	bool ret = true;
 
 	down_write(&env->bpf_progs.lock);
 	p = &env->bpf_progs.btfs.rb_node;

@@ -93,6 +94,7 @@ void perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node)
 			p = &(*p)->rb_right;
 		} else {
 			pr_debug("duplicated btf %u\n", btf_id);
+			ret = false;
 			goto out;
 		}
 	}

@@ -102,6 +104,7 @@ void perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node)
 	env->bpf_progs.btfs_cnt++;
 out:
 	up_write(&env->bpf_progs.lock);
+	return ret;
 }
 
 struct btf_node *perf_env__find_btf(struct perf_env *env, __u32 btf_id)
tools/perf/util/env.h +1 -1
index 1383876f72b3..163e5ec503a2 100644
@@ -167,7 +167,7 @@ void perf_env__insert_bpf_prog_info(struct perf_env *env,
 				    struct bpf_prog_info_node *info_node);
 struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
 							__u32 prog_id);
-void perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node);
+bool perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node);
 struct btf_node *perf_env__find_btf(struct perf_env *env, __u32 btf_id);
 
 int perf_env__numa_node(struct perf_env *env, int cpu);
Signature · 1 line
2.34.0.rc0.344.g81b53c2807-goog
Signature · 1 line
- Arnaldo