[PATCH v2] locale: fix memory leaks in write_locales and write_charmaps

Ruslan Valiyev <[email protected]>
Newsgroups gmane.comp.lib.glibc.alpha
Message-ID <[email protected]>
Fix multiple memory leaks in the locale program:

1. PUT (xstrdup (...)) leaks when tsearch finds a duplicate entry,
   since tsearch returns the existing node and the newly allocated
   string is orphaned.  Introduce PUT_UNIQUE, which looks the name up
   with GET first and only allocates when it is actually inserted.

2. String literals "POSIX" and "C" passed to PUT cannot be freed by
   tdestroy.  They now go through PUT_UNIQUE, which duplicates them,
   so tdestroy (all_data, free) is safe.

3. Add tdestroy (all_data, free) at the end of write_locales and
   write_charmaps to free the search trees.

4. Free dirents[cnt] entries in the scandir loop (only the dirents
   array pointer was freed, not the individual entries).

5. Free alias_path allocated by argz_create_sep in write_locales.

Before this change "locale -a" leaked 74 bytes in 3 blocks directly
and 835 bytes in 49 blocks indirectly, and "locale -m" leaked 2190
bytes in 227 blocks.  Both are valgrind-clean afterwards.

These leaks were reported by Arjun Shankar via GCC -fanalyzer
(OpenScanHub/Fedora).

Resolves: BZ #33972
Signed-off-by: Ruslan Valiyev <[email protected]>
---
 locale/programs/locale.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/locale/programs/locale.c b/locale/programs/locale.c
index 15f109f3..2e47b14f 100644
--- a/locale/programs/locale.c
+++ b/locale/programs/locale.c
@@ -429,10 +429,21 @@ write_locales (void)
 #define GET(name) tfind (name, &all_data, \
 			   (int (*) (const void *, const void *)) strcoll)
 
+/* Insert a copy of NAME into the tree, unless an equal string is already
+   present.  Only allocates when the name is actually new.  */
+#define PUT_UNIQUE(name) \
+  do \
+    { \
+      const char *put_name_ = (name); \
+      if (GET (put_name_) == NULL) \
+	PUT (xstrdup (put_name_)); \
+    } \
+  while (0)
+
   /* `POSIX' locale is always available (POSIX.2 4.34.3).  */
-  PUT ("POSIX");
+  PUT_UNIQUE ("POSIX");
   /* And so is the "C" locale.  */
-  PUT ("C");
+  PUT_UNIQUE ("C");
 
   memset (linebuf, '-', sizeof (linebuf) - 1);
   linebuf[sizeof (linebuf) - 1] = '\0';
@@ -510,8 +521,9 @@ write_locales (void)
 
 	  /* If the verbose format is not selected we simply
 	     collect the names.  */
-	  PUT (xstrdup (dirents[cnt]->d_name));
+	  PUT_UNIQUE (dirents[cnt]->d_name);
 	}
+      free (dirents[cnt]);
     }
   if (ndirents > 0)
     free (dirents);
@@ -591,7 +603,7 @@ write_locales (void)
 
 		  /* Add the alias.  */
 		  if (! verbose && GET (value) != NULL)
-		    PUT (xstrdup (alias));
+		    PUT_UNIQUE (alias);
 		}
 	    }
 
@@ -610,10 +622,14 @@ write_locales (void)
       fclose (fp);
     }
 
+  free (alias_path);
+
   if (! verbose)
     {
       twalk (all_data, print_names);
     }
+
+  tdestroy (all_data, free);
 }
 
 
@@ -669,7 +685,7 @@ write_archive_locales (void **all_datap, char *linebuf)
       for (cnt = 0; cnt < head->namehash_size; ++cnt)
 	if (namehashtab[cnt].locrec_offset != 0)
 	  {
-	    PUT (xstrdup (addr + namehashtab[cnt].name_offset));
+	    PUT_UNIQUE (addr + namehashtab[cnt].name_offset);
 	    ++ret;
 	  }
     }
@@ -694,7 +710,7 @@ write_archive_locales (void **all_datap, char *linebuf)
 	{
 	  struct locrecent *locrec;
 
-	  PUT (xstrdup (names[cnt].name));
+	  PUT_UNIQUE (names[cnt].name);
 
 	  if (cnt)
 	    putchar_unlocked ('\n');
@@ -744,19 +760,19 @@ write_charmaps (void)
       char **aliases;
       char **p;
 
-      PUT (xstrdup (dirent));
+      PUT_UNIQUE (dirent);
 
       aliases = charmap_aliases (CHARMAP_PATH, dirent);
 
 #if 0
       /* Add the code_set_name and the aliases.  */
       for (p = aliases; *p; p++)
-	PUT (xstrdup (*p));
+	PUT_UNIQUE (*p);
 #else
       /* Add the code_set_name only.  Most aliases are obsolete.  */
       p = aliases;
       if (*p)
-	PUT (xstrdup (*p));
+	PUT_UNIQUE (*p);
 #endif
 
       charmap_free_aliases (aliases);
@@ -765,6 +781,8 @@ write_charmaps (void)
   charmap_closedir (dir);
 
   twalk (all_data, print_names);
+
+  tdestroy (all_data, free);
 }
 
 /* Print a properly quoted assignment of NAME with VAL, using double

base-commit: a1e333203ac2d9f90bf9fbc09b4f43f1d16efef5
-- 
2.43.0
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.