linux-kernel - Re: [BUG] kobject module-ref race-condition or unsafe module_put(THIS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANq1E4QDmffb_kWycA+HM=1qQPC8Xm5Ft2ZP-p7JJ+U+e7X=oA@mail.gmail.com>
Date:	Sun, 30 Oct 2011 01:46:13 +0200
From:	David Herrmann <dh.herrmann@...glemail.com>
To:	Greg KH <gregkh@...e.de>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [BUG] kobject module-ref race-condition or unsafe module_put(THIS_MODULE)

On Sun, Oct 30, 2011 at 12:39 AM, Greg KH <gregkh@...e.de> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/