Home Original page

Protect mips_hi16_list from fuzzed debug info

Alan Modra amodra@gmail.com
Thu Feb 9 10:14:08 GMT 2023 
On Thu, Feb 09, 2023 at 01:26:11AM +0000, Maciej W. Rozycki wrote:
> On Thu, 9 Feb 2023, Alan Modra wrote:
> > If it is really true that hi16/lo16 pairs are always in the same
> > section, then we wouldn't need freeze_mips_hi16_list.
> 
>  It is, by definition[1]:
> 
> "The AHL addend is a composite computed from the addends of two 
> consecutive relocation entries.  Each relocation type of R_MIPS_HI16 must 
> have an associated R_MIPS_LO16 entry immediately following it in the list 
> of relocations.
> 
> "These relocation entries are always processed as a pair and both addend 
> fields contribute to the AHL addend.  If AHI and ALO are the addends from 
> the paired R_MIPS_HI16 and R_MIPS_LO16 entries, then the addend AHL is com 
> puted as (AHI << 16) + (short)ALO.  R_MIPS_LO16 entries without an 
> R_MIPS_HI16 entry immediately preceding are orphaned and the previously 
> defined R_MIPS_HI16 is used for computing the addend."
> 
> Two consecutive entries must come from a single .rel.* section or they 
> wouldn't be consecutive (there's no relationship between different .rel.* 
> sections).

Yes, but see the comment before _bfd_mips_elf_hi16_reloc

   The ABI requires that the *LO16 immediately follow the *HI16.
   However, as a GNU extension, we permit an arbitrary number of
   *HI16s to be associated with a single *LO16.  This significantly
   simplies the relocation handling in gcc.

>  While not explicitly stated I don't think anyone considered reusing an 
> R_MIPS_HI16 reloc defined in another relocation section for an orphaned 
> R_MIPS_LO16 reloc.  That would IMO make no semantic sense (for instance 
> you can shuffle sections via a linker script in a relocatable link) and I 
> think we ought not strive for doing so (i.e. there is not supposed to be 
> any "previously defined R_MIPS_HI16" at the beginning of any given reloc 
> section).
> 
>  Of course this consideration applies to the REL format only (i.e. o32); 
> there's no need to track HI16/LO16 pairing with RELA objects as the addend 
> is readily available.  AFAICT we don't get this right either: for 
> `!partial_inplace' howtos we need not use `_bfd_mips_elf_hi16_reloc' or 
> `_bfd_mips_elf_lo16_reloc' and just `_bfd_mips_elf_generic_reloc' will do.
> 
> >  Instead it
> > would be much better to attach the list to mips_elf_section_data.  I
> > wasn't sure enough to do that, given things like gcc's hot/cold
> > section partitioning, when I moved the mip_hi16_list to be per-bfd.
> 
>  That sounds like a plan to me, but it's unrealistic for me to commit to 
> it in the next two days (and then I head out to California for a week).

Patch attached.  Note the FIXME.

> References:
> 
> [1] "SYSTEM V APPLICATION BINARY INTERFACE, MIPS RISC Processor
>     Supplement, 3rd Edition", Section "Relocation", pp. 4-17, 4-18

-- 
Alan Modra
Australia Development Lab, IBM
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Move-mips_hi16_list-to-mips_elf_section_data.patch
Type: text/x-diff
Size: 6947 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20230209/e1c9d42e/attachment.bin>
More information about the Binutils mailing list