Re: [BUG] kobject module-ref race-condition or unsafe module_put(THIS_MODULE)

From: David Herrmann
Date: Sat Oct 29 2011 - 19:46:28 EST


On Sun, Oct 30, 2011 at 12:39 AM, Greg KH <gregkh@xxxxxxx> wrote:
> On Sat, Oct 29, 2011 at 07:52:47PM +0200, David Herrmann wrote:
>> Hi
>>
>> I currently do not understand how kobjects keep a reference to the
>> owning module. Lets assume I provide a "release" method via a
>> kobj_type for my kobject. I want to go sure my module is still alive
>> when this method is called. Otherwise, this "release" method would be
>> no longer available and we would jump into invalid memory. Therefore,
>> I need to take a reference to my own module. This seems trivial.
>> However, how do I release this reference again?
>> The most simple solution might be calling module_put(THIS_MODULE) in
>> my "release" method. However, if this call drops the module-refcount
>> to 0 and immediately removes the module, the module_put() returns to
>> my "release" function which now is no longer available.
>>
>> It seems quite unlikely that the cleanup of a module is faster than
>> two function-returns, however, theoretically there is a race
>> condition.
>>
>> The following example is based on fs/char_dev.c. Lets assume my module
>> provides a  kobject structure. On init I take a ref to myself with
>> try_module_get(THIS_MODULE).
>
> No, never do that, why would you?
>
>> I add my own kobj_type with the following release function:
>>
>> static void mydev_put(struct mydev *p)
>> {
>>       if (p) {
>>               kobject_put(&p->kobj);
>>               module_put(THIS_MODULE);
>>       }
>> }
>
> Ick, don't do that.
>
>> How can we go sure that module_put() doesn't free my own module before
>> it returns? Isn't a call to module_put(THIS_MODULE) always unsafe
>> (unless I own at least two references)?
>
> Yes, that's why you shouldn't be doing this :)
>
>> Maybe I have missed some important fact here, but this seems quite
>> unsafe to me. Adding a "owner" field to a kobj_type would fix that
>> issue for kobjects/devices.
>
> No, let's determine exactly what you are trying to do first, and why in
> the world you are dealing with "raw" kobjects.  You should almost never
> never never do that.
>
> What driver are you writing?  Have a pointer to the code somewhere?

Or for instance look at drivers/input/input.c at input_dev_release().
It also calls module_put(THIS_MODULE) and it looks quite unsafe to me.
However, I really have no idea how to do this right in these cases.
I think this is the same as the thing I wanted to describe about hci_dev.

> greg k-h

Regards
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/