All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] perf bpf: Avoid memory leak from perf_env__insert_btf
@ 2021-11-06  5:37 Ian Rogers
  2021-11-06  5:37 ` [PATCH 2/2] perf bpf: Add missing free to bpf_event__print_bpf_prog_info Ian Rogers
  2021-11-06 19:01 ` [PATCH 1/2] perf bpf: Avoid memory leak from perf_env__insert_btf Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 4+ messages in thread
From: Ian Rogers @ 2021-11-06  5:37 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Song Liu, linux-kernel, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Yonghong Song, John Fastabend,
	KP Singh, Tiezhu Yang, linux-perf-users, netdev, bpf
  Cc: Stephane Eranian, Ian Rogers

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;
 }
 
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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] perf bpf: Add missing free to bpf_event__print_bpf_prog_info
  2021-11-06  5:37 [PATCH 1/2] perf bpf: Avoid memory leak from perf_env__insert_btf Ian Rogers
@ 2021-11-06  5:37 ` Ian Rogers
  2021-11-06 19:03   ` Arnaldo Carvalho de Melo
  2021-11-06 19:01 ` [PATCH 1/2] perf bpf: Avoid memory leak from perf_env__insert_btf Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2021-11-06  5:37 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Song Liu, linux-kernel, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Yonghong Song, John Fastabend,
	KP Singh, Tiezhu Yang, linux-perf-users, netdev, bpf
  Cc: Stephane Eranian, Ian Rogers

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 <irogers@google.com>
---
 tools/perf/util/bpf-event.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index 0783b464777a..1f813d8bb946 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -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);
 }
-- 
2.34.0.rc0.344.g81b53c2807-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] perf bpf: Avoid memory leak from perf_env__insert_btf
  2021-11-06  5:37 [PATCH 1/2] perf bpf: Avoid memory leak from perf_env__insert_btf Ian Rogers
  2021-11-06  5:37 ` [PATCH 2/2] perf bpf: Add missing free to bpf_event__print_bpf_prog_info Ian Rogers
@ 2021-11-06 19:01 ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-11-06 19:01 UTC (permalink / raw)
  To: Ian Rogers, Song Liu
  Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, linux-kernel, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Yonghong Song, John Fastabend, KP Singh, Tiezhu Yang,
	linux-perf-users, netdev, bpf, Stephane Eranian

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

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] perf bpf: Add missing free to bpf_event__print_bpf_prog_info
  2021-11-06  5:37 ` [PATCH 2/2] perf bpf: Add missing free to bpf_event__print_bpf_prog_info Ian Rogers
@ 2021-11-06 19:03   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-11-06 19:03 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Song Liu, linux-kernel,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Yonghong Song, John Fastabend, KP Singh,
	Tiezhu Yang, linux-perf-users, netdev, bpf, Stephane Eranian

Em Fri, Nov 05, 2021 at 10:37:33PM -0700, Ian Rogers escreveu:
> If btf__new is called then there needs to be a corresponding btf__free.


Thanks, applied.

- Arnaldo


 
> Fixes: f8dfeae009ef ("perf bpf: Show more BPF program info in print_bpf_prog_info()")
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/bpf-event.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index 0783b464777a..1f813d8bb946 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -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);
>  }
> -- 
> 2.34.0.rc0.344.g81b53c2807-goog

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-06 19:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-06  5:37 [PATCH 1/2] perf bpf: Avoid memory leak from perf_env__insert_btf Ian Rogers
2021-11-06  5:37 ` [PATCH 2/2] perf bpf: Add missing free to bpf_event__print_bpf_prog_info Ian Rogers
2021-11-06 19:03   ` Arnaldo Carvalho de Melo
2021-11-06 19:01 ` [PATCH 1/2] perf bpf: Avoid memory leak from perf_env__insert_btf Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.