Re: [PATCH] perf header: Fix memory leaks

From: Jiri Olsa
Date: Sun Nov 28 2021 - 10:49:03 EST


On Thu, Nov 18, 2021 at 12:17:30PM -0800, Ian Rogers wrote:
> These leaks were found with leak sanitizer running "perf pipe recording
> and injection test". In pipe mode feat_fd may hold onto an events struct
> that needs freeing. When string features are processed they may
> overwrite an already created string, so free this before the overwrite.
>
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
> tools/perf/util/header.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 79cce216727e..e3c1a532d059 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2321,6 +2321,7 @@ static int perf_header__read_build_ids(struct perf_header *header,
> #define FEAT_PROCESS_STR_FUN(__feat, __feat_env) \
> static int process_##__feat(struct feat_fd *ff, void *data __maybe_unused) \
> {\
> + free(ff->ph->env.__feat_env); \

hm, how is this set before this callback is triggered?

jirka

> ff->ph->env.__feat_env = do_read_string(ff); \
> return ff->ph->env.__feat_env ? 0 : -ENOMEM; \
> }
> @@ -4124,6 +4125,7 @@ int perf_event__process_feature(struct perf_session *session,
> struct perf_record_header_feature *fe = (struct perf_record_header_feature *)event;
> int type = fe->header.type;
> u64 feat = fe->feat_id;
> + int ret = 0;
>
> if (type < 0 || type >= PERF_RECORD_HEADER_MAX) {
> pr_warning("invalid record type %d in pipe-mode\n", type);
> @@ -4141,11 +4143,13 @@ int perf_event__process_feature(struct perf_session *session,
> ff.size = event->header.size - sizeof(*fe);
> ff.ph = &session->header;
>
> - if (feat_ops[feat].process(&ff, NULL))
> - return -1;
> + if (feat_ops[feat].process(&ff, NULL)) {
> + ret = -1;
> + goto out;
> + }
>
> if (!feat_ops[feat].print || !tool->show_feat_hdr)
> - return 0;
> + goto out;
>
> if (!feat_ops[feat].full_only ||
> tool->show_feat_hdr >= SHOW_FEAT_HEADER_FULL_INFO) {
> @@ -4154,8 +4158,9 @@ int perf_event__process_feature(struct perf_session *session,
> fprintf(stdout, "# %s info available, use -I to display\n",
> feat_ops[feat].name);
> }
> -
> - return 0;
> +out:
> + free_event_desc(ff.events);
> + return ret;
> }
>
> size_t perf_event__fprintf_event_update(union perf_event *event, FILE *fp)
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>