linux-kernel - Re: [PATCH] net: hns: use put_device() if device

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] [day] [month] [year] [list]

Message-ID: <5AA73E06.1050604@gmail.com>
Date:   Tue, 13 Mar 2018 08:27:10 +0530
From:   arvindY <arvind.yadav.cs@...il.com>
To:     Richard Weinberger <richard.weinberger@...il.com>
Cc:     David Miller <davem@...emloft.net>, yisen.zhuang@...wei.com,
        salil.mehta@...wei.com, linyunsheng@...wei.com,
        LKML <linux-kernel@...r.kernel.org>, netdev@...r.kernel.org,
        linux-mtd@...ts.infradead.org
Subject: Re: [PATCH] net: hns: use put_device() if device_register fail



On Monday 12 March 2018 10:59 PM, Richard Weinberger wrote:
> On Mon, Mar 12, 2018 at 5:27 PM, arvindY <arvind.yadav.cs@...il.com> wrote:
>>
>> On Monday 12 March 2018 08:13 PM, David Miller wrote:
>>> From: Arvind Yadav <arvind.yadav.cs@...il.com>
>>> Date: Fri,  9 Mar 2018 16:11:17 +0530
>>>
>>>> if device_register() returned an error! Always use put_device()
>>>> to give up the reference initialized.
>>>>
>>>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@...il.com>
>>> I do not see anything giving cls_dev an initial non-zero reference
>>> count before this device_register() call.
>> Yes,  you are correct there is nothing to release (hnae_release).
> Wait, this is not what DaveM said.
> Since you sent also patches to MTD I care about this too.
>
> Do we have to call put_device() in any case after a failure of
> device_register() or not?
> In this case the release function is empty, so nothing is going to released.
> But technically a put_device() is needed and correct according to your
> change log.
>
> I have to admit I don't know all details of the driver core, maybe you
> can help me.
> device_register() calls device_initialize() followed by device_add().
> device_initialize() does a kobject_init() which again sets the
> reference counter to 1 via kref_init().
> In the next step device_add() does a get_device() --> reference
> counter goes up to 2.
> But in the exit path of the function a put_device() is done, also in
> case of an error.
> So device_register() always returns with reference count 1.
>
> If I understand this correctly a put_device() is mandatory.
> Also in this driver.
> Can you please enlighten me? :-)
>
Oh, I just miss understood david question.

But what ever you are telling that is correct.
device_initialize() will call kref_init() which is setting
refcount 1 by refcount_set(&kref->refcount, 1).
and device_add() will call kref_get() to increment refcount
by calling refcount_inc(&kref->refcount).
So we need put_device() to decrement and release the
kboject.
Internally it'll call kref_put() -> refcount_dec_and_test(&kref->refcount)
to decrement refcount.

~arvind