Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Salil Mehta <salil.mehta@huawei.com>
To:     John Garry <john.garry@huawei.com>,
        Huang Zijiang <huang.zijiang@zte.com.cn>,
        "Zhuangyuzeng (Yisen)" <yisen.zhuang@huawei.com>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        "lipeng (Y)" <lipeng321@huawei.com>,
        liuyonglong <liuyonglong@huawei.com>,
        yuehaibing <yuehaibing@huawei.com>,
        "keescook@chromium.org" <keescook@chromium.org>,
        "wangxi (M)" <wangxi11@huawei.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "wang.yi59@zte.com.cn" <wang.yi59@zte.com.cn>,
        Linuxarm <linuxarm@huawei.com>
Subject: RE: [PATCH] net: hns: Fix object reference leaks in
 hns_dsaf_roce_reset()
Thread-Topic: [PATCH] net: hns: Fix object reference leaks in
 hns_dsaf_roce_reset()
Thread-Index: AQHUxDBEhCVMlZMv6EeAXB5DT3ZcMKXgsKCAgAAGiyCAAAx1gIAFGOMA
Date:   Mon, 18 Feb 2019 17:54:08 +0000
Message-ID: <961cf1cfaf05448092e517f4058ea062@huawei.com>
References: <1550126505-28394-1-git-send-email-huang.zijiang@zte.com.cn>
 <b20d235c-779c-77bd-e69f-2d63ffd79ec1@huawei.com>
 <11b3bcdf50ee435587fbfad6043e49bc@huawei.com>
 <4ca251e6-22a1-a792-bb64-2498d8cb6fcd@huawei.com>
In-Reply-To: <4ca251e6-22a1-a792-bb64-2498d8cb6fcd@huawei.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> From: John Garry
> Sent: Friday, February 15, 2019 12:00 PM
> 
> On 15/02/2019 11:25, Salil Mehta wrote:
> >> From: John Garry
> >> Sent: Friday, February 15, 2019 10:52 AM
> >>
> >> On 14/02/2019 06:41, Huang Zijiang wrote:
> >>> The of_find_device_by_node() takes a reference to the underlying device
> >>> structure, we should release that reference.
> >>
> >> of_find_device_by_node() is not called for every path, so is this change proper:
> >
> >
> > It looks okay to me and with the suggested device reference is being
> > released from every possible error leg in the function. Could you be
> > more specific which error path is not being addressed?
> >
> >> 	/* find the platform device corresponding to fwnode */
> >> 	if (is_of_node(dsaf_fwnode)) {
> >> 		pdev = of_find_device_by_node(to_of_node(dsaf_fwnode));
> >
> >
> > This will get the reference to the device, which needs to be
> > released later.
> >
> >
> >> 	} else if (is_acpi_device_node(dsaf_fwnode)) {
> >> 		pdev = hns_dsaf_find_platform_device(dsaf_fwnode);
> >
> >
> > This will also get the reference to the device when bus_find_device()
> > gets called and returns the 'device'. Therefore, this needs to be
> > released as well using put_device()
> 
> OK, I see. That's non-obvious.
> 
> And so the commit message was simply incomplete.
> 
> So who finally drops this reference? This patch only seems to release on
> error path. I checked the callers and they don't seem to.


Yes, in the positive leg the put_device() was missing. Thanks for identifying!
I have floated the patch to fix it https://www.lkml.org/lkml/2019/2/18/1161


Salil.