[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