Re: Fix some memory leaks + double free

Sylvestre Ledru <[email protected]>
Newsgroups gmane.comp.lib.cairo
Message-ID <[email protected]>
On 08/05/2014 21:45, Uli Schlachter wrote:
> On 07.05.2014 19:04, Sylvestre Ledru wrote:
>> From 3cb1dd13903014814dced740d785b101c17fd622 Mon Sep 17 00:00:00 2001
>> From: Sylvestre Ledru <[email protected]>
>> Date: Wed, 7 May 2014 17:15:22 +0200
>> Subject: [PATCH 1/2] Fix some memory leaks found by scan-build, the LLVM/Clang
>>  static analyzer
> [...]
>> diff --git a/src/cairo-pdf-surface.c b/src/cairo-pdf-surface.c
>> index c200c28..8e92df2 100644
>> --- a/src/cairo-pdf-surface.c
>> +++ b/src/cairo-pdf-surface.c
>> @@ -2645,8 +2645,10 @@ _cairo_pdf_surface_lookup_jbig2_global (cairo_pdf_surface_t       *surface,
>>      memcpy (global.id, global_id, global_id_length);
>>      global.id_length = global_id_length;
>>      global.res = _cairo_pdf_surface_new_object (surface);
>> -    if (global.res.id == 0)
>> +    if (global.res.id == 0) {
> 
> Could you make this "if (global.id == NULL || global.res.id == 0) {"?
> 
> Hm, no, would also have to free global.res in this case... Uhm...
> 
> Add a check for malloc() failure after the line that sets global.id, please.
Sure. Done. Thanks for the review.

Here are the two patches updated.

Cheers,
Sylvestre

-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo
0002-Fix-some-memory-leaks-found-by-scan-build-the-LLVM-C.patch (text/x-patch, 2.2 KB)
From 53e93b46b90182b2d24d55310a9bbde01098e8df Mon Sep 17 00:00:00 2001
From: Sylvestre Ledru <[email protected]>
Date: Wed, 7 May 2014 17:15:22 +0200
Subject: [PATCH 2/2] Fix some memory leaks found by scan-build, the LLVM/Clang
 static analyzer

Reviewed-by: Uli Schlachter <[email protected]>

---
 src/cairo-cff-subset.c  | 4 +++-
 src/cairo-pdf-surface.c | 4 ++++
 test/pdf-mime-data.c    | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/cairo-cff-subset.c b/src/cairo-cff-subset.c
index 1ae032c..1bb9461 100644
--- a/src/cairo-cff-subset.c
+++ b/src/cairo-cff-subset.c
@@ -1851,8 +1851,10 @@ cairo_cff_font_subset_fontdict (cairo_cff_font_t  *font)
     for (i = 0; i < font->scaled_font_subset->num_glyphs; i++) {
 	cid = font->scaled_font_subset->glyphs[i];
 	status = cairo_cff_font_get_gid_for_cid (font, cid, &gid);
-	if (unlikely (status))
+	if (unlikely (status)) {
+	    free (reverse_map);
 	    return status;
+	}
 
         fd = font->fdselect[gid];
         if (reverse_map[fd] < 0) {
diff --git a/src/cairo-pdf-surface.c b/src/cairo-pdf-surface.c
index c200c28..17fe1df 100644
--- a/src/cairo-pdf-surface.c
+++ b/src/cairo-pdf-surface.c
@@ -2642,6 +2642,10 @@ _cairo_pdf_surface_lookup_jbig2_global (cairo_pdf_surface_t       *surface,
     }
 
     global.id = malloc(global_id_length);
+    if (unlikely (global.id == NULL)) {
+	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+    }
+
     memcpy (global.id, global_id, global_id_length);
     global.id_length = global_id_length;
     global.res = _cairo_pdf_surface_new_object (surface);
diff --git a/test/pdf-mime-data.c b/test/pdf-mime-data.c
index c575c4a..e2c529e 100644
--- a/test/pdf-mime-data.c
+++ b/test/pdf-mime-data.c
@@ -105,6 +105,7 @@ preamble (cairo_test_context_t *ctx)
     test_status = read_file (ctx, IMAGE_FILE ".jpg", &data, &len);
     if (test_status) {
 	cairo_test_log (ctx, "Could not read input jpeg file %s\n", IMAGE_FILE ".jpg");
+	free(data);
 	return test_status;
     }
 
@@ -149,6 +150,7 @@ preamble (cairo_test_context_t *ctx)
     test_status = read_file (ctx, IMAGE_FILE ".jpg", &data, &len);
     if (test_status) {
 	cairo_test_log (ctx, "Could not read input jpeg file %s\n", IMAGE_FILE ".jpg");
+	free(data);
 	return test_status;
     }
 
-- 
2.0.0.rc0
0001-Remove-some-potential-double-free.patch (text/x-patch, 846 B)
From 8cbc0d8373fc6e593ea85e0d21e06c8004f8cfa6 Mon Sep 17 00:00:00 2001
From: Sylvestre Ledru <[email protected]>
Date: Wed, 7 May 2014 17:16:09 +0200
Subject: [PATCH 1/2] Remove some potential double free

Reviewed-by: Uli Schlachter <[email protected]>

---
 src/cairo-truetype-subset.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/cairo-truetype-subset.c b/src/cairo-truetype-subset.c
index 3d55fef..645a89a 100644
--- a/src/cairo-truetype-subset.c
+++ b/src/cairo-truetype-subset.c
@@ -1564,12 +1564,12 @@ _cairo_truetype_read_font_name (cairo_scaled_font_t  	 *scaled_font,
 	    goto fail;
     }
 
-    free (name);
-
     status = _cairo_escape_ps_name (&ps_name);
     if (unlikely(status))
 	goto fail;
 
+    free (name);
+
     *ps_name_out = ps_name;
     *font_name_out = family_name;
 
-- 
2.0.0.rc0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.