diff mbox series

locale: fix memory leaks in write_locales and write_charmaps

Message ID 20260326201923.3975252-1-linuxoid@gmail.com (mailing list archive)
State Under Review
Delegated to: Arjun Shankar
Headers
Series locale: fix memory leaks in write_locales and write_charmaps |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed

Commit Message

Ruslan Valiyev March 26, 2026, 8:19 p.m. UTC 
  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 macro that checks with
   GET (tfind) before inserting, freeing the duplicate if it already
   exists.

2. String literals "POSIX" and "C" passed to PUT cannot be freed
   by tdestroy. Wrap them in xstrdup 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 individual entries).

5. Free alias_path allocated by argz_create_sep in write_locales.

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

Resolves: BZ #33972
Signed-off-by: Ruslan Valiyev <linuxoid@gmail.com>
---
 locale/programs/locale.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)
diff mbox series

Patch

diff --git a/locale/programs/locale.c b/locale/programs/locale.c
index 15f109f3..396821a1 100644
--- a/locale/programs/locale.c
+++ b/locale/programs/locale.c
@@ -429,10 +429,20 @@  write_locales (void)
 #define GET(name) tfind (name, &all_data, \
 			   (int (*) (const void *, const void *)) strcoll)
 
+  /* Insert NAME into the tree, freeing it if a duplicate exists.  */
+#define PUT_UNIQUE(name) \
+  do {\
+    char *put_name_ = (name);\
+    if (GET (put_name_) != NULL)\
+      free (put_name_);\
+    else\
+      PUT (put_name_);\
+  } while (0)
+
   /* `POSIX' locale is always available (POSIX.2 4.34.3).  */
-  PUT ("POSIX");
+  PUT (xstrdup ("POSIX"));
   /* And so is the "C" locale.  */
-  PUT ("C");
+  PUT (xstrdup ("C"));
 
   memset (linebuf, '-', sizeof (linebuf) - 1);
   linebuf[sizeof (linebuf) - 1] = '\0';
@@ -510,8 +520,9 @@  write_locales (void)
 
 	  /* If the verbose format is not selected we simply
 	     collect the names.  */
-	  PUT (xstrdup (dirents[cnt]->d_name));
+	  PUT_UNIQUE (xstrdup (dirents[cnt]->d_name));
 	}
+      free (dirents[cnt]);
     }
   if (ndirents > 0)
     free (dirents);
@@ -591,7 +602,7 @@  write_locales (void)
 
 		  /* Add the alias.  */
 		  if (! verbose && GET (value) != NULL)
-		    PUT (xstrdup (alias));
+		    PUT_UNIQUE (xstrdup (alias));
 		}
 	    }
 
@@ -610,10 +621,14 @@  write_locales (void)
       fclose (fp);
     }
 
+  free (alias_path);
+
   if (! verbose)
     {
       twalk (all_data, print_names);
     }
+
+  tdestroy (all_data, free);
 }
 
 
@@ -669,7 +684,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 (xstrdup (addr + namehashtab[cnt].name_offset));
 	    ++ret;
 	  }
     }
@@ -694,7 +709,7 @@  write_archive_locales (void **all_datap, char *linebuf)
 	{
 	  struct locrecent *locrec;
 
-	  PUT (xstrdup (names[cnt].name));
+	  PUT_UNIQUE (xstrdup (names[cnt].name));
 
 	  if (cnt)
 	    putchar_unlocked ('\n');
@@ -744,19 +759,19 @@  write_charmaps (void)
       char **aliases;
       char **p;
 
-      PUT (xstrdup (dirent));
+      PUT_UNIQUE (xstrdup (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 (xstrdup (*p));
 #else
       /* Add the code_set_name only.  Most aliases are obsolete.  */
       p = aliases;
       if (*p)
-	PUT (xstrdup (*p));
+	PUT_UNIQUE (xstrdup (*p));
 #endif
 
       charmap_free_aliases (aliases);
@@ -765,6 +780,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