| Message ID | 20211106053733.3580931-1-irogers@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [1/2] perf bpf: Avoid memory leak from perf_env__insert_btf | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf | success | VM_Test |
| bpf/vmtest-bpf-PR | success | PR summary |
| bpf/vmtest-bpf-next | success | VM_Test |
| netdev/tree_selection | success | Not a local patch |
Em Fri, Nov 05, 2021 at 10:37:32PM -0700, Ian Rogers escreveu: > 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 <irogers@google.com> > --- > 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(-) > > diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c > index 1a7112a87736..0783b464777a 100644 > --- a/tools/perf/util/bpf-event.c > +++ b/tools/perf/util/bpf-event.c > @@ -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 > } > > diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c > index cf773f0dec38..5b24eb010336 100644 > --- a/tools/perf/util/env.c > +++ b/tools/perf/util/env.c > @@ -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) > diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h > index 1383876f72b3..163e5ec503a2 100644 > --- a/tools/perf/util/env.h > +++ b/tools/perf/util/env.h > @@ -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); > -- > 2.34.0.rc0.344.g81b53c2807-goog
diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c index 1a7112a87736..0783b464777a 100644 --- a/tools/perf/util/bpf-event.c +++ b/tools/perf/util/bpf-event.c @@ -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; } diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c index cf773f0dec38..5b24eb010336 100644 --- a/tools/perf/util/env.c +++ b/tools/perf/util/env.c @@ -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) diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h index 1383876f72b3..163e5ec503a2 100644 --- a/tools/perf/util/env.h +++ b/tools/perf/util/env.h @@ -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);
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 <irogers@google.com> --- 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(-)