Bug 90867 - Memory Leak during error case in fccharset
Summary: Memory Leak during error case in fccharset
Status: RESOLVED FIXED
Alias: None
Product: fontconfig
Classification: Unclassified
Component: library (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: Akira TAGOH
QA Contact: Behdad Esfahbod
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-05 08:33 UTC by Pranay Kumar Samanta
Modified: 2015-06-24 06:52 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
This is a proposed patch for the memroy leak. (592 bytes, text/plain)
2015-06-05 08:33 UTC, Pranay Kumar Samanta
Details
This is a proposed patch for the memroy leak. (592 bytes, patch)
2015-06-05 08:34 UTC, Pranay Kumar Samanta
Details | Splinter Review

Description Pranay Kumar Samanta 2015-06-05 08:33:24 UTC
Created attachment 116313 [details]
This is a proposed patch for the memroy leak.

There is a possible memory leak in fccharset.c file in FcCharSetPutLeaf function.

There is proposed patch attached.
Comment 1 Pranay Kumar Samanta 2015-06-05 08:34:52 UTC
Created attachment 116314 [details] [review]
This is a proposed patch for the memroy leak.
Comment 2 Akira TAGOH 2015-06-05 09:04:10 UTC
Thank you for catching this up. but that patch looks not correct to me. particularly when reallocating leaves was success but not for numbers. the pointer may be updated so next access to leaves may causes segfault then.

Here is another proposal to fix it:
diff --git a/src/fccharset.c b/src/fccharset.c
index 6e0093f..3f17892 100644
--- a/src/fccharset.c
+++ b/src/fccharset.c
@@ -164,6 +164,14 @@ FcCharSetPutLeaf (FcCharSet	*fcs,
         unsigned int alloced = 8;
 	leaves = malloc (alloced * sizeof (*leaves));
 	numbers = malloc (alloced * sizeof (*numbers));
+	if (!leaves || !numbers)
+	{
+	    if (leaves)
+		free (leaves);
+	    if (numbers)
+		free (numbers);
+	    return FcFalse;
+	}
       }
       else
       {
@@ -172,8 +180,19 @@ FcCharSetPutLeaf (FcCharSet	*fcs,
 
 	alloced *= 2;
 	new_leaves = realloc (leaves, alloced * sizeof (*leaves));
+	if (!new_leaves)
+	    return FcFalse;
 	numbers = realloc (numbers, alloced * sizeof (*numbers));
-
+	if (!numbers)
+	{
+	    /* Revert the reallocation of leaves */
+	    leaves = realloc (new_leaves, (alloced / 2) * sizeof (*new_leaves));
+	    /* unlikely to fail though */
+	    if (!leaves)
+		return FcFalse;
+	    fcs->leaves_offset = FcPtrToOffset (fcs, leaves);
+	    return FcFalse;
+	}
 	distance = (intptr_t) new_leaves - (intptr_t) leaves;
 	if (new_leaves && distance)
 	{
@@ -184,9 +203,6 @@ FcCharSetPutLeaf (FcCharSet	*fcs,
 	leaves = new_leaves;
       }
 
-      if (!leaves || !numbers)
-	  return FcFalse;
-
       fcs->leaves_offset = FcPtrToOffset (fcs, leaves);
       fcs->numbers_offset = FcPtrToOffset (fcs, numbers);
     }


Please test if you have any testcase for that.
Comment 3 Akira TAGOH 2015-06-24 06:52:51 UTC
fixed in git.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.