[PATCH] Improve talloc security

Jeremy Allison jra at samba.org
Mon Feb 23 19:24:55 MST 2015


On Tue, Feb 24, 2015 at 03:03:34PM +1300, Andrew Bartlett wrote:
> On Mon, 2015-02-23 at 17:30 -0800, Jeremy Allison wrote:
> > 
> > Problem with this I can see is it makes talloc *more* MT-unsafe
> > when used without the TALLOC_FREE_FILL environment variable,
> > and without talloc_enable_null_tracking().
> > 
> > I know it's a small race condition on initialization but
> > eventually all these things add up.
> 
> Indeed, I can see this might be an issue.  libbsd hit the same thing
> here trying to play around with the global environ[] to implement
> setproctitle:
> 
> http://cgit.freedesktop.org/libbsd/commit/?id=c5b959028734ca2281250c85773d9b5e1d259bc8
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=66679
> 
> In short, we can already be in threads when we are in the library init,
> if we are invoke for the first time by dlopen(). 
> 
> But the above is racing against other parts of the system that can see
> and use environ[].  What I don't really see is how we get init() to run
> twice, because dlopen() will only load a library once (otherwise we
> would be in bigger trouble).  
> 
> In any case, dropping the rand() patch would avoid that part of the
> issue, as it would be deterministic.  I still don't see the race, but I
> understand why it raised red flags for you.

Never mind, I initially missed we're in the contructor
when that gets run (obviously completely ignored the
first patch which explicitly checks the platform supports
running constructors :-) which should mean we're safe there.

In fact it might be better to move the TALLOC_FREE_FILL
code also into a library constructor (if we're adding
one) in order to remove one of the races we already
have with MT-code.

Cheers,

	Jeremy.


More information about the samba-technical mailing list