Protect mips_hi16_list from fuzzed debug info

[Alan Modra]

On Tue, Feb 07, 2023 at 02:11:23PM +0000, Maciej W. Rozycki wrote:
> On Mon, 6 Feb 2023, Alan Modra wrote:
> 
> > This is another fix for the testcase mentioned in
> > https://sourceware.org/pipermail/binutils/2023-February/125915.html
> > either of which will stop the addr2line segfault.  This one also fixes
> > a potential problem when linking corrupted debug info.
> 
>  Hmm, from the other message I gather DWARF info is not going to be 
> processed twice anymore, so why is this change to the MIPS backend also 
> required?

See below.

>  Also would it be possible to have a MIPS test case for your change?  
> Orchestrating HI16/LO16 relocations in a debug section should be pretty 
> straightforward with the use of the `.reloc' pseudo-op.  This might help 
> me understand what is really going on here.

I could do that, but my time is limited for mips problems.  I'll
understand if you say the patch is not worth committing just to cover
a potential fuzzed object file segfault.

>  Also one concern about code proposed itself, see below.
> 
> > @@ -2596,38 +2600,41 @@ _bfd_mips_elf_lo16_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
> [...]
> > +  if (!tdata->freeze_mips_hi16_list)
> 
>  This conditional ought to wrap all the preceding code in the function as 
> well (including the declaration block), because it's sole purpose is to 
> retrieve `vallo', which is only used within the `while' loop now placed 
> under the conditional...

OK, done.  I'm presuming I don't need to repost the patch.

> > +  /* Debug info should not contain hi16 or lo16 relocs.  If it does
> > +     then someone is playing fuzzing games.  Altering the hi16 list
> > +     during linking when printing an error message is bad.  */

s/an error/a warning/

>  And I really cannot extract the meaning of the second sentence here.  I 
> mean I know what it literally means, but that does not really tell me 
> anything.  Why would altering the list be a problem given that we're 
> bailing out anyway?  I'm confused.

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.  Is this likely
to happen in the real world?  No, of course not, but fuzzers keep
finding this sort of thing, and the occasional real problem found by
the fuzzers is enough that I haven't yet decided to ignore all fuzzing
reports.

-- 
Alan Modra
Australia Development Lab, IBM