Re: PR22245, Fix potential UB in bfd_set_error

[Alan Modra <amodra at gmail dot com>]

On Wed, Oct 04, 2017 at 03:00:47PM +0100, Pedro Alves wrote:
> Hi Alan,
> 
> On 10/04/2017 04:59 AM, Alan Modra wrote:
> 
> >  
> > -void bfd_set_error (bfd_error_type error_tag, ...);
> > +void bfd_set_error (int error_tag, ...);
> 
> A downside here is that this can now silently accept invalid
> errors if/when someone passes the a value of the wrong
> enumeration type, which previously would be caught by the
> new -Wenum-conversion warning.
> 
> AFAICS, the only reason bfd_set_error is a variadic function is
> to handle bfd_error_on_input.
> 
> I'd suggest instead to move the bfd_error_on_input functionality
> to separate function, and make bfd_set_error non-variadic.
> 
> I gave it a quick try (only tested ld on x86_64 GNU/Linux), and
> that exercise even caught an invalid use
> of "bfd_set_error (bfd_error_on_input)" in elflink.c:
> 
>  --- c/bfd/elflink.c
>  +++ w/bfd/elflink.c
>  @@ -10444,7 +10444,7 @@ elf_link_input_bfd (struct elf_final_link_info  *flinfo, bfd *input_bfd)
>                      (_("error: %B: size of section %A is not "
>                        "multiple of address size"),
>                      input_bfd, o);
>  -                 bfd_set_error (bfd_error_on_input);
>  +                 bfd_set_input_error (input_bfd, bfd_get_error ());
>                    return FALSE;
> 

This one is just using the wrong enum bfd_error.

> That's broken currently because bfd_set_error is going to
> access random stack/registers as second and third arguments
> and setting input_bfd and input_error from that.
> 
> I pushed this to the users/palves/bfd_set_input_error
> branch in case you'd like to run with it.

I'll take the patch minus the elflink.c change, and even write a
ChangeLog.  Thanks!

-- 
Alan Modra
Australia Development Lab, IBM