Messages in this thread | | | Date | Mon, 24 Apr 2023 16:06:46 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH] recordmcount: Fix memory leaks in the uwrite function |
| |
On Wed, 12 Apr 2023 17:30:48 +0800 Hao Zeng <zenghao@kylinos.cn> wrote:
> Common realloc mistake: 'file_append' nulled but not freed upon failure > > Signed-off-by: Hao Zeng <zenghao@kylinos.cn> > --- > scripts/recordmcount.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c > index e30216525325..2b7173a86d4c 100644 > --- a/scripts/recordmcount.c > +++ b/scripts/recordmcount.c > @@ -110,22 +110,23 @@ static ssize_t uwrite(void const *const buf, size_t const count) > { > size_t cnt = count; > off_t idx = 0; > - > + void *p = NULL; > file_updated = 1; > > if (file_ptr + count >= file_end) { > off_t aoffset = (file_ptr + count) - file_end; > > if (aoffset > file_append_size) { > - file_append = realloc(file_append, aoffset); > + p = realloc(file_append, aoffset); > + if (!p) { > + perror("write"); > + file_append_cleanup(); > + mmap_cleanup(); > + return -1; > + } > + file_append = p; > file_append_size = aoffset; > }
This changes the logic of the function. If file_append is NULL when entering, and does not get into the allocate path we still want this to error.
Just do:
p = realloc(file_append, aoffset); if (!p) { free(file_append); file_append = NULL; }
And that keeps the same logic but removes the memory leak.
-- Steve
> - if (!file_append) { > - perror("write"); > - file_append_cleanup(); > - mmap_cleanup(); > - return -1; > - } > if (file_ptr < file_end) { > cnt = file_end - file_ptr; > } else {
| |