Re: bug report: use after free bug leading to kernel panic

[Mathias Krause <minipli@googlemail.com>]

On Fri, Oct 31, 2014 at 05:50:42PM +0100, Florian Westphal wrote:
> eric gisse <jowr.pi@gmail.com> wrote:
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 3e8afcc07a76..08a7cbcf2274 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -2643,6 +2643,12 @@ static __always_inline void slab_free(struct kmem_cache *s,
> >  
> >  	slab_free_hook(s, x);
> >  
> > +	if (pax_sanitize_slab && !(s->flags & SLAB_NO_SANITIZE)) {
> > +		memset(x, PAX_MEMORY_SANITIZE_VALUE, s->object_size);
> > +		if (s->ctor)
> > +			s->ctor(x);
> > +	}
> > +
> 
> I am no SLUB expert, but this looks wrong.
> slab_free() is called directly via kmem_cache_free().
> 
> conntrack objects are alloc'd/free'd from a SLAB_DESTROY_BY_RCU cache.
> 
> It is therefore legal to access a conntrack object from another
> CPU even after kmem_cache_free() was invoked on another cpu, provided all
> readers that do so hold rcu_read_lock, and verify that object has not been
> freed yet by issuing appropriate atomic_inc_not_zero calls.
> 
> Therefore, object poisoning will only be safe from rcu callback, after
> accesses are known to be illegal/invalid.

Snap, you're right! I was misreading the following comment in
include/linux/slab.h to allow "free reuse" by the slab allocator as
well, e.g. for sanitizing/poisoning the object:

 * SLAB_DESTROY_BY_RCU - **WARNING** READ THIS!
 *  
 * This delays freeing the SLAB page by a grace period, it does _NOT_
 * delay object freeing. This means that if you do kmem_cache_free()
 * that memory location is free to be reused at any time. Thus it may
 * be possible to see another object there in the same RCU grace period.

But, in fact, that assumption is not true. I now see how the conntrack
code exploits this feature by testing &ct->ct_general.use. So if we
scratch that by writing '\xfe' everywhere over the object, that test
will no longer work.

I guess we need to change the slab sanitization feature in PaX to handle
SLAB_DESTROY_BY_RCU marked slabs the way they need to.

> 
> (not saying that conntrack is bug free..., we had races there in the
>  past).
> 
> From a short glance at SLUB it seems poisoning objects for SLAB_DESTROY_BY_RCU
> caches is safe in __free_slab(), but not earlier.
> 
> If you use different allocator, please tell us which one (check kernel
> config, slub is default).
> 
> If its reproduceable with poisoning done after the RCU grace periods
> have elapsed (i.e., where its not legal anymore to access the memory),
> please let us know and we can have another look at it.
> 

Thanks,
Mathias