diff mbox

Fix BZ 19012 -- memory leak on error path in iconv_open

Message ID CALoOobODv_G89OCTm6Lot5MA19i3-M8TD9iLaE=ivsO8dCxyJQ@mail.gmail.com
State New
Headers show

Commit Message

Paul Pluzhnikov Oct. 3, 2015, 6:41 p.m. UTC 
On Mon, Sep 28, 2015 at 10:31 AM, Mike Frysinger <vapier@gentoo.org> wrote:

> what about the other failure case (init_fct returns an error) ?

Good point.

> the strduping seems a bit convoluted and makes error handling error prone.
> what about something a bit more straightforward:

May as well also add a check for strdup failure. Updated patch attached.

Thanks,

2015-10-03  Paul Pluzhnikov  <ppluzhnikov@google.com>

        [BZ #19012]
        * iconv/gconv_db.c (gen_steps): Check for additional errors.
Clean up on failure.

Comments

Mike Frysinger Oct. 3, 2015, 8:08 p.m. UTC | #1 
lgtm
-mike
diff mbox

Patch

diff --git a/iconv/gconv_db.c b/iconv/gconv_db.c
index 4e6ec65..74b4ff5 100644
--- a/iconv/gconv_db.c
+++ b/iconv/gconv_db.c
@@ -243,6 +243,8 @@  gen_steps (struct derivation_step *best, const char *toset,
   struct __gconv_step *result;
   struct derivation_step *current;
   int status = __GCONV_NOMEM;
+  char *from_name = NULL;
+  char *to_name = NULL;
 
   /* First determine number of steps.  */
   for (current = best; current->last != NULL; current = current->last)
@@ -259,12 +261,30 @@  gen_steps (struct derivation_step *best, const char *toset,
       current = best;
       while (step_cnt-- > 0)
 	{
-	  result[step_cnt].__from_name = (step_cnt == 0
-					  ? __strdup (fromset)
-					  : (char *)current->last->result_set);
-	  result[step_cnt].__to_name = (step_cnt + 1 == *nsteps
-					? __strdup (current->result_set)
-					: result[step_cnt + 1].__from_name);
+	  if (step_cnt == 0)
+	    {
+	      result[step_cnt].__from_name = from_name = __strdup (fromset);
+	      if (from_name == NULL)
+		{
+		  failed = 1;
+		  break;
+		}
+	    }
+	  else
+	    result[step_cnt].__from_name = (char *)current->last->result_set;
+
+	  if (step_cnt + 1 == *nsteps)
+	    {
+	      result[step_cnt].__to_name = to_name
+		= __strdup (current->result_set);
+	      if (to_name == NULL)
+		{
+		  failed = 1;
+		  break;
+		}
+	    }
+	  else
+	    result[step_cnt].__to_name = result[step_cnt + 1].__from_name;
 
 	  result[step_cnt].__counter = 1;
 	  result[step_cnt].__data = NULL;
@@ -332,6 +352,8 @@  gen_steps (struct derivation_step *best, const char *toset,
 	  while (++step_cnt < *nsteps)
 	    __gconv_release_step (&result[step_cnt]);
 	  free (result);
+	  free (from_name);
+	  free (to_name);
 	  *nsteps = 0;
 	  *handle = NULL;
 	  if (status == __GCONV_OK)
@@ -828,8 +850,9 @@  free_modules_db (struct gconv_module *node)
 /* Free all resources if necessary.  */
 libc_freeres_fn (free_mem)
 {
-  /* First free locale memory.  This needs to be done before freeing derivations,
-     as ctype cleanup functions dereference steps arrays which we free below.  */
+  /* First free locale memory.  This needs to be done before freeing
+     derivations, as ctype cleanup functions dereference steps arrays which we
+     free below.  */
   _nl_locale_subfreeres ();
 
   /* finddomain.c has similar problem.  */