[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <961cf1cfaf05448092e517f4058ea062@huawei.com>
Date: Mon, 18 Feb 2019 17:54:08 +0000
From: Salil Mehta <salil.mehta@...wei.com>
To: John Garry <john.garry@...wei.com>,
Huang Zijiang <huang.zijiang@....com.cn>,
"Zhuangyuzeng (Yisen)" <yisen.zhuang@...wei.com>
CC: "davem@...emloft.net" <davem@...emloft.net>,
"lipeng (Y)" <lipeng321@...wei.com>,
liuyonglong <liuyonglong@...wei.com>,
yuehaibing <yuehaibing@...wei.com>,
"keescook@...omium.org" <keescook@...omium.org>,
"wangxi (M)" <wangxi11@...wei.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"wang.yi59@....com.cn" <wang.yi59@....com.cn>,
Linuxarm <linuxarm@...wei.com>
Subject: RE: [PATCH] net: hns: Fix object reference leaks in
hns_dsaf_roce_reset()
> 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.
Powered by blists - more mailing lists