Memory leaks were detected by clang-tidy.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/header.c | 63 ++++++++++++++++++++++++----------------
1 file changed, 38 insertions(+), 25 deletions(-)
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index d812e1e371a7..41b78e40b22b 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2598,8 +2598,10 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
goto error;
/* include a NULL character at the end */
- if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
+ if (strbuf_add(&sb, str, strlen(str) + 1) < 0) {
+ free(str);
goto error;
+ }
size += string_size(str);
free(str);
}
@@ -2617,8 +2619,10 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
goto error;
/* include a NULL character at the end */
- if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
+ if (strbuf_add(&sb, str, strlen(str) + 1) < 0) {
+ free(str);
goto error;
+ }
size += string_size(str);
free(str);
}
@@ -2681,8 +2685,10 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
goto error;
/* include a NULL character at the end */
- if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
+ if (strbuf_add(&sb, str, strlen(str) + 1) < 0) {
+ free(str);
goto error;
+ }
size += string_size(str);
free(str);
}
@@ -2736,10 +2742,9 @@ static int process_numa_topology(struct feat_fd *ff, void *data __maybe_unused)
goto error;
n->map = perf_cpu_map__new(str);
+ free(str);
if (!n->map)
goto error;
-
- free(str);
}
ff->ph->env.nr_numa_nodes = nr;
ff->ph->env.numa_nodes = nodes;
@@ -2913,10 +2918,10 @@ static int process_cache(struct feat_fd *ff, void *data __maybe_unused)
return -1;
for (i = 0; i < cnt; i++) {
- struct cpu_cache_level c;
+ struct cpu_cache_level *c = &caches[i];
#define _R(v) \
- if (do_read_u32(ff, &c.v))\
+ if (do_read_u32(ff, &c->v)) \
goto out_free_caches; \
_R(level)
@@ -2926,22 +2931,25 @@ static int process_cache(struct feat_fd *ff, void *data __maybe_unused)
#undef _R
#define _R(v) \
- c.v = do_read_string(ff); \
- if (!c.v) \
- goto out_free_caches;
+ c->v = do_read_string(ff); \
+ if (!c->v) \
+ goto out_free_caches; \
_R(type)
_R(size)
_R(map)
#undef _R
-
- caches[i] = c;
}
ff->ph->env.caches = caches;
ff->ph->env.caches_cnt = cnt;
return 0;
out_free_caches:
+ for (i = 0; i < cnt; i++) {
+ free(caches[i].type);
+ free(caches[i].size);
+ free(caches[i].map);
+ }
free(caches);
return -1;
}
@@ -3585,18 +3593,16 @@ static int perf_header__adds_write(struct perf_header *header,
struct feat_copier *fc)
{
int nr_sections;
- struct feat_fd ff;
+ struct feat_fd ff = {
+ .fd = fd,
+ .ph = header,
+ };
struct perf_file_section *feat_sec, *p;
int sec_size;
u64 sec_start;
int feat;
int err;
- ff = (struct feat_fd){
- .fd = fd,
- .ph = header,
- };
-
nr_sections = bitmap_weight(header->adds_features, HEADER_FEAT_BITS);
if (!nr_sections)
return 0;
@@ -3623,6 +3629,7 @@ static int perf_header__adds_write(struct perf_header *header,
err = do_write(&ff, feat_sec, sec_size);
if (err < 0)
pr_debug("failed to write feature section\n");
+ free(ff.buf);
free(feat_sec);
return err;
}
@@ -3630,11 +3637,11 @@ static int perf_header__adds_write(struct perf_header *header,
int perf_header__write_pipe(int fd)
{
struct perf_pipe_file_header f_header;
- struct feat_fd ff;
+ struct feat_fd ff = {
+ .fd = fd,
+ };
int err;
- ff = (struct feat_fd){ .fd = fd };
-
f_header = (struct perf_pipe_file_header){
.magic = PERF_MAGIC,
.size = sizeof(f_header),
@@ -3645,7 +3652,7 @@ int perf_header__write_pipe(int fd)
pr_debug("failed to write perf pipe header\n");
return err;
}
-
+ free(ff.buf);
return 0;
}
@@ -3658,11 +3665,12 @@ static int perf_session__do_write_header(struct perf_session *session,
struct perf_file_attr f_attr;
struct perf_header *header = &session->header;
struct evsel *evsel;
- struct feat_fd ff;
+ struct feat_fd ff = {
+ .fd = fd,
+ };
u64 attr_offset;
int err;
- ff = (struct feat_fd){ .fd = fd};
lseek(fd, sizeof(f_header), SEEK_SET);
evlist__for_each_entry(session->evlist, evsel) {
@@ -3670,6 +3678,7 @@ static int perf_session__do_write_header(struct perf_session *session,
err = do_write(&ff, evsel->core.id, evsel->core.ids * sizeof(u64));
if (err < 0) {
pr_debug("failed to write perf header\n");
+ free(ff.buf);
return err;
}
}
@@ -3695,6 +3704,7 @@ static int perf_session__do_write_header(struct perf_session *session,
err = do_write(&ff, &f_attr, sizeof(f_attr));
if (err < 0) {
pr_debug("failed to write perf header attribute\n");
+ free(ff.buf);
return err;
}
}
@@ -3705,8 +3715,10 @@ static int perf_session__do_write_header(struct perf_session *session,
if (at_exit) {
err = perf_header__adds_write(header, evlist, fd, fc);
- if (err < 0)
+ if (err < 0) {
+ free(ff.buf);
return err;
+ }
}
f_header = (struct perf_file_header){
@@ -3728,6 +3740,7 @@ static int perf_session__do_write_header(struct perf_session *session,
lseek(fd, 0, SEEK_SET);
err = do_write(&ff, &f_header, sizeof(f_header));
+ free(ff.buf);
if (err < 0) {
pr_debug("failed to write perf header\n");
return err;
--
2.42.0.609.gbb76f46606-googOn Thu, Oct 5, 2023 at 4:09 PM Ian Rogers <irogers@google.com> wrote:
>
> Memory leaks were detected by clang-tidy.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/header.c | 63 ++++++++++++++++++++++++----------------
> 1 file changed, 38 insertions(+), 25 deletions(-)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index d812e1e371a7..41b78e40b22b 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2598,8 +2598,10 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
> goto error;
>
> /* include a NULL character at the end */
> - if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
> + if (strbuf_add(&sb, str, strlen(str) + 1) < 0) {
> + free(str);
> goto error;
> + }
> size += string_size(str);
> free(str);
> }
> @@ -2617,8 +2619,10 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
> goto error;
>
> /* include a NULL character at the end */
> - if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
> + if (strbuf_add(&sb, str, strlen(str) + 1) < 0) {
> + free(str);
> goto error;
> + }
> size += string_size(str);
> free(str);
> }
> @@ -2681,8 +2685,10 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
> goto error;
>
> /* include a NULL character at the end */
> - if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
> + if (strbuf_add(&sb, str, strlen(str) + 1) < 0) {
> + free(str);
> goto error;
> + }
> size += string_size(str);
> free(str);
> }
For these cases, it'd be simpler to free it in the error path.
> @@ -2736,10 +2742,9 @@ static int process_numa_topology(struct feat_fd *ff, void *data __maybe_unused)
> goto error;
>
> n->map = perf_cpu_map__new(str);
> + free(str);
> if (!n->map)
> goto error;
> -
> - free(str);
> }
> ff->ph->env.nr_numa_nodes = nr;
> ff->ph->env.numa_nodes = nodes;
> @@ -2913,10 +2918,10 @@ static int process_cache(struct feat_fd *ff, void *data __maybe_unused)
> return -1;
>
> for (i = 0; i < cnt; i++) {
> - struct cpu_cache_level c;
> + struct cpu_cache_level *c = &caches[i];
>
> #define _R(v) \
> - if (do_read_u32(ff, &c.v))\
> + if (do_read_u32(ff, &c->v)) \
> goto out_free_caches; \
>
> _R(level)
> @@ -2926,22 +2931,25 @@ static int process_cache(struct feat_fd *ff, void *data __maybe_unused)
> #undef _R
>
> #define _R(v) \
> - c.v = do_read_string(ff); \
> - if (!c.v) \
> - goto out_free_caches;
> + c->v = do_read_string(ff); \
> + if (!c->v) \
> + goto out_free_caches; \
>
> _R(type)
> _R(size)
> _R(map)
> #undef _R
> -
> - caches[i] = c;
> }
>
> ff->ph->env.caches = caches;
> ff->ph->env.caches_cnt = cnt;
> return 0;
> out_free_caches:
> + for (i = 0; i < cnt; i++) {
> + free(caches[i].type);
> + free(caches[i].size);
> + free(caches[i].map);
> + }
> free(caches);
> return -1;
> }
Looks ok.
> @@ -3585,18 +3593,16 @@ static int perf_header__adds_write(struct perf_header *header,
> struct feat_copier *fc)
> {
> int nr_sections;
> - struct feat_fd ff;
> + struct feat_fd ff = {
> + .fd = fd,
> + .ph = header,
> + };
I'm fine with this change.
> struct perf_file_section *feat_sec, *p;
> int sec_size;
> u64 sec_start;
> int feat;
> int err;
>
> - ff = (struct feat_fd){
> - .fd = fd,
> - .ph = header,
> - };
> -
> nr_sections = bitmap_weight(header->adds_features, HEADER_FEAT_BITS);
> if (!nr_sections)
> return 0;
> @@ -3623,6 +3629,7 @@ static int perf_header__adds_write(struct perf_header *header,
> err = do_write(&ff, feat_sec, sec_size);
> if (err < 0)
> pr_debug("failed to write feature section\n");
> + free(ff.buf);
But it looks like false alarams. Since the feat_fd has fd
and no buf, it won't allocate the buffer in do_write() and
just use __do_write_fd(). So no need to free the buf.
Thanks,
Namhyung
> free(feat_sec);
> return err;
> }
> @@ -3630,11 +3637,11 @@ static int perf_header__adds_write(struct perf_header *header,
> int perf_header__write_pipe(int fd)
> {
> struct perf_pipe_file_header f_header;
> - struct feat_fd ff;
> + struct feat_fd ff = {
> + .fd = fd,
> + };
> int err;
>
> - ff = (struct feat_fd){ .fd = fd };
> -
> f_header = (struct perf_pipe_file_header){
> .magic = PERF_MAGIC,
> .size = sizeof(f_header),
> @@ -3645,7 +3652,7 @@ int perf_header__write_pipe(int fd)
> pr_debug("failed to write perf pipe header\n");
> return err;
> }
> -
> + free(ff.buf);
> return 0;
> }
>
> @@ -3658,11 +3665,12 @@ static int perf_session__do_write_header(struct perf_session *session,
> struct perf_file_attr f_attr;
> struct perf_header *header = &session->header;
> struct evsel *evsel;
> - struct feat_fd ff;
> + struct feat_fd ff = {
> + .fd = fd,
> + };
> u64 attr_offset;
> int err;
>
> - ff = (struct feat_fd){ .fd = fd};
> lseek(fd, sizeof(f_header), SEEK_SET);
>
> evlist__for_each_entry(session->evlist, evsel) {
> @@ -3670,6 +3678,7 @@ static int perf_session__do_write_header(struct perf_session *session,
> err = do_write(&ff, evsel->core.id, evsel->core.ids * sizeof(u64));
> if (err < 0) {
> pr_debug("failed to write perf header\n");
> + free(ff.buf);
> return err;
> }
> }
> @@ -3695,6 +3704,7 @@ static int perf_session__do_write_header(struct perf_session *session,
> err = do_write(&ff, &f_attr, sizeof(f_attr));
> if (err < 0) {
> pr_debug("failed to write perf header attribute\n");
> + free(ff.buf);
> return err;
> }
> }
> @@ -3705,8 +3715,10 @@ static int perf_session__do_write_header(struct perf_session *session,
>
> if (at_exit) {
> err = perf_header__adds_write(header, evlist, fd, fc);
> - if (err < 0)
> + if (err < 0) {
> + free(ff.buf);
> return err;
> + }
> }
>
> f_header = (struct perf_file_header){
> @@ -3728,6 +3740,7 @@ static int perf_session__do_write_header(struct perf_session *session,
>
> lseek(fd, 0, SEEK_SET);
> err = do_write(&ff, &f_header, sizeof(f_header));
> + free(ff.buf);
> if (err < 0) {
> pr_debug("failed to write perf header\n");
> return err;
> --
> 2.42.0.609.gbb76f46606-goog
>
On Sun, Oct 8, 2023 at 11:57 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Thu, Oct 5, 2023 at 4:09 PM Ian Rogers <irogers@google.com> wrote:
> >
> > Memory leaks were detected by clang-tidy.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/util/header.c | 63 ++++++++++++++++++++++++----------------
> > 1 file changed, 38 insertions(+), 25 deletions(-)
> >
> > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> > index d812e1e371a7..41b78e40b22b 100644
> > --- a/tools/perf/util/header.c
> > +++ b/tools/perf/util/header.c
> > @@ -2598,8 +2598,10 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
> > goto error;
> >
> > /* include a NULL character at the end */
> > - if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
> > + if (strbuf_add(&sb, str, strlen(str) + 1) < 0) {
> > + free(str);
> > goto error;
> > + }
> > size += string_size(str);
> > free(str);
> > }
> > @@ -2617,8 +2619,10 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
> > goto error;
> >
> > /* include a NULL character at the end */
> > - if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
> > + if (strbuf_add(&sb, str, strlen(str) + 1) < 0) {
> > + free(str);
> > goto error;
> > + }
> > size += string_size(str);
> > free(str);
> > }
> > @@ -2681,8 +2685,10 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
> > goto error;
> >
> > /* include a NULL character at the end */
> > - if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
> > + if (strbuf_add(&sb, str, strlen(str) + 1) < 0) {
> > + free(str);
> > goto error;
> > + }
> > size += string_size(str);
> > free(str);
> > }
>
> For these cases, it'd be simpler to free it in the error path.
It was a slightly larger change to do it that way, but I'll alter it.
The issue is not getting a double free on str, which is most easily
handled by switching frees to zfrees.
>
>
> > @@ -2736,10 +2742,9 @@ static int process_numa_topology(struct feat_fd *ff, void *data __maybe_unused)
> > goto error;
> >
> > n->map = perf_cpu_map__new(str);
> > + free(str);
> > if (!n->map)
> > goto error;
> > -
> > - free(str);
> > }
> > ff->ph->env.nr_numa_nodes = nr;
> > ff->ph->env.numa_nodes = nodes;
> > @@ -2913,10 +2918,10 @@ static int process_cache(struct feat_fd *ff, void *data __maybe_unused)
> > return -1;
> >
> > for (i = 0; i < cnt; i++) {
> > - struct cpu_cache_level c;
> > + struct cpu_cache_level *c = &caches[i];
> >
> > #define _R(v) \
> > - if (do_read_u32(ff, &c.v))\
> > + if (do_read_u32(ff, &c->v)) \
> > goto out_free_caches; \
> >
> > _R(level)
> > @@ -2926,22 +2931,25 @@ static int process_cache(struct feat_fd *ff, void *data __maybe_unused)
> > #undef _R
> >
> > #define _R(v) \
> > - c.v = do_read_string(ff); \
> > - if (!c.v) \
> > - goto out_free_caches;
> > + c->v = do_read_string(ff); \
> > + if (!c->v) \
> > + goto out_free_caches; \
> >
> > _R(type)
> > _R(size)
> > _R(map)
> > #undef _R
> > -
> > - caches[i] = c;
> > }
> >
> > ff->ph->env.caches = caches;
> > ff->ph->env.caches_cnt = cnt;
> > return 0;
> > out_free_caches:
> > + for (i = 0; i < cnt; i++) {
> > + free(caches[i].type);
> > + free(caches[i].size);
> > + free(caches[i].map);
> > + }
> > free(caches);
> > return -1;
> > }
>
> Looks ok.
>
>
> > @@ -3585,18 +3593,16 @@ static int perf_header__adds_write(struct perf_header *header,
> > struct feat_copier *fc)
> > {
> > int nr_sections;
> > - struct feat_fd ff;
> > + struct feat_fd ff = {
> > + .fd = fd,
> > + .ph = header,
> > + };
>
> I'm fine with this change.
>
>
> > struct perf_file_section *feat_sec, *p;
> > int sec_size;
> > u64 sec_start;
> > int feat;
> > int err;
> >
> > - ff = (struct feat_fd){
> > - .fd = fd,
> > - .ph = header,
> > - };
> > -
> > nr_sections = bitmap_weight(header->adds_features, HEADER_FEAT_BITS);
> > if (!nr_sections)
> > return 0;
> > @@ -3623,6 +3629,7 @@ static int perf_header__adds_write(struct perf_header *header,
> > err = do_write(&ff, feat_sec, sec_size);
> > if (err < 0)
> > pr_debug("failed to write feature section\n");
> > + free(ff.buf);
>
> But it looks like false alarams. Since the feat_fd has fd
> and no buf, it won't allocate the buffer in do_write() and
> just use __do_write_fd(). So no need to free the buf.
This code looks like it has had a half-done optimization applied to it
- why have >1 buffer? why are we making the code's life hard? In
__do_write_buf there is a test that can be true even when a buf is
provided:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/header.c?h=perf-tools-next#n125
```
if (ff->size < new_size) {
addr = realloc(ff->buf, new_size);
if (!addr)
return -ENOMEM;
ff->buf = addr;
```
In the case clang-tidy (I'll put the analysis below) has determined
that new_size may be greater than size (I believe the intent in the
code is they both evaluate to 0) and this causes the memory leak being
fixed here. I'll add a TODO comment in v3 but things like 'buf' are
opaque and not intention revealing in the code, which makes any kind
of interpretation hard.
I think again this is a good signal that there is worthwhile
simplification/cleanup in this code.
Thanks,
Ian
```
tools/perf/util/header.c:3628:6: warning: Potential leak of memory
pointed to by 'ff.buf' [clang-analyzer-unix.Malloc]
err = do_write(&ff, feat_sec, sec_size);
^
tools/perf/util/header.c:3778:9: note: Calling 'perf_session__do_write_header'
return perf_session__do_write_header(session, evlist, fd, true, fc);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:3675:2: note: Loop condition is false.
Execution continues on line 3685
evlist__for_each_entry(session->evlist, evsel) {
^
tools/perf/util/evlist.h:276:2: note: expanded from macro
'evlist__for_each_entry'
__evlist__for_each_entry(&(evlist)->core.entries, evsel)
^
tools/perf/util/evlist.h:268:9: note: expanded from macro
'__evlist__for_each_entry'
list_for_each_entry(evsel, list, core.node)
^
tools/include/linux/list.h:458:2: note: expanded from macro
'list_for_each_entry'
for (pos = list_first_entry(head, typeof(*pos), member); \
^
tools/perf/util/header.c:3687:2: note: Loop condition is false.
Execution continues on line 3711
evlist__for_each_entry(evlist, evsel) {
^
tools/perf/util/evlist.h:276:2: note: expanded from macro
'evlist__for_each_entry'
__evlist__for_each_entry(&(evlist)->core.entries, evsel)
^
tools/perf/util/evlist.h:268:9: note: expanded from macro
'__evlist__for_each_entry'
list_for_each_entry(evsel, list, core.node)
^
tools/include/linux/list.h:458:2: note: expanded from macro
'list_for_each_entry'
for (pos = list_first_entry(head, typeof(*pos), member); \
^
tools/perf/util/header.c:3711:6: note: Assuming field 'data_offset' is
not equal to 0
if (!header->data_offset)
^~~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:3711:2: note: Taking false branch
if (!header->data_offset)
^
tools/perf/util/header.c:3715:6: note: 'at_exit' is true
if (at_exit) {
^~~~~~~
tools/perf/util/header.c:3715:2: note: Taking true branch
if (at_exit) {
^
tools/perf/util/header.c:3716:9: note: Calling 'perf_header__adds_write'
err = perf_header__adds_write(header, evlist, fd, fc);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:3606:6: note: Assuming 'nr_sections' is not equal
to 0
if (!nr_sections)
^~~~~~~~~~~~
tools/perf/util/header.c:3606:2: note: Taking false branch
if (!nr_sections)
^
tools/perf/util/header.c:3610:6: note: Assuming 'feat_sec' is not equal to
NULL
if (feat_sec == NULL)
^~~~~~~~~~~~~~~~
tools/perf/util/header.c:3610:2: note: Taking false branch
if (feat_sec == NULL)
^
tools/perf/util/header.c:3618:2: note: Assuming 'feat' is < HEADER_FEAT_BITS
for_each_set_bit(feat, header->adds_features, HEADER_FEAT_BITS) {
^
tools/include/linux/bitops.h:54:7: note: expanded from macro 'for_each_set_bit'
(bit) < (size); \
^~~~~~~~~~~~~~
tools/perf/util/header.c:3618:2: note: Loop condition is true.
Entering loop body
for_each_set_bit(feat, header->adds_features, HEADER_FEAT_BITS) {
^
tools/include/linux/bitops.h:53:2: note: expanded from macro 'for_each_set_bit'
for ((bit) = find_first_bit((addr), (size)); \
^
tools/perf/util/header.c:3619:3: note: Taking true branch
if (do_write_feat(&ff, feat, &p, evlist, fc))
^
tools/perf/util/header.c:3618:2: note: Assuming 'feat' is >= HEADER_FEAT_BITS
for_each_set_bit(feat, header->adds_features, HEADER_FEAT_BITS) {
tools/include/linux/bitops.h:54:7: note: expanded from macro 'for_each_set_bit'
(bit) < (size); \
^~~~~~~~~~~~~~
tools/perf/util/header.c:3618:2: note: Loop condition is false.
Execution continues on line 3623
for_each_set_bit(feat, header->adds_features, HEADER_FEAT_BITS) {
^
tools/include/linux/bitops.h:53:2: note: expanded from macro 'for_each_set_bit'
for ((bit) = find_first_bit((addr), (size)); \
^
tools/perf/util/header.c:3628:8: note: Calling 'do_write'
err = do_write(&ff, feat_sec, sec_size);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:141:6: note: Assuming field 'buf' is non-null
if (!ff->buf)
^~~~~~~~
tools/perf/util/header.c:141:2: note: Taking false branch
if (!ff->buf)
^
tools/perf/util/header.c:143:9: note: Calling '__do_write_buf'
return __do_write_buf(ff, buf, size);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:117:6: note: Assuming the condition is false
if (size + ff->offset > max_size)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:117:2: note: Taking false branch
if (size + ff->offset > max_size)
^
tools/perf/util/header.c:120:9: note: Assuming the condition is true
while (size > (new_size - ff->offset))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:120:2: note: Loop condition is true.
Entering loop body
while (size > (new_size - ff->offset))
^
tools/perf/util/header.c:120:9: note: Assuming the condition is false
while (size > (new_size - ff->offset))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:120:2: note: Loop condition is false.
Execution continues on line 122
while (size > (new_size - ff->offset))
^
tools/perf/util/header.c:122:13: note: Assuming '_min1' is >= '_min2'
new_size = min(max_size, new_size);
^
tools/include/linux/kernel.h:53:2: note: expanded from macro 'min'
_min1 < _min2 ? _min1 : _min2; })
^~~~~~~~~~~~~
tools/perf/util/header.c:122:13: note: '?' condition is false
new_size = min(max_size, new_size);
^
tools/include/linux/kernel.h:53:2: note: expanded from macro 'min'
_min1 < _min2 ? _min1 : _min2; })
^
tools/perf/util/header.c:124:6: note: Assuming 'new_size' is > field 'size'
if (ff->size < new_size) {
^~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:124:2: note: Taking true branch
if (ff->size < new_size) {
^
tools/perf/util/header.c:125:10: note: Memory is allocated
addr = realloc(ff->buf, new_size);
^~~~~~~~~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:126:7: note: Assuming 'addr' is non-null
if (!addr)
^~~~~
tools/perf/util/header.c:126:3: note: Taking false branch
if (!addr)
^
tools/perf/util/header.c:143:9: note: Returned allocated memory
return __do_write_buf(ff, buf, size);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:3628:8: note: Returned allocated memory
err = do_write(&ff, feat_sec, sec_size);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/perf/util/header.c:3628:6: note: Potential leak of memory
pointed to by 'ff.buf'
err = do_write(&ff, feat_sec, sec_size);
```
> Thanks,
> Namhyung
>
>
> > free(feat_sec);
> > return err;
> > }
> > @@ -3630,11 +3637,11 @@ static int perf_header__adds_write(struct perf_header *header,
> > int perf_header__write_pipe(int fd)
> > {
> > struct perf_pipe_file_header f_header;
> > - struct feat_fd ff;
> > + struct feat_fd ff = {
> > + .fd = fd,
> > + };
> > int err;
> >
> > - ff = (struct feat_fd){ .fd = fd };
> > -
> > f_header = (struct perf_pipe_file_header){
> > .magic = PERF_MAGIC,
> > .size = sizeof(f_header),
> > @@ -3645,7 +3652,7 @@ int perf_header__write_pipe(int fd)
> > pr_debug("failed to write perf pipe header\n");
> > return err;
> > }
> > -
> > + free(ff.buf);
> > return 0;
> > }
> >
> > @@ -3658,11 +3665,12 @@ static int perf_session__do_write_header(struct perf_session *session,
> > struct perf_file_attr f_attr;
> > struct perf_header *header = &session->header;
> > struct evsel *evsel;
> > - struct feat_fd ff;
> > + struct feat_fd ff = {
> > + .fd = fd,
> > + };
> > u64 attr_offset;
> > int err;
> >
> > - ff = (struct feat_fd){ .fd = fd};
> > lseek(fd, sizeof(f_header), SEEK_SET);
> >
> > evlist__for_each_entry(session->evlist, evsel) {
> > @@ -3670,6 +3678,7 @@ static int perf_session__do_write_header(struct perf_session *session,
> > err = do_write(&ff, evsel->core.id, evsel->core.ids * sizeof(u64));
> > if (err < 0) {
> > pr_debug("failed to write perf header\n");
> > + free(ff.buf);
> > return err;
> > }
> > }
> > @@ -3695,6 +3704,7 @@ static int perf_session__do_write_header(struct perf_session *session,
> > err = do_write(&ff, &f_attr, sizeof(f_attr));
> > if (err < 0) {
> > pr_debug("failed to write perf header attribute\n");
> > + free(ff.buf);
> > return err;
> > }
> > }
> > @@ -3705,8 +3715,10 @@ static int perf_session__do_write_header(struct perf_session *session,
> >
> > if (at_exit) {
> > err = perf_header__adds_write(header, evlist, fd, fc);
> > - if (err < 0)
> > + if (err < 0) {
> > + free(ff.buf);
> > return err;
> > + }
> > }
> >
> > f_header = (struct perf_file_header){
> > @@ -3728,6 +3740,7 @@ static int perf_session__do_write_header(struct perf_session *session,
> >
> > lseek(fd, 0, SEEK_SET);
> > err = do_write(&ff, &f_header, sizeof(f_header));
> > + free(ff.buf);
> > if (err < 0) {
> > pr_debug("failed to write perf header\n");
> > return err;
> > --
> > 2.42.0.609.gbb76f46606-goog
> >
© 2016 - 2026 Red Hat, Inc.