[v3,11/33] libctf: fix memory leak on ctf_compress_write error path

Message ID 20190906225520.169680-12-nick.alcock@oracle.com
State Superseded
Headers show
Series
  • CTF linking support
Related show

Commit Message

Nick Alcock Sept. 6, 2019, 10:54 p.m.
We were failing to free the compressed-data buffer if compression
failed.

libctf/
	* ctf-create.c (ctf_compress_write): Fix leak.
---
 libctf/ctf-create.c | 1 -
 1 file changed, 1 deletion(-)

-- 
2.23.0.239.g28aa4420fd

Comments

Hans-Peter Nilsson Sept. 6, 2019, 11:06 p.m. | #1
On Fri, 6 Sep 2019, Nick Alcock wrote:

> We were failing to free the compressed-data buffer if compression

> failed.

>

> libctf/

> 	* ctf-create.c (ctf_compress_write): Fix leak.


Aren't you removing a call to free in this patch, thus actually
*introducing* a leak, considering that ctf_free is just a
wrapper for free?

> ---

>  libctf/ctf-create.c | 1 -

>  1 file changed, 1 deletion(-)

>

> diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c

> index 6189042fdb0..76304f724c7 100644

> --- a/libctf/ctf-create.c

> +++ b/libctf/ctf-create.c

> @@ -1997,7 +1997,6 @@ ctf_compress_write (ctf_file_t *fp, int fd)

>      {

>        ctf_dprintf ("zlib deflate err: %s\n", zError (rc));

>        err = ctf_set_errno (fp, ECTF_COMPRESS);

> -      ctf_free (buf);

>        goto ret;

>      }

>

> --

> 2.23.0.239.g28aa4420fd

>


brgds, H-P
Nick Alcock Sept. 6, 2019, 11:18 p.m. | #2
On 7 Sep 2019, Hans-Peter Nilsson verbalised:

> On Fri, 6 Sep 2019, Nick Alcock wrote:

>

>> We were failing to free the compressed-data buffer if compression

>> failed.

>>

>> libctf/

>> 	* ctf-create.c (ctf_compress_write): Fix leak.

>

> Aren't you removing a call to free in this patch, thus actually

> *introducing* a leak, considering that ctf_free is just a

> wrapper for free?


That had me scratching my head for a moment -- but it's the commit
message (and changelog text) that's wrong. It should be something like

    write: avoid double-free on compression error path

>> ---

>>  libctf/ctf-create.c | 1 -

>>  1 file changed, 1 deletion(-)

>>

>> diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c

>> index 6189042fdb0..76304f724c7 100644

>> --- a/libctf/ctf-create.c

>> +++ b/libctf/ctf-create.c

>> @@ -1997,7 +1997,6 @@ ctf_compress_write (ctf_file_t *fp, int fd)

>>      {

>>        ctf_dprintf ("zlib deflate err: %s\n", zError (rc));

>>        err = ctf_set_errno (fp, ECTF_COMPRESS);

>> -      ctf_free (buf);

>>        goto ret;

>>      }


This is correct: we free the buffer here:

> ret:

>   ctf_free (buf);

>   return err;

> }


Fixed locally. Thank you!

Patch

diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c
index 6189042fdb0..76304f724c7 100644
--- a/libctf/ctf-create.c
+++ b/libctf/ctf-create.c
@@ -1997,7 +1997,6 @@  ctf_compress_write (ctf_file_t *fp, int fd)
     {
       ctf_dprintf ("zlib deflate err: %s\n", zError (rc));
       err = ctf_set_errno (fp, ECTF_COMPRESS);
-      ctf_free (buf);
       goto ret;
     }