Thread (29 messages) 29 messages, 7 authors, 2013-12-19

Re: netfilter: active obj WARN when cleaning up

From: Greg KH <hidden>
Date: 2013-11-27 23:34:20
Also in: linux-mm, lkml, netfilter-devel

On Wed, Nov 27, 2013 at 02:44:58PM +0100, Thomas Gleixner wrote:
On Wed, 27 Nov 2013, Russell King - ARM Linux wrote:
quoted
On Wed, Nov 27, 2013 at 01:32:31PM +0000, Russell King - ARM Linux wrote:
quoted
On Wed, Nov 27, 2013 at 02:29:41PM +0100, Thomas Gleixner wrote:
quoted
Though the kobject is the only thing which has a delayed work embedded
inside struct kmem_cache. And the debug object splat points at the
kmem_cache_free() of the struct kmem_cache itself. That's why I
assumed the wreckage around that place. And indeed:

kmem_cache_destroy(s)
    __kmem_cache_shutdown(s)
      sysfs_slab_remove(s)
        ....
	kobject_put(&s->kobj)
           kref_put(&kobj->kref, kobject_release);
	     kobject_release(kref)
    	       #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
	         schedule_delayed_work(&kobj->release)
	       #else
	        kobject_cleanup(kobj)
	       #endif

So in the CONFIG_DEBUG_KOBJECT_RELEASE=y case, schedule_delayed_work()
_IS_ called which arms the timer. debugobjects catches the attempt to
free struct kmem_cache which contains the armed timer.
You fail to show where the free is in the above path.
Right, there's a kmem_cache_free(kmem_cache, s); by the slob code
after the above sequence, which is The Bug(tm).

As I said, a kobject has its own lifetime.  If you embed that into
another structure, that structure inherits the lifetime of the kobject,
which is from the point at which it's created to the point at which the
kobject's release function is called.

So no, the code here is buggy.  The kobject debugging has yet again
found a violation of the kobject lifetime rules.  slub needs fixing.
I leave that discussion to you, greg and the slub folks.
/me grabs some popcorn from tglx

It's really not that much of a discussion, Documentation/kobject.txt has
said this for years, it's as if no one even reads documentation
anymore...

If you embed a kobject into a structure, you have to use the kobject for
the reference counting of the structure, otherwise it's a bug.  If you
don't want to use a kobject to reference count the structure, don't
embed it into it, use a pointer.

Are "slabs" never freed in the slub allocator?  Surely someone should
have seen the huge "this kobject doesn't have a release function" error
message that the kernel should have spit out for it?

Just make the kobject "dynamic" instead of embedded in struct kmem_cache
and all will be fine.  I can't believe this code has been broken for
this long.

thanks,

greg k-h
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help