xcoff dynamic symbol string sanity

Message ID Z_Pc5RUto3cAiZmR@squeak.grove.modra.org
State New
Headers
Series xcoff dynamic symbol string sanity |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_build--master-arm fail Patch failed to apply

Commit Message

Alan Modra April 7, 2025, 2:10 p.m. UTC
  Sanity check symbol string table offsets, and tidy structs.  "long"
isn't a good choice for _l_zeroes and _l_offset since it can be 64
bits which blows out the size of the symbol struct unnecessarily.
Also, all of the sizes in internal_ldsym need only be 32 bits, but I
made them size_t because I didn't want to audit all expressions using
them for overflow.

bfd/
	* xcofflink.c (_bfd_xcoff_canonicalize_dynamic_symtab): Sanity
	check symbol _l_offset.
	(xcoff_link_add_dynamic_symbols),
	(xcoff_link_check_dynamic_ar_symbols): Likewise.
include/
	* coff/xcoff.h (struct internal_ldhdr): Tidy types.
	(struct internal_ldsym): Use uint32_t for _l_zeroes and _l_offset.
  

Patch

diff --git a/bfd/xcofflink.c b/bfd/xcofflink.c
index 5f0efb45df2..b50b17fab1c 100644
--- a/bfd/xcofflink.c
+++ b/bfd/xcofflink.c
@@ -367,9 +367,7 @@  _bfd_xcoff_canonicalize_dynamic_symtab (bfd *abfd, asymbol **psyms)
 
       symbuf->symbol.the_bfd = abfd;
 
-      if (ldsym._l._l_l._l_zeroes == 0)
-	symbuf->symbol.name = strings + ldsym._l._l_l._l_offset;
-      else
+      if (ldsym._l._l_l._l_zeroes != 0)
 	{
 	  char *c;
 
@@ -380,6 +378,10 @@  _bfd_xcoff_canonicalize_dynamic_symtab (bfd *abfd, asymbol **psyms)
 	  c[SYMNMLEN] = '\0';
 	  symbuf->symbol.name = c;
 	}
+      else if (ldsym._l._l_l._l_offset < ldhdr.l_stlen)
+	symbuf->symbol.name = strings + ldsym._l._l_l._l_offset;
+      else
+	symbuf->symbol.name = _("<corrupt>");
 
       if (ldsym.l_smclas == XMC_XO)
 	symbuf->symbol.section = bfd_abs_section_ptr;
@@ -973,14 +975,16 @@  xcoff_link_add_dynamic_symbols (bfd *abfd, struct bfd_link_info *info)
       if ((ldsym.l_smtype & L_EXPORT) == 0)
 	continue;
 
-      if (ldsym._l._l_l._l_zeroes == 0)
-	name = strings + ldsym._l._l_l._l_offset;
-      else
+      if (ldsym._l._l_l._l_zeroes != 0)
 	{
 	  memcpy (nambuf, ldsym._l._l_name, SYMNMLEN);
 	  nambuf[SYMNMLEN] = '\0';
 	  name = nambuf;
 	}
+      else if (ldsym._l._l_l._l_offset < ldhdr.l_stlen)
+	name = strings + ldsym._l._l_l._l_offset;
+      else
+	continue;
 
       /* Normally we could not call xcoff_link_hash_lookup in an add
 	 symbols routine, since we might not be using an XCOFF hash
@@ -2429,14 +2433,16 @@  xcoff_link_check_dynamic_ar_symbols (bfd *abfd,
       if ((ldsym.l_smtype & L_EXPORT) == 0)
 	continue;
 
-      if (ldsym._l._l_l._l_zeroes == 0)
-	name = strings + ldsym._l._l_l._l_offset;
-      else
+      if (ldsym._l._l_l._l_zeroes != 0)
 	{
 	  memcpy (nambuf, ldsym._l._l_name, SYMNMLEN);
 	  nambuf[SYMNMLEN] = '\0';
 	  name = nambuf;
 	}
+      else if (ldsym._l._l_l._l_offset < ldhdr.l_stlen)
+	name = strings + ldsym._l._l_l._l_offset;
+      else
+	continue;
 
       h = bfd_link_hash_lookup (info->hash, name, false, false, true);
 
diff --git a/include/coff/xcoff.h b/include/coff/xcoff.h
index 86887423bd9..cae0029c4b4 100644
--- a/include/coff/xcoff.h
+++ b/include/coff/xcoff.h
@@ -204,30 +204,30 @@  struct internal_ldhdr
   /* The version number: 
      1 : 32 bit
      2 : 64 bit */
-  unsigned long l_version;
+  unsigned int l_version;
 
   /* The number of symbol table entries.  */
-  bfd_size_type l_nsyms;
+  size_t l_nsyms;
 
   /* The number of relocation table entries.  */
-  bfd_size_type l_nreloc;
+  size_t l_nreloc;
 
   /* The length of the import file string table.  */
-  bfd_size_type l_istlen;
+  size_t l_istlen;
 
   /* The number of import files.  */
-  bfd_size_type l_nimpid;
+  size_t l_nimpid;
+
+  /* The length of the string table.  */
+  size_t l_stlen;
 
   /* The offset from the start of the .loader section to the first
      entry in the import file table.  */
-  bfd_size_type l_impoff;
-
-  /* The length of the string table.  */
-  bfd_size_type l_stlen;
+  bfd_vma l_impoff;
 
   /* The offset from the start of the .loader section to the first
      entry in the string table.  */
-  bfd_size_type l_stoff;
+  bfd_vma l_stoff;
 
   /* The offset to start of the symbol table, only in XCOFF64 */
   bfd_vma l_symoff;
@@ -248,11 +248,11 @@  struct internal_ldsym
     struct
     {
       /* Zero if the symbol name is more than SYMNMLEN characters.  */
-	long _l_zeroes;
+      uint32_t _l_zeroes;
       
       /* The offset in the string table if the symbol name is more
 	 than SYMNMLEN characters.  */
-      long _l_offset;
+      uint32_t _l_offset;
     } 
     _l_l;
   }