[binutils-gdb] xcoff reading dynamic relocs

[Alan Modra]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a21e2f0c20565216fbc560225ba15d8c5c1e1aa8

commit a21e2f0c20565216fbc560225ba15d8c5c1e1aa8
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Dec 11 17:02:00 2024 +1030

    xcoff reading dynamic relocs
    
    This adds a sanity check to relocation symbol indices, and tidies code
    a little.
    
    The patch does result in a couple of testsuite failures
    rs6000-aix7.2  +FAIL: TLS relocations (32-bit)
    rs6000-aix7.2  +FAIL: TLS relocations (64-bit)
    
    That seems reasonable to me, because prior to this patch l_symndx was
    being set to -1 and -2 for .tdata and .tbss symbols resulting in a
    buffer overflow when accessing the syms array.
    
    bfd/
            * xcofflink.c (_bfd_xcoff_canonicalize_dynamic_reloc): Prevent
            symbol array overflow on invalid relocation symbol index.
            Tidy code for relocs against standard sections.
            (xcoff_create_ldrel): Remove cast.
    include/
            * coff/xcoff.h (struct internal_ldrel): Make l_symndx uint32_t.
            Make l_rtype and l_rsecnm int16_t.

Diff:
---
 bfd/xcofflink.c      | 41 +++++++++++++++++------------------------
 include/coff/xcoff.h |  6 +++---
 2 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/bfd/xcofflink.c b/bfd/xcofflink.c
index 49ac8efd1e8..b75fb42eed5 100644
--- a/bfd/xcofflink.c
+++ b/bfd/xcofflink.c
@@ -439,30 +439,13 @@ _bfd_xcoff_canonicalize_dynamic_reloc (bfd *abfd,
 
       bfd_xcoff_swap_ldrel_in (abfd, elrel, &ldrel);
 
-      if (ldrel.l_symndx >= 3)
-	relbuf->sym_ptr_ptr = syms + (ldrel.l_symndx - 3);
-      else
+      if (ldrel.l_symndx == -1u)
+	relbuf->sym_ptr_ptr = bfd_abs_section_ptr->symbol_ptr_ptr;
+      else if (ldrel.l_symndx < 3)
 	{
-	  const char *name;
-	  asection *sec;
-
-	  switch (ldrel.l_symndx)
-	    {
-	    case 0:
-	      name = ".text";
-	      break;
-	    case 1:
-	      name = ".data";
-	      break;
-	    case 2:
-	      name = ".bss";
-	      break;
-	    default:
-	      abort ();
-	      break;
-	    }
-
-	  sec = bfd_get_section_by_name (abfd, name);
+	  static const char stdsec[3][8] = { ".text", ".data", ".bss" };
+	  const char *name = stdsec[ldrel.l_symndx];
+	  asection *sec = bfd_get_section_by_name (abfd, name);
 	  if (sec == NULL)
 	    {
 	      bfd_set_error (bfd_error_bad_value);
@@ -471,6 +454,16 @@ _bfd_xcoff_canonicalize_dynamic_reloc (bfd *abfd,
 
 	  relbuf->sym_ptr_ptr = sec->symbol_ptr_ptr;
 	}
+      else if (ldrel.l_symndx - 3 < ldhdr.l_nsyms)
+	relbuf->sym_ptr_ptr = syms + (ldrel.l_symndx - 3);
+      else
+	{
+	  _bfd_error_handler
+	    /* xgettext:c-format */
+	    (_("%pB: warning: illegal symbol index %lu in relocs"),
+	     abfd, (unsigned long) ldrel.l_symndx);
+	  relbuf->sym_ptr_ptr = bfd_abs_section_ptr->symbol_ptr_ptr;
+	}
 
       relbuf->address = ldrel.l_vaddr;
       relbuf->addend = 0;
@@ -5097,7 +5090,7 @@ xcoff_create_ldrel (bfd *output_bfd, struct xcoff_final_link_info *flinfo,
       ldrel.l_symndx = h->ldindx;
     }
   else
-    ldrel.l_symndx = -(bfd_size_type) 1;
+    ldrel.l_symndx = -1;
 
   ldrel.l_rtype = (irel->r_size << 8) | irel->r_type;
   ldrel.l_rsecnm = output_section->target_index;
diff --git a/include/coff/xcoff.h b/include/coff/xcoff.h
index 104660fa748..82d82ebdd3d 100644
--- a/include/coff/xcoff.h
+++ b/include/coff/xcoff.h
@@ -298,13 +298,13 @@ struct internal_ldrel
   bfd_vma l_vaddr;
 
   /* The symbol table index in the .loader section symbol table.  */
-  bfd_size_type l_symndx;
+  uint32_t l_symndx;
 
   /* The relocation type and size.  */
-  short l_rtype;
+  int16_t l_rtype;
 
   /* The section number this relocation applies to.  */
-  short l_rsecnm;
+  int16_t l_rsecnm;
 };
 
 /* An entry in the XCOFF linker hash table.  */