[PATCH][WIP] Make exploiting talloc harder by using a random talloc_magic

[Simo]

On Thu, 2013-10-17 at 16:38 +1300, Andrew Bartlett wrote:
> On Wed, 2013-10-16 at 22:32 -0400, Simo wrote:
> > On Thu, 2013-10-17 at 14:34 +1300, Andrew Bartlett wrote:
> > > What do folks think, and can I get some help to prove it would disrupts
> > > these exploits?
> > 
> > Well it may make this specific exploit harder, but if they can't use
> > talloc() they'll probably find something else.
> 
> They might, but the same could be said about -fstack-protector',
> FORTIFY_SOURCE etc.  We have a lot of talloc destructors around, and
> while lots of other plugin points exist, none exist so predictably in
> every memory structure. 

I am in favor additional protection do not mistake me.

> > Ok coming to the code I am not completely positive your change would
> > prevent the attack in all cases, you move flags on top, but nothing
> > prevents an optimizing compiler to shuffle them around the structure in
> > theory.
> 
> In theory, perhaps.  Have you ever seen that happen in practice?  Given
> how much the 'unsafe' trick of reading via different union arms is done
> in practice (and is depended on in Samba), I suggest that this would be
> the least of our worries. 

No, I have not, and it is unlikely, but perhaps we want to use some
pragma directive to make sure ?

> > It is unclear to me why you sprinkle talloc_set_magic() around, why
> > isn't it sufficient to just call it only from __talloc_with_prefix() ?
> 
> It is, and a revised patch is attached.  I was originally trying not to
> need to add the initialisation to that critical codepath.  In the long
> term, running this from some other point may be better.  Is there a hook
> we can declare to be run on shared library loading?

I forgot the exact semantics with libraries but you can look at
__attribute__((constructor)), some info here:
http://gcc.gnu.org/onlinedocs/gcc-4.1.1/gcc/Function-Attributes.html

A function declared constructor should be run at library load time or
before main() execution.

However I think in this case you have to be careful as it is not clear
to me that it is ok to use rand()/srand() at this time. I wonder if
glibc does any initialization there and running in a constructor could
cause issue. It's unlikely but something to watch for.

HTH,
Simo.