[PATCH] Improve talloc security
jra at samba.org
Mon Feb 23 18:30:19 MST 2015
On Tue, Feb 24, 2015 at 01:54:35PM +1300, Andrew Bartlett wrote:
> Attached are a few patches to improve the security of our systems while
> running talloc. I got back onto this after our recent security issue
> where again, talloc was the exploit vector, after the initial error.
> The first patch set finishes the work I did earlier last year to change
> the talloc_magic to be a random value. I found out (via libressl's
> getentropy_linux) that we can ask the Linux kernel for a random number
> without opening /dev/urandom. The AT_RANDOM value appears to have been
> provided since 2008 for ASLR, which is essentially what we are doing.
> This is also ideal, as it is designed exactly for the environment during
> library initialisation.
> (Note, the change makes me think the attempted destructor in
> nsswich/wb_common.c isn't quite right, I don't think that actually works
> on a non-static function, but I've not touched the behaviour).
> The second patch tries to restrict the talloc destructor function to
> functions already enrolled. This is done by recording a whitelist of 0
> and 1 bits that must be present in the functions.
> I use a bitmask so that it is fast to operate (rather than a sorted
> table), yet apparently good enough to tell the difference between
> legitimate functions and (say) system().
> Comments and thoughts most welcome!
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.
I think if we want to do this we need to bite the bullet
and add in the pthread_mutex_t guards around the global
variables and make talloc fully MT-safe.
Just my 2 cents. I do like the idea of protecting against
more talloc-destructor disasters, but I think we've reached
the end of the road w.r.t non-mt talloc.
More information about the samba-technical