Re: [patch] fix uninitialized variable in dynamic linker

[Sandra Loosemore <sandra at codesourcery dot com>]

On 04/13/2015 04:29 PM, Matthew Fortune wrote:
> Sandra Loosemore <sandra@codesourcery.com> writes:
> > When we were testing glibc 2.21 here, we were getting some mysterious
> > dynamic linking failures on MIPS due to the recent-ish changes for that
> > target to ignore objects with incompatible FP ABIs.  On further
>
> Thanks for looking at this, sorry to not find it during my testing of
> the fpabi work.
>
> > investigation, it turned out that the dynamic linker's own entry in the
> > link map had an invalid l_mach.fpabi value.  I tracked this down to
> > failure to initialize that field in the stack-allocated bootstrap_map
> > structure in _dl_start; _dl_start_final was then happily copying the
> > uninitialized l_mach value into the real map data structure.
>
> I'm quite confused as to why I never saw this during test, I had over
> 70000 tests run correctly and I'd have thought I would see this at least
> once.

If you run the dynamic linker in the usual way in a fresh process, it 
gets a fresh stack that is almost certainly still zero-initialized when 
it gets to this code.

But....  we have a utility that wraps the dynamic linker startup with 
some magic stuff to override the DT_RPATH, etc in the executable.  So in 
this case the dynamic linker isn't starting with a clean stack.

> > The attached patch zero-initializes the entire bootstrap_map data
> > structure.  I thought this was better than selectively initializing
> > specific fields since it future-proofs the code against similar errors
> > involving other fields that might be added (or used) in the future.
>
> This sounds good.

Even though we ran into this in an atypical scenario, explicitly 
initializing stack-allocated data structures seems like the best 
solution for robustness.

> > I've verified that this fixes the 2.21 MIPS problems we saw.  Is this
> > patch OK for mainline head, or is further testing required?
>
> I can't say OK as I don't understand this area of the code well enough
> but can at least confirm that the code I added relies on the fpabi
> field being zero initialised which looked like it held true.
>
> Thanks,
> Matthew

-Sandra