[dm-devel] kobject: provide kobject_put_wait to fix module unload race

[Mikulas Patocka]

On Tue, 7 Jan 2014, Mike Snitzer wrote:

> On Tue, Jan 07 2014 at  1:00pm -0500,
> Mikulas Patocka <mpatocka at redhat.com> wrote:
> 
> > 
> > 
> > On Tue, 7 Jan 2014, Linus Torvalds wrote:
> > 
> > > This looks completely broken to me. You do a "kobject_put()" and then
> > > after you've dropped that last use, you wait for the completion of
> > > something that may already have been free'd.
> > > 
> > > Wtf? Am I missing something?
> > > 
> > >                Linus
> > 
> > It is correct. The release method dm_kobject_release doesn't free the 
> > kobject. It just signals the completion and returns.
> > 
> > This is the sequence of operations:
> > * call kobject_put
> > * wait until all users stop using the kobject, when it happens the release 
> >   method is called
> > * the release method signals the completion and returns
> > * the unload code that waits on the completion continues
> > * the unload code frees the mapped_device structure that contains the 
> >   kobject
> > 
> > Using kobject this way avoids the module unload race that was mentioned at 
> > the beginning of this thread.
> 
> I've staged your patch in linux-next for 3.14, see:
> http://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=af7b1e5c767fc895788c971c8f4686402ac8344f

Looking at this patch, I realize that it is buggy too. If module unload 
happens at this point (after the completion is signaled, but before the 
release function returns), it crashes.

static void dm_kobject_release(struct kobject *kobj)
{
	complete(dm_get_completion_from_kobject(kobj));
	>========== module unload here ===============<
}

The patch that I sent initially in this thread doesn't have this bug 
because the completion is signaled in non-module code.

That goes back to my initial claim - it is impossible to use the kobject 
interface correctly! But if Greg doesn't want a patch that fixes the 
kobject interface, I don't really know what to do about it.

Maybe another possibility - maintain a list of all kobjects and walk them 
in the generic module unload code to check if their release method ponits 
to the module that is being unloaded? Greg, would you accept a patch like 
this?

Mikulas