public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: binutils@sourceware.org
Cc: "Maciej W . Rozycki" <macro@orcam.me.uk>,
	Chenghua Xu <paul.hua.gm@gmail.com>,
	Alan Modra <amodra@gmail.com>
Subject: Move mips_hi16_list to mips_elf_section_data
Date: Tue, 19 Dec 2023 20:05:45 +1030	[thread overview]
Message-ID: <20231219093546.2112095-2-amodra@gmail.com> (raw)
In-Reply-To: <20231219093546.2112095-1-amodra@gmail.com>

This patch is in response to fuzzing testcases that manage to cause
segfaults due to stale references to freed memory via mips_hi16.data.

A number of the error/warning handlers in ldmain.c use %C.  This can
cause debug info to be parsed for the first time in order to print
file/function/line.  If one of those warnings is triggered after some
hi16 relocs have been processed but before the matching lo16 reloc is
handled, *and* the debug info is corrupted with a lo16 reloc, then the
mips_hi16_list will be flushed with the result that printing a warning
changes linker output.  It is also possible that corrupted debug info
adds to the hi16 list, with the result that when the linker handles a
later lo16 reloc in a text section, ld will segfault accessing
mips_hi16.data after the debug buffers have be freed.  Both of these
problems are fixed by keeping a per-section mips_hi16_list rather than
a per-file list.

	* elfxx-mips.c (struct mips_hi16): Move earlier, deleting
	input_section field.
	(struct _mips_elf_section_data): Add mips_hi16_list.
	(struct mips_elf_obj_tdata): Delete mips_hi16_list.
	(free_mips_hi16_list): New function.
	(_bfd_mips_elf_free_cached_info): Adjust to suit new location
	of mips_hi16_list.
	(_bfd_mips_elf_hi16_reloc, _bfd_mips_elf_lo16_reloc): Likewise.
	(_bfd_elf_mips_get_relocated_section_contents): Likewise.

diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
index bae8622fd34..00111553c30 100644
--- a/bfd/elfxx-mips.c
+++ b/bfd/elfxx-mips.c
@@ -222,6 +222,19 @@ struct mips_elf_traverse_got_arg
   int value;
 };
 
+/* Used to store a REL high-part relocation such as R_MIPS_HI16 or
+   R_MIPS_GOT16.  DATA nominally points to the start of the section
+   contents, but note that gas may use multiple chunks of memory for a
+   section (with DATA + [offset,offset+frag_size) addressing a given
+   frag).  A hi16 reloc might need a different "data" to a lo16.  */
+
+struct mips_hi16
+{
+  struct mips_hi16 *next;
+  bfd_byte *data;
+  arelent rel;
+};
+
 struct _mips_elf_section_data
 {
   struct bfd_elf_section_data elf;
@@ -229,6 +242,8 @@ struct _mips_elf_section_data
   {
     bfd_byte *tdata;
   } u;
+
+  struct mips_hi16 *mips_hi16_list;
 };
 
 #define mips_elf_section_data(sec) \
@@ -549,19 +564,6 @@ struct mips_htab_traverse_info
   bool error;
 };
 
-/* Used to store a REL high-part relocation such as R_MIPS_HI16 or
-   R_MIPS_GOT16.  REL is the relocation, INPUT_SECTION is the section
-   that contains the relocation field and DATA points to the start of
-   INPUT_SECTION.  */
-
-struct mips_hi16
-{
-  struct mips_hi16 *next;
-  bfd_byte *data;
-  asection *input_section;
-  arelent rel;
-};
-
 /* MIPS ELF private object data.  */
 
 struct mips_elf_obj_tdata
@@ -597,8 +599,6 @@ struct mips_elf_obj_tdata
   asymbol *elf_text_symbol;
   asection *elf_data_section;
   asection *elf_text_section;
-
-  struct mips_hi16 *mips_hi16_list;
 };
 
 /* Get MIPS ELF private object data from BFD's tdata.  */
@@ -1389,6 +1389,30 @@ struct mips_elf_find_line
   struct ecoff_find_line i;
 };
 
+/* Free the mips_hi16_list attached to S.  Return true if there were
+   unmatched hi16 relocs.  */
+
+static bool
+free_mips_hi16_list (asection *s)
+{
+  struct mips_hi16 *hi;
+  struct mips_hi16 **hip = &mips_elf_section_data (s)->mips_hi16_list;
+  bool ret = false;
+
+  while ((hi = *hip) != NULL)
+    {
+      *hip = hi->next;
+      /* See gas/config/tc-mips.c reloc_needs_lo_p.  Not all hi16
+	 relocs need lo16 relocs.  */
+      if (hi->rel.howto->type == R_MIPS_HI16
+	  || hi->rel.howto->type == R_MIPS16_HI16
+	  || hi->rel.howto->type == R_MICROMIPS_HI16)
+	ret = true;
+      free (hi);
+    }
+  return ret;
+}
+
 bool
 _bfd_mips_elf_free_cached_info (bfd *abfd)
 {
@@ -1399,14 +1423,12 @@ _bfd_mips_elf_free_cached_info (bfd *abfd)
       && (tdata = mips_elf_tdata (abfd)) != NULL)
     {
       BFD_ASSERT (tdata->root.object_id == MIPS_ELF_DATA);
-      while (tdata->mips_hi16_list != NULL)
-	{
-	  struct mips_hi16 *hi = tdata->mips_hi16_list;
-	  tdata->mips_hi16_list = hi->next;
-	  free (hi);
-	}
       if (tdata->find_line_info != NULL)
 	_bfd_ecoff_free_ecoff_debug_info (&tdata->find_line_info->d);
+      for (asection *s = abfd->sections; s; s = s->next)
+	if (free_mips_hi16_list (s))
+	  _bfd_error_handler
+	    (_("%pB(%pA): unmatched hi16 reloc"), abfd, s);
     }
   return _bfd_elf_free_cached_info (abfd);
 }
@@ -2537,22 +2559,18 @@ _bfd_mips_elf_hi16_reloc (bfd *abfd, arelent *reloc_entry,
 			  asection *input_section, bfd *output_bfd,
 			  char **error_message ATTRIBUTE_UNUSED)
 {
-  struct mips_hi16 *n;
-  struct mips_elf_obj_tdata *tdata;
-
   if (reloc_entry->address > bfd_get_section_limit (abfd, input_section))
     return bfd_reloc_outofrange;
 
-  n = bfd_malloc (sizeof *n);
+  struct mips_hi16 *n = bfd_malloc (sizeof (*n));
   if (n == NULL)
     return bfd_reloc_outofrange;
 
-  tdata = mips_elf_tdata (abfd);
-  n->next = tdata->mips_hi16_list;
+  struct _mips_elf_section_data *sdata = mips_elf_section_data (input_section);
+  n->next = sdata->mips_hi16_list;
   n->data = data;
-  n->input_section = input_section;
   n->rel = *reloc_entry;
-  tdata->mips_hi16_list = n;
+  sdata->mips_hi16_list = n;
 
   if (output_bfd != NULL)
     reloc_entry->address += input_section->output_offset;
@@ -2590,64 +2608,66 @@ _bfd_mips_elf_lo16_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
 			  void *data, asection *input_section,
 			  bfd *output_bfd, char **error_message)
 {
-  bfd_vma vallo;
-  bfd_byte *location = (bfd_byte *) data + reloc_entry->address;
-  struct mips_elf_obj_tdata *tdata;
+  struct _mips_elf_section_data *sdata = mips_elf_section_data (input_section);
 
-  if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd, input_section,
-				  reloc_entry->address))
-    return bfd_reloc_outofrange;
+  if (sdata->mips_hi16_list != NULL)
+    {
+      if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd, input_section,
+				      reloc_entry->address))
+	return bfd_reloc_outofrange;
 
-  _bfd_mips_elf_reloc_unshuffle (abfd, reloc_entry->howto->type, false,
-				 location);
-  /* The high 16 bits of the addend are stored in the high insn, the
-     low 16 bits in the low insn, but there is a catch:  You can't
-     just concatenate the high and low parts.  The high part of the
-     addend is adjusted for the fact that the low part is sign
-     extended.  For example, an addend of 0x38000 would have 0x0004 in
-     the high part and 0x8000 (=0xff..f8000) in the low part.
-     To extract the actual addend, calculate (a)
-     ((hi & 0xffff) << 16) + ((lo & 0xffff) ^ 0x8000) - 0x8000.
-     We will be applying (symbol + addend) & 0xffff to the low insn,
-     and we want to apply (b) (symbol + addend + 0x8000) >> 16 to the
-     high insn (the +0x8000 adjusting for when the applied low part is
-     negative).  Substituting (a) into (b) and recognising that
-     (hi & 0xffff) is already in the high insn gives a high part
-     addend adjustment of (lo & 0xffff) ^ 0x8000.  */
-  vallo = (bfd_get_32 (abfd, location) & 0xffff) ^ 0x8000;
-  _bfd_mips_elf_reloc_shuffle (abfd, reloc_entry->howto->type, false,
-			       location);
+      bfd_byte *location = (bfd_byte *) data + reloc_entry->address;
+      _bfd_mips_elf_reloc_unshuffle (abfd, reloc_entry->howto->type, false,
+				     location);
+      /* The high 16 bits of the addend are stored in the high insn, the
+	 low 16 bits in the low insn, but there is a catch:  You can't
+	 just concatenate the high and low parts.  The high part of the
+	 addend is adjusted for the fact that the low part is sign
+	 extended.  For example, an addend of 0x38000 would have 0x0004 in
+	 the high part and 0x8000 (=0xff..f8000) in the low part.
+	 To extract the actual addend, calculate (a)
+	 ((hi & 0xffff) << 16) + ((lo & 0xffff) ^ 0x8000) - 0x8000.
+	 We will be applying (symbol + addend) & 0xffff to the low insn,
+	 and we want to apply (b) (symbol + addend + 0x8000) >> 16 to the
+	 high insn (the +0x8000 adjusting for when the applied low part is
+	 negative).  Substituting (a) into (b) and recognising that
+	 (hi & 0xffff) is already in the high insn gives a high part
+	 addend adjustment of (lo & 0xffff) ^ 0x8000.  */
+      bfd_vma vallo = (bfd_get_32 (abfd, location) & 0xffff) ^ 0x8000;
+      _bfd_mips_elf_reloc_shuffle (abfd, reloc_entry->howto->type, false,
+				   location);
 
-  tdata = mips_elf_tdata (abfd);
-  while (tdata->mips_hi16_list != NULL)
-    {
-      bfd_reloc_status_type ret;
-      struct mips_hi16 *hi;
-
-      hi = tdata->mips_hi16_list;
-
-      /* R_MIPS*_GOT16 relocations are something of a special case.  We
-	 want to install the addend in the same way as for a R_MIPS*_HI16
-	 relocation (with a rightshift of 16).  However, since GOT16
-	 relocations can also be used with global symbols, their howto
-	 has a rightshift of 0.  */
-      if (hi->rel.howto->type == R_MIPS_GOT16)
-	hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS_HI16, false);
-      else if (hi->rel.howto->type == R_MIPS16_GOT16)
-	hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS16_HI16, false);
-      else if (hi->rel.howto->type == R_MICROMIPS_GOT16)
-	hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MICROMIPS_HI16, false);
-
-      hi->rel.addend += vallo;
-
-      ret = _bfd_mips_elf_generic_reloc (abfd, &hi->rel, symbol, hi->data,
-					 hi->input_section, output_bfd,
-					 error_message);
-      if (ret != bfd_reloc_ok)
-	return ret;
-
-      tdata->mips_hi16_list = hi->next;
-      free (hi);
+      while (sdata->mips_hi16_list != NULL)
+	{
+	  bfd_reloc_status_type ret;
+	  struct mips_hi16 *hi = sdata->mips_hi16_list;
+
+	  /* R_MIPS*_GOT16 relocations are something of a special case.
+	     We want to install the addend in the same way as for a
+	     R_MIPS*_HI16 relocation (with a rightshift of 16).
+	     However, since GOT16 relocations can also be used with
+	     global symbols, their howto has a rightshift of 0.  */
+	  if (hi->rel.howto->type == R_MIPS_GOT16)
+	    hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS_HI16,
+						     false);
+	  else if (hi->rel.howto->type == R_MIPS16_GOT16)
+	    hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MIPS16_HI16,
+						     false);
+	  else if (hi->rel.howto->type == R_MICROMIPS_GOT16)
+	    hi->rel.howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, R_MICROMIPS_HI16,
+						     false);
+
+	  hi->rel.addend += vallo;
+
+	  ret = _bfd_mips_elf_generic_reloc (abfd, &hi->rel, symbol, hi->data,
+					     input_section, output_bfd,
+					     error_message);
+	  if (ret != bfd_reloc_ok)
+	    return ret;
+
+	  sdata->mips_hi16_list = hi->next;
+	  free (hi);
+	}
     }
 
   return _bfd_mips_elf_generic_reloc (abfd, reloc_entry, symbol, data,
@@ -13333,24 +13353,8 @@ _bfd_elf_mips_get_relocated_section_contents
   reloc_vector = (arelent **) bfd_malloc (reloc_size);
   if (reloc_vector == NULL)
     {
-      struct mips_elf_obj_tdata *tdata;
-      struct mips_hi16 **hip, *hi;
     error_return:
-      /* If we are going to return an error, remove entries on
-	 mips_hi16_list that point into this section's data.  Data
-	 will typically be freed on return from this function.  */
-      tdata = mips_elf_tdata (abfd);
-      hip = &tdata->mips_hi16_list;
-      while ((hi = *hip) != NULL)
-	{
-	  if (hi->input_section == input_section)
-	    {
-	      *hip = hi->next;
-	      free (hi);
-	    }
-	  else
-	    hip = &hi->next;
-	}
+      free_mips_hi16_list (input_section);
       if (orig_data == NULL)
 	free (data);
       data = NULL;

  reply	other threads:[~2023-12-19  9:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-19  9:35 mips hi16/lo16 reloc handling Alan Modra
2023-12-19  9:35 ` Alan Modra [this message]
2023-12-19  9:35 ` coff-mips refhi list Alan Modra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231219093546.2112095-2-amodra@gmail.com \
    --to=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=macro@orcam.me.uk \
    --cc=paul.hua.gm@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).