COFF swap_aux_in
authorAlan Modra <amodra@gmail.com>
Mon, 28 Aug 2023 11:37:52 +0000 (21:07 +0930)
committerAlan Modra <amodra@gmail.com>
Mon, 28 Aug 2023 13:40:57 +0000 (23:10 +0930)
A low level function like coff_swap_aux_in really has no business
concatenating multiple auxents for the old PE multi-aux scheme of
handling long file names.  In doing so, it assumes multiple internal
auxent buffers are available, which they are not in most calls to
bfd_coff_swap_aux_in, both inside BFD and outside, eg. GDB.  Buffer
overflow fun.  Concatenating multiple auxents belongs at a higher
level.

This required some changes to coff_get_normalized_symtab, which now
uses the external auxents to access the concatenated file name.
(Internal auxents are larger than the x_fname array, so the pieces of
the file name are not adjacent as they are in the external auxents.)

* coffswap.h (coff_swap_aux_in): Do not write more than one
internal auxent.
* coffcode.h (coff_bigobj_swap_aux_in): Likewise.
* coffgen.c (coff_get_normalized_symtab): Normalize strings
after swapping in each symbol so that external auxents are
available.  Use external auxents for multi-aux long file
names.  Formatting.  Wrap long lines.  Remove excess parens
and unnecessary casts.  Don't zalloc when only the string
terminator needs zeroing, and memcpy rather than strncpy.
Delete unnecessary sanity check with unsigned _n_offset.
Return with failure if debug section can't be read, to avoid
trying to read it multiple times.  Correct sanity check
against debug section size.

bfd/coffcode.h
bfd/coffgen.c
bfd/coffswap.h

index 908dc93c64acfbac13d8da286833cb6c0355608c..0487555dda1911b0934df5289f0b47f7e0762adf 100644 (file)
@@ -5813,8 +5813,8 @@ coff_bigobj_swap_aux_in (bfd *abfd,
                         void * ext1,
                         int type,
                         int in_class,
-                        int indx,
-                        int numaux,
+                        int indx ATTRIBUTE_UNUSED,
+                        int numaux ATTRIBUTE_UNUSED,
                         void * in1)
 {
   AUXENT_BIGOBJ *ext = (AUXENT_BIGOBJ *) ext1;
@@ -5826,14 +5826,7 @@ coff_bigobj_swap_aux_in (bfd *abfd,
   switch (in_class)
     {
     case C_FILE:
-      if (numaux > 1)
-       {
-         if (indx == 0)
-           memcpy (in->x_file.x_n.x_fname, ext->File.Name,
-                   numaux * sizeof (AUXENT_BIGOBJ));
-       }
-      else
-       memcpy (in->x_file.x_n.x_fname, ext->File.Name, sizeof (ext->File.Name));
+      memcpy (in->x_file.x_n.x_fname, ext->File.Name, sizeof (ext->File.Name));
       break;
 
     case C_STAT:
index 91667267cbc88005107c6605ca82c9abdf91173c..c1aacc707b541001852a25f4640042c99b28bd08 100644 (file)
@@ -1843,8 +1843,6 @@ coff_get_normalized_symtab (bfd *abfd)
 {
   combined_entry_type *internal;
   combined_entry_type *internal_ptr;
-  combined_entry_type *symbol_ptr;
-  combined_entry_type *internal_end;
   size_t symesz;
   char *raw_src;
   char *raw_end;
@@ -1867,7 +1865,6 @@ coff_get_normalized_symtab (bfd *abfd)
   internal = (combined_entry_type *) bfd_zalloc (abfd, size);
   if (internal == NULL && size != 0)
     return NULL;
-  internal_end = internal + obj_raw_syment_count (abfd);
 
   raw_src = (char *) obj_coff_external_syms (abfd);
 
@@ -1887,48 +1884,32 @@ coff_get_normalized_symtab (bfd *abfd)
 
       bfd_coff_swap_sym_in (abfd, (void *) raw_src,
                            (void *) & internal_ptr->u.syment);
-      symbol_ptr = internal_ptr;
       internal_ptr->is_sym = true;
+      combined_entry_type *sym = internal_ptr;
 
       /* PR 17512: Prevent buffer overrun.  */
-      if (symbol_ptr->u.syment.n_numaux > ((raw_end - 1) - raw_src) / symesz)
+      if (sym->u.syment.n_numaux > ((raw_end - 1) - raw_src) / symesz)
        return NULL;
 
-      for (i = 0;
-          i < symbol_ptr->u.syment.n_numaux;
-          i++)
+      for (i = 0; i < sym->u.syment.n_numaux; i++)
        {
          internal_ptr++;
          raw_src += symesz;
 
          bfd_coff_swap_aux_in (abfd, (void *) raw_src,
-                               symbol_ptr->u.syment.n_type,
-                               symbol_ptr->u.syment.n_sclass,
-                               (int) i, symbol_ptr->u.syment.n_numaux,
+                               sym->u.syment.n_type,
+                               sym->u.syment.n_sclass,
+                               (int) i, sym->u.syment.n_numaux,
                                &(internal_ptr->u.auxent));
 
          internal_ptr->is_sym = false;
-         coff_pointerize_aux (abfd, internal, symbol_ptr, i, internal_ptr);
+         coff_pointerize_aux (abfd, internal, sym, i, internal_ptr);
        }
-    }
 
-  /* Free the raw symbols.  */
-  if (obj_coff_external_syms (abfd) != NULL
-      && ! obj_coff_keep_syms (abfd))
-    {
-      free (obj_coff_external_syms (abfd));
-      obj_coff_external_syms (abfd) = NULL;
-    }
-
-  for (internal_ptr = internal; internal_ptr < internal_end;
-       internal_ptr++)
-    {
-      BFD_ASSERT (internal_ptr->is_sym);
-
-      if (internal_ptr->u.syment.n_sclass == C_FILE
-         && internal_ptr->u.syment.n_numaux > 0)
+      if (sym->u.syment.n_sclass == C_FILE
+         && sym->u.syment.n_numaux > 0)
        {
-         combined_entry_type * aux = internal_ptr + 1;
+         combined_entry_type * aux = sym + 1;
 
          /* Make a file symbol point to the name in the auxent, since
             the text ".file" is redundant.  */
@@ -1944,12 +1925,12 @@ coff_get_normalized_symtab (bfd *abfd)
                    return NULL;
                }
 
-             if ((bfd_size_type)(aux->u.auxent.x_file.x_n.x_n.x_offset)
+             if ((bfd_size_type) aux->u.auxent.x_file.x_n.x_n.x_offset
                  >= obj_coff_strings_len (abfd))
-               internal_ptr->u.syment._n._n_n._n_offset =
+               sym->u.syment._n._n_n._n_offset =
                  (uintptr_t) _("<corrupt>");
              else
-               internal_ptr->u.syment._n._n_n._n_offset =
+               sym->u.syment._n._n_n._n_offset =
                  (uintptr_t) (string_table
                               + aux->u.auxent.x_file.x_n.x_n.x_offset);
            }
@@ -1958,30 +1939,35 @@ coff_get_normalized_symtab (bfd *abfd)
              /* Ordinary short filename, put into memory anyway.  The
                 Microsoft PE tools sometimes store a filename in
                 multiple AUX entries.  */
-             if (internal_ptr->u.syment.n_numaux > 1 && obj_pe (abfd))
-               internal_ptr->u.syment._n._n_n._n_offset =
-                 ((uintptr_t)
-                  copy_name (abfd,
-                             aux->u.auxent.x_file.x_n.x_fname,
-                             internal_ptr->u.syment.n_numaux * symesz));
+             size_t len;
+             char *src;
+             if (sym->u.syment.n_numaux > 1 && obj_pe (abfd))
+               {
+                 len = sym->u.syment.n_numaux * symesz;
+                 src = raw_src - (len - symesz);
+               }
              else
-               internal_ptr->u.syment._n._n_n._n_offset =
-                 ((uintptr_t)
-                  copy_name (abfd,
-                             aux->u.auxent.x_file.x_n.x_fname,
-                             (size_t) bfd_coff_filnmlen (abfd)));
+               {
+                 len = bfd_coff_filnmlen (abfd);
+                 src = aux->u.auxent.x_file.x_n.x_fname;
+               }
+             sym->u.syment._n._n_n._n_offset =
+               (uintptr_t) copy_name (abfd, src, len);
            }
 
          /* Normalize other strings available in C_FILE aux entries.  */
          if (!obj_pe (abfd))
-           for (int numaux = 1; numaux < internal_ptr->u.syment.n_numaux; numaux++)
+           for (int numaux = 1;
+                numaux < sym->u.syment.n_numaux;
+                numaux++)
              {
-               aux = internal_ptr + numaux + 1;
+               aux = sym + numaux + 1;
                BFD_ASSERT (! aux->is_sym);
 
                if (aux->u.auxent.x_file.x_n.x_n.x_zeroes == 0)
                  {
-                   /* The string information is a long one, point into the string table.  */
+                   /* The string information is a long one, point
+                      into the string table.  */
                    if (string_table == NULL)
                      {
                        string_table = _bfd_coff_read_string_table (abfd);
@@ -1989,48 +1975,48 @@ coff_get_normalized_symtab (bfd *abfd)
                          return NULL;
                      }
 
-                   if ((bfd_size_type)(aux->u.auxent.x_file.x_n.x_n.x_offset)
+                   if ((bfd_size_type) aux->u.auxent.x_file.x_n.x_n.x_offset
                        >= obj_coff_strings_len (abfd))
                      aux->u.auxent.x_file.x_n.x_n.x_offset =
                        (uintptr_t) _("<corrupt>");
                    else
                      aux->u.auxent.x_file.x_n.x_n.x_offset =
                        (uintptr_t) (string_table
-                                    + (aux->u.auxent.x_file.x_n.x_n.x_offset));
+                                    + aux->u.auxent.x_file.x_n.x_n.x_offset);
                  }
                else
                  aux->u.auxent.x_file.x_n.x_n.x_offset =
                    ((uintptr_t)
                     copy_name (abfd,
                                aux->u.auxent.x_file.x_n.x_fname,
-                               (size_t) bfd_coff_filnmlen (abfd)));
+                               bfd_coff_filnmlen (abfd)));
              }
 
        }
       else
        {
-         if (internal_ptr->u.syment._n._n_n._n_zeroes != 0)
+         if (sym->u.syment._n._n_n._n_zeroes != 0)
            {
              /* This is a "short" name.  Make it long.  */
-             size_t i;
              char *newstring;
 
              /* Find the length of this string without walking into memory
                 that isn't ours.  */
-             for (i = 0; i < 8; ++i)
-               if (internal_ptr->u.syment._n._n_name[i] == '\0')
+             for (i = 0; i < SYMNMLEN; ++i)
+               if (sym->u.syment._n._n_name[i] == '\0')
                  break;
 
-             newstring = (char *) bfd_zalloc (abfd, (bfd_size_type) (i + 1));
+             newstring = bfd_alloc (abfd, i + 1);
              if (newstring == NULL)
                return NULL;
-             strncpy (newstring, internal_ptr->u.syment._n._n_name, i);
-             internal_ptr->u.syment._n._n_n._n_offset = (uintptr_t) newstring;
-             internal_ptr->u.syment._n._n_n._n_zeroes = 0;
+             memcpy (newstring, sym->u.syment._n._n_name, i);
+             newstring[i] = 0;
+             sym->u.syment._n._n_n._n_offset = (uintptr_t) newstring;
+             sym->u.syment._n._n_n._n_zeroes = 0;
            }
-         else if (internal_ptr->u.syment._n._n_n._n_offset == 0)
-           internal_ptr->u.syment._n._n_n._n_offset = (uintptr_t) "";
-         else if (!bfd_coff_symname_in_debug (abfd, &internal_ptr->u.syment))
+         else if (sym->u.syment._n._n_n._n_offset == 0)
+           sym->u.syment._n._n_n._n_offset = (uintptr_t) "";
+         else if (!bfd_coff_symname_in_debug (abfd, &sym->u.syment))
            {
              /* Long name already.  Point symbol at the string in the
                 table.  */
@@ -2040,43 +2026,47 @@ coff_get_normalized_symtab (bfd *abfd)
                  if (string_table == NULL)
                    return NULL;
                }
-             if (internal_ptr->u.syment._n._n_n._n_offset >= obj_coff_strings_len (abfd)
-                 || string_table + internal_ptr->u.syment._n._n_n._n_offset < string_table)
-               internal_ptr->u.syment._n._n_n._n_offset =
+             if (sym->u.syment._n._n_n._n_offset >= obj_coff_strings_len (abfd))
+               sym->u.syment._n._n_n._n_offset =
                  (uintptr_t) _("<corrupt>");
              else
-               internal_ptr->u.syment._n._n_n._n_offset =
-                 ((uintptr_t) (string_table
-                               + internal_ptr->u.syment._n._n_n._n_offset));
+               sym->u.syment._n._n_n._n_offset =
+                 (uintptr_t) (string_table
+                              + sym->u.syment._n._n_n._n_offset);
            }
          else
            {
              /* Long name in debug section.  Very similar.  */
              if (debug_sec_data == NULL)
-               debug_sec_data = build_debug_section (abfd, & debug_sec);
-             if (debug_sec_data != NULL)
                {
-                 BFD_ASSERT (debug_sec != NULL);
-                 /* PR binutils/17512: Catch out of range offsets into the debug data.  */
-                 if (internal_ptr->u.syment._n._n_n._n_offset > debug_sec->size
-                     || debug_sec_data + internal_ptr->u.syment._n._n_n._n_offset < debug_sec_data)
-                   internal_ptr->u.syment._n._n_n._n_offset =
-                     (uintptr_t) _("<corrupt>");
-                 else
-                   internal_ptr->u.syment._n._n_n._n_offset =
-                     (uintptr_t) (debug_sec_data
-                                  + internal_ptr->u.syment._n._n_n._n_offset);
+                 debug_sec_data = build_debug_section (abfd, &debug_sec);
+                 if (debug_sec_data == NULL)
+                   return NULL;
                }
+             /* PR binutils/17512: Catch out of range offsets into
+                the debug data.  */
+             if (sym->u.syment._n._n_n._n_offset >= debug_sec->size)
+               sym->u.syment._n._n_n._n_offset =
+                 (uintptr_t) _("<corrupt>");
              else
-               internal_ptr->u.syment._n._n_n._n_offset = (uintptr_t) "";
+               sym->u.syment._n._n_n._n_offset =
+                 (uintptr_t) (debug_sec_data
+                              + sym->u.syment._n._n_n._n_offset);
            }
        }
-      internal_ptr += internal_ptr->u.syment.n_numaux;
+    }
+
+  /* Free the raw symbols.  */
+  if (obj_coff_external_syms (abfd) != NULL
+      && ! obj_coff_keep_syms (abfd))
+    {
+      free (obj_coff_external_syms (abfd));
+      obj_coff_external_syms (abfd) = NULL;
     }
 
   obj_raw_syments (abfd) = internal;
   BFD_ASSERT (obj_raw_syment_count (abfd)
-             == (unsigned int) (internal_ptr - internal));
+             == (size_t) (internal_ptr - internal));
 
   return internal;
 }
index 190b8f02a0b6f0404b5f5f37ef8c9daeae5fe1d4..f0677ff8857d8b109e1dfb751e97f9ce2bce0574 100644 (file)
@@ -402,8 +402,8 @@ coff_swap_aux_in (bfd *abfd,
                  void * ext1,
                  int type,
                  int in_class,
-                 int indx,
-                 int numaux,
+                 int indx ATTRIBUTE_UNUSED,
+                 int numaux ATTRIBUTE_UNUSED,
                  void * in1)
 {
   AUXENT *ext = (AUXENT *) ext1;
@@ -426,14 +426,7 @@ coff_swap_aux_in (bfd *abfd,
 #if FILNMLEN != E_FILNMLEN
 #error we need to cope with truncating or extending FILNMLEN
 #else
-         if (numaux > 1 && obj_pe (abfd))
-           {
-             if (indx == 0)
-               memcpy (in->x_file.x_n.x_fname, ext->x_file.x_fname,
-                       numaux * sizeof (AUXENT));
-           }
-         else
-           memcpy (in->x_file.x_n.x_fname, ext->x_file.x_fname, FILNMLEN);
+         memcpy (in->x_file.x_n.x_fname, ext->x_file.x_fname, FILNMLEN);
 #endif
        }
       goto end;