PR28306, segfault in _bfd_mips_elf_reloc_unshuffle

[Maciej W. Rozycki]

On Wed, 8 Sep 2021, Alan Modra wrote:

> Protect the _bfd_mips_elf_reloc_unshuffle call in mips16_gprel_reloc
> by checking the reloc offset.  The other changes catch potential
> buffer overflows when processing relocations near the end of a
> section.
> 
> OK to apply?

 Thank you for working on this issue.  Overall it looks good to me, but 
see one question below.

> diff --git a/bfd/elfn32-mips.c b/bfd/elfn32-mips.c
> index dc607e776d1..2ab0bae976a 100644
> --- a/bfd/elfn32-mips.c
> +++ b/bfd/elfn32-mips.c
> @@ -877,7 +877,7 @@ static reloc_howto_type elf_mips_howto_table_rela[] =
>    /* No relocation.  */
>    HOWTO (R_MIPS_NONE,		/* type */
>  	 0,			/* rightshift */
> -	 0,			/* size (0 = byte, 1 = short, 2 = long) */
> +	 3,			/* size (0 = byte, 1 = short, 2 = long) */
>  	 0,			/* bitsize */
>  	 false,			/* pc_relative */
>  	 0,			/* bitpos */

 This might well be a separate change, applied right away as obvious.

> diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
> index e4827fd17de..aef5ede3ef0 100644
> --- a/bfd/elfxx-mips.c
> +++ b/bfd/elfxx-mips.c
[...]
> @@ -2595,9 +2598,19 @@ _bfd_mips_elf_generic_reloc (bfd *abfd ATTRIBUTE_UNUSED, arelent *reloc_entry,
>    bfd_reloc_status_type status;
>    bool relocatable;
>  
> +  /* ld -r or gas.  */
>    relocatable = (output_bfd != NULL);
>  
> -  if (reloc_entry->address > bfd_get_section_limit (abfd, input_section))
> +  /* We only use bfd_reloc_offset_in_range for final linking because
> +     mips object files may use relocations that seem to access beyond
> +     section limits.  gas/testsuite/gas/mips/dla-reloc.s is an example
> +     that puts R_MIPS_SUB, a 64-bit relocation, on the last
> +     instruction in the section.  If final linking that object file
> +     the R_MIPS_SUB won't be processed here since it applies to the
> +     addend for the next reloc rather than the section contents.  */
> +  if (!relocatable
> +      && !bfd_reloc_offset_in_range (reloc_entry->howto, abfd,
> +				     input_section, reloc_entry->address))
>      return bfd_reloc_outofrange;

 Would a correct check be feasible here?  For a composed relocation only 
the final entry is applied to output, so could we instead check if there 
is a follow-up relocation?

  Maciej