diff mbox series

PR 34327 Out of bounds accesses in reloc special functions.

Message ID akXVE3vcyZt4qhY-@squeak.grove.modra.org
State New
Headers
Series PR 34327 Out of bounds accesses in reloc special functions. |

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 July 2, 2026, 3:03 a.m. UTC 
  As per the PR, s12z lacked any reloc offset sanity checking, the
others all just checked that the offset started within the section
rather than checking the field was contained in the section.

Using the proper check for mmix exposed a problem in the howto table,
present since the initial mmix commit.  The R_MMIX_BASE_PLUS_OFFSET
field is actually two bytes, located at the reloc address.  Making it
an eight byte field is just wrong, as doing that indicates the field
is at the reloc address plus six bytes for a big-endian target.
Presumably this was done for overlow reporting, which is properly done
by appropriately setting complain_on_overflow.

	* elf32-d30v.c (bfd_elf_d30v_reloc, bfd_elf_d30v_reloc_21): Use
	bfd_reloc_offset_in_range
	* elf32-s12z.c (opru18_reloc): Likewise.
	* elf32-spu.c (spu_elf_rel9): Likewise.
	* elf32-xstormy16.c (xstormy16_elf_24_reloc): Likewise.
	* elf32-visium.c (visium_elf_howto_parity_reloc): Likewise.
	Remove unnecessary casts too.
	* elf64-s390.c (s390_elf_ldisp_reloc): Likewise.
	* elfxx-sparc.c (init_insn_reloc): Likewise.
	* elf64-mmix.c (mmix_elf_reloc): Likewise.  Remove unnecessary
	variable too.
	(elf_mmix_howto_table[R_MMIX_BASE_PLUS_OFFSET]): Correct size,
	bitsize and complain_on_overflow.

Comments

Hans-Peter Nilsson July 2, 2026, 3:40 a.m. UTC | #1 
On Thu, 2 Jul 2026, Alan Modra wrote:

> As per the PR, s12z lacked any reloc offset sanity checking, the
> others all just checked that the offset started within the section
> rather than checking the field was contained in the section.
> 
> Using the proper check for mmix exposed a problem in the howto table,
> present since the initial mmix commit.

Looks like it; R_MMIX_BASE_PLUS_OFFSET was there originally, but 
most of the implementation came later.

>  The R_MMIX_BASE_PLUS_OFFSET
> field is actually two bytes, located at the reloc address.  Making it
> an eight byte field is just wrong, as doing that indicates the field
> is at the reloc address plus six bytes for a big-endian target.

Yeah, I might have misunderstood a thing here.

> Presumably this was done for overlow reporting, which is properly done
> by appropriately setting complain_on_overflow.

Which you now change to complain_overflow_dont?

Not saying the change is incorrect (it certainly looks right, 
thanks), because if it was, I have confidence that the 
test-suite would trigger a failure, but that wording is 
confusing.

brgds, H-P
Alan Modra July 2, 2026, 6:04 a.m. UTC | #2 
On Wed, Jul 01, 2026 at 11:40:40PM -0400, Hans-Peter Nilsson wrote:
> On Thu, 2 Jul 2026, Alan Modra wrote:
> > Presumably this was done for overlow reporting, which is properly done
> > by appropriately setting complain_on_overflow.
> 
> Which you now change to complain_overflow_dont?
> 
> Not saying the change is incorrect (it certainly looks right, 
> thanks), because if it was, I have confidence that the 
> test-suite would trigger a failure, but that wording is 
> confusing.

I meant, presumably this was done to disable overflow reporting.
diff mbox series

Patch

diff --git a/bfd/elf32-d30v.c b/bfd/elf32-d30v.c
index 95277412346..a74107cff21 100644
--- a/bfd/elf32-d30v.c
+++ b/bfd/elf32-d30v.c
@@ -66,8 +66,8 @@  bfd_elf_d30v_reloc (bfd *abfd,
       && output_bfd == NULL)
     flag = bfd_reloc_undefined;
 
-  /* Is the address of the relocation really within the section?  */
-  if (reloc_entry->address > bfd_get_section_limit (abfd, input_section))
+  if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd,
+				  input_section, reloc_entry->address))
     return bfd_reloc_outofrange;
 
   /* Work out which section the relocation is targeted at and the
@@ -174,8 +174,8 @@  bfd_elf_d30v_reloc_21 (bfd *abfd,
       && output_bfd == NULL)
     flag = bfd_reloc_undefined;
 
-  /* Is the address of the relocation really within the section?  */
-  if (reloc_entry->address > bfd_get_section_limit (abfd, input_section))
+  if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd,
+				  input_section, reloc_entry->address))
     return bfd_reloc_outofrange;
 
   /* Work out which section the relocation is targeted at and the
diff --git a/bfd/elf32-s12z.c b/bfd/elf32-s12z.c
index 1630895136e..52eaf4f4e6c 100644
--- a/bfd/elf32-s12z.c
+++ b/bfd/elf32-s12z.c
@@ -48,6 +48,9 @@  opru18_reloc (bfd *abfd, arelent *reloc_entry, struct bfd_symbol *symbol,
 
   bfd_size_type octets = (reloc_entry->address
 			  * OCTETS_PER_BYTE (abfd, input_section));
+  if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd,
+				  input_section, octets))
+    return bfd_reloc_outofrange;
   bfd_vma result = bfd_get_24 (abfd, (unsigned char *) data + octets);
   bfd_vma val = bfd_asymbol_value (symbol);
 
diff --git a/bfd/elf32-spu.c b/bfd/elf32-spu.c
index e0e74b781ff..5f1183ce7d1 100644
--- a/bfd/elf32-spu.c
+++ b/bfd/elf32-spu.c
@@ -213,9 +213,10 @@  spu_elf_rel9 (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
     return bfd_elf_generic_reloc (abfd, reloc_entry, symbol, data,
 				  input_section, output_bfd, error_message);
 
-  if (reloc_entry->address > bfd_get_section_limit (abfd, input_section))
-    return bfd_reloc_outofrange;
   octets = reloc_entry->address * OCTETS_PER_BYTE (abfd, input_section);
+  if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd,
+				  input_section, octets))
+    return bfd_reloc_outofrange;
 
   /* Get symbol value.  */
   val = 0;
diff --git a/bfd/elf32-visium.c b/bfd/elf32-visium.c
index fe5f4d5eafb..b862b13614e 100644
--- a/bfd/elf32-visium.c
+++ b/bfd/elf32-visium.c
@@ -303,7 +303,7 @@  visium_parity_bit (bfd_vma insn)
    It sets instruction parity to even.  This cannot be done by a howto.  */
 
 static bfd_reloc_status_type
-visium_elf_howto_parity_reloc (bfd * input_bfd, arelent *reloc_entry,
+visium_elf_howto_parity_reloc (bfd *input_bfd, arelent *reloc_entry,
 			       asymbol *symbol, void *data,
 			       asection *input_section, bfd *output_bfd,
 			       char **error_message ATTRIBUTE_UNUSED)
@@ -316,7 +316,7 @@  visium_elf_howto_parity_reloc (bfd * input_bfd, arelent *reloc_entry,
   /* This part is from bfd_elf_generic_reloc.
      If we're relocating, and this an external symbol, we don't want
      to change anything.  */
-  if (output_bfd != (bfd *) NULL && (symbol->flags & BSF_SECTION_SYM) == 0)
+  if (output_bfd != NULL && (symbol->flags & BSF_SECTION_SYM) == 0)
     {
       reloc_entry->address += input_section->output_offset;
       return bfd_reloc_ok;
@@ -324,21 +324,21 @@  visium_elf_howto_parity_reloc (bfd * input_bfd, arelent *reloc_entry,
 
   /* Now do the reloc in the usual way.  */
 
-  /* Sanity check the address (offset in section).  */
-  if (reloc_entry->address > bfd_get_section_limit (input_bfd, input_section))
+  if (!bfd_reloc_offset_in_range (reloc_entry->howto, input_bfd,
+				  input_section, reloc_entry->address))
     return bfd_reloc_outofrange;
 
   ret = bfd_reloc_ok;
-  if (bfd_is_und_section (symbol->section) && output_bfd == (bfd *) NULL)
+  if (bfd_is_und_section (symbol->section) && output_bfd == NULL)
     ret = bfd_reloc_undefined;
 
-  if (bfd_is_com_section (symbol->section) || output_bfd != (bfd *) NULL)
+  if (bfd_is_com_section (symbol->section) || output_bfd != NULL)
     relocation = 0;
   else
     relocation = symbol->value;
 
   /* Only do this for a final link.  */
-  if (output_bfd == (bfd *) NULL)
+  if (output_bfd == NULL)
     {
       relocation += symbol->section->output_section->vma;
       relocation += symbol->section->output_offset;
@@ -383,7 +383,7 @@  visium_elf_howto_parity_reloc (bfd * input_bfd, arelent *reloc_entry,
   insn |= visium_parity_bit (insn);
   bfd_put_32 (input_bfd, insn, inplace_address);
 
-  if (output_bfd != (bfd *) NULL)
+  if (output_bfd != NULL)
     reloc_entry->address += input_section->output_offset;
 
   return ret;
diff --git a/bfd/elf32-xstormy16.c b/bfd/elf32-xstormy16.c
index 60ab5e6875a..04782449b8a 100644
--- a/bfd/elf32-xstormy16.c
+++ b/bfd/elf32-xstormy16.c
@@ -44,7 +44,8 @@  xstormy16_elf_24_reloc (bfd *abfd,
       return bfd_reloc_ok;
     }
 
-  if (reloc_entry->address > bfd_get_section_limit (abfd, input_section))
+  if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd,
+				  input_section, reloc_entry->address))
     return bfd_reloc_outofrange;
 
   if (bfd_is_com_section (symbol->section))
diff --git a/bfd/elf64-mmix.c b/bfd/elf64-mmix.c
index 4036dcb7c3d..ca611b75012 100644
--- a/bfd/elf64-mmix.c
+++ b/bfd/elf64-mmix.c
@@ -703,16 +703,14 @@  static reloc_howto_type elf_mmix_howto_table[] =
 	 0xff,			/* dst_mask */
 	 false),		/* pcrel_offset */
 
-  /* A register plus an index, corresponding to the relocation expression.
-     The sizes must correspond to the valid range of the expression, while
-     the bitmasks correspond to what we store in the image.  */
+  /* A register plus an index, corresponding to the relocation expression.  */
   HOWTO (R_MMIX_BASE_PLUS_OFFSET,	/* type */
 	 0,			/* rightshift */
-	 8,			/* size */
-	 64,			/* bitsize */
+	 2,			/* size */
+	 16,			/* bitsize */
 	 false,			/* pc_relative */
 	 0,			/* bitpos */
-	 complain_overflow_bitfield, /* complain_on_overflow */
+	 complain_overflow_dont, /* complain_on_overflow */
 	 mmix_elf_reloc,	/* special_function */
 	 "R_MMIX_BASE_PLUS_OFFSET", /* name */
 	 false,			/* partial_inplace */
@@ -1282,8 +1280,7 @@  mmix_elf_reloc (bfd *abfd,
   bfd_vma relocation;
   bfd_reloc_status_type r;
   asection *reloc_target_output_section;
-  bfd_reloc_status_type flag = bfd_reloc_ok;
-  bfd_vma output_base = 0;
+  bfd_vma output_base;
 
   r = bfd_elf_generic_reloc (abfd, reloc_entry, symbol, data,
 			     input_section, output_bfd, error_message);
@@ -1295,13 +1292,9 @@  mmix_elf_reloc (bfd *abfd,
 
   if (bfd_is_und_section (symbol->section)
       && (symbol->flags & BSF_WEAK) == 0
-      && output_bfd == (bfd *) NULL)
+      && output_bfd == NULL)
     return bfd_reloc_undefined;
 
-  /* Is the address of the relocation really within the section?  */
-  if (reloc_entry->address > bfd_get_section_limit (abfd, input_section))
-    return bfd_reloc_outofrange;
-
   /* Work out which section the relocation is targeted at and the
      initial relocation command value.  */
 
@@ -1322,7 +1315,7 @@  mmix_elf_reloc (bfd *abfd,
 
   relocation += output_base + symbol->section->output_offset;
 
-  if (output_bfd != (bfd *) NULL)
+  if (output_bfd != NULL)
     {
       /* Add in supplied addend.  */
       relocation += reloc_entry->addend;
@@ -1332,9 +1325,13 @@  mmix_elf_reloc (bfd *abfd,
 	 Modify the reloc inplace to reflect what we now know.  */
       reloc_entry->addend = relocation;
       reloc_entry->address += input_section->output_offset;
-      return flag;
+      return bfd_reloc_ok;
     }
 
+  if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd,
+				  input_section, reloc_entry->address))
+    return bfd_reloc_outofrange;
+
   return mmix_final_link_relocate (reloc_entry->howto, input_section,
 				   data, reloc_entry->address,
 				   reloc_entry->addend, relocation,
diff --git a/bfd/elf64-s390.c b/bfd/elf64-s390.c
index cbffbc99a78..9150670f0ce 100644
--- a/bfd/elf64-s390.c
+++ b/bfd/elf64-s390.c
@@ -417,7 +417,7 @@  s390_elf_ldisp_reloc (bfd *abfd,
   bfd_vma relocation;
   bfd_vma insn;
 
-  if (output_bfd != (bfd *) NULL
+  if (output_bfd != NULL
       && (symbol->flags & BSF_SECTION_SYM) == 0
       && (! howto->partial_inplace
 	  || reloc_entry->addend == 0))
@@ -428,7 +428,8 @@  s390_elf_ldisp_reloc (bfd *abfd,
   if (output_bfd != NULL)
     return bfd_reloc_continue;
 
-  if (reloc_entry->address > bfd_get_section_limit (abfd, input_section))
+  if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd,
+				  input_section, reloc_entry->address))
     return bfd_reloc_outofrange;
 
   relocation = (symbol->value
diff --git a/bfd/elfxx-sparc.c b/bfd/elfxx-sparc.c
index 14d8fb9986f..641b806f686 100644
--- a/bfd/elfxx-sparc.c
+++ b/bfd/elfxx-sparc.c
@@ -55,7 +55,7 @@  init_insn_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
   bfd_vma relocation;
   reloc_howto_type *howto = reloc_entry->howto;
 
-  if (output_bfd != (bfd *) NULL
+  if (output_bfd != NULL
       && (symbol->flags & BSF_SECTION_SYM) == 0
       && (! howto->partial_inplace
 	  || reloc_entry->addend == 0))
@@ -68,7 +68,8 @@  init_insn_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
   if (output_bfd != NULL)
     return bfd_reloc_continue;
 
-  if (reloc_entry->address > bfd_get_section_limit (abfd, input_section))
+  if (!bfd_reloc_offset_in_range (reloc_entry->howto, abfd,
+				  input_section, reloc_entry->address))
     return bfd_reloc_outofrange;
 
   relocation = (symbol->value