Hi Mark, On Tue, May 26, 2026 at 1:09 PM Mark Wielaard <[email protected]> wrote: > > We track the shstrtab and symtab section names in case they need to be > renamed. These names are sometimes assigned a static string (from the > shstrtab string pool), sometimes created as a copy of an existing > string name and sometimes created by constructing a new string. This > means they might leak because they are never freed. > > Fix this leak by defining the variables early (as NULL) before looping > over all sections, making sure to always assign a allocated (copy) of > the (new) name and explicitly free them at cleanup (run both on > success and failure). > > This makes it easier to reason about allocation status and string > "ownership" making it easy to see there are no memory issues even in > the rare case of a .[z]debug section being used as a symtab or > shstrtab table (which is why there is no testcase, because it is > questionable whether such a file is actually valid). > > * src/elfcompress.c (process_file): Define shstrtab_name, > shstrtab_newname, symtab_name and symtab_newname > early. xstrdup shstrtab_name and symtab_name if they were > assigned to a static string. Free all four strings at > cleanup. > > Signed-off-by: Mark Wielaard <[email protected]>
LGTM. Aaron > --- > src/elfcompress.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/src/elfcompress.c b/src/elfcompress.c > index 789bcb383786..6f21e7c330b8 100644 > --- a/src/elfcompress.c > +++ b/src/elfcompress.c > @@ -357,6 +357,12 @@ process_file (const char *fname) > /* Which sections match and need to be (un)compressed. */ > unsigned int *sections = NULL; > > + /* Specific section names when renaming shstrtab or symtab. */ > + char *shstrtab_name = NULL; > + char *shstrtab_newname = NULL; > + char *symtab_name = NULL; > + char *symtab_newname = NULL; > + > /* How many sections are we talking about? */ > size_t shnum = 0; > int res = 1; > @@ -703,12 +709,8 @@ process_file (const char *fname) > difference later when we do compress. */ > enum ch_type shstrtab_compressed = UNSET; > size_t shstrtab_size = 0; > - char *shstrtab_name = NULL; > - char *shstrtab_newname = NULL; > enum ch_type symtab_compressed = UNSET; > size_t symtab_size = 0; > - char *symtab_name = NULL; > - char *symtab_newname = NULL; > > /* Collection pass. Copy over the sections, (de)compresses matching > sections, collect names of sections and symbol table if > @@ -1141,6 +1143,7 @@ process_file (const char *fname) > shdrstrndx); > goto cleanup; > } > + shstrtab_name = xstrdup (shstrtab_name); > > shstrtab_size = shdr->sh_size; > if ((shdr->sh_flags & SHF_COMPRESSED) != 0) > @@ -1282,6 +1285,7 @@ process_file (const char *fname) > symtabndx); > goto cleanup; > } > + symtab_name = xstrdup (symtab_name); > > symtab_size = shdr->sh_size; > if ((shdr->sh_flags & SHF_COMPRESSED) != 0) > @@ -1396,6 +1400,10 @@ cleanup: > } > > free (sections); > + free (shstrtab_name); > + free (shstrtab_newname); > + free (symtab_name); > + free (symtab_newname); > return res; > } > > -- > 2.54.0 >
