linux-kernel - RE: [PATCH] net: hns: Fix object reference leaks in hns_dsaf_roce

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: <11b3bcdf50ee435587fbfad6043e49bc@huawei.com>
Date:   Fri, 15 Feb 2019 11:25:18 +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 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()



> 	} else {
> 		pr_err("fwnode is neither OF or ACPI type\n");
> 		return -EINVAL;
> 	}
> 
> 	/* check if we were a success in fetching pdev */
> 	if (!pdev) {
> 		pr_err("couldn't find platform device for node\n");
> 		return -ENODEV;
> 	}
> 
> 	/* retrieve the dsaf_device from the driver data */
> 	dsaf_dev = dev_get_drvdata(&pdev->dev);
> 	if (!dsaf_dev) {
> 		dev_err(&pdev->dev, "dsaf_dev is NULL\n");
> 		return -ENODEV;
> 	}
> 
> John


[...]

> > --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
> > +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
> > @@ -3081,6 +3081,7 @@ int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool dereset)
> >  	dsaf_dev = dev_get_drvdata(&pdev->dev);
> >  	if (!dsaf_dev) {
> >  		dev_err(&pdev->dev, "dsaf_dev is NULL\n");
> > +		put_device(&pdev->dev);


This looks okay.


> >  		return -ENODEV;
> >  	}
> >
> > @@ -3088,6 +3089,7 @@ int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool dereset)
> >  	if (AE_IS_VER1(dsaf_dev->dsaf_ver)) {
> >  		dev_err(dsaf_dev->dev, "%s v1 chip doesn't support RoCE!\n",
> >  			dsaf_dev->ae_dev.name);
> > +		put_device(&pdev->dev);


This looks okay.


Salil.