linux-kernel - Re: [PATCH 2/9] staging: fsl-mc: fix device ref counting

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: <20170203103121.GA30656@kroah.com>
Date:   Fri, 3 Feb 2017 11:31:21 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Laurentiu Tudor <laurentiu.tudor@....com>
Cc:     "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "agraf@...e.de" <agraf@...e.de>, "arnd@...db.de" <arnd@...db.de>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Ruxandra Ioana Radulescu <ruxandra.radulescu@....com>,
        Bharat Bhushan <bharat.bhushan@....com>,
        Stuart Yoder <stuart.yoder@....com>,
        Catalin Horghidan <catalin.horghidan@....com>,
        Leo Li <leoyang.li@....com>, Roy Pledge <roy.pledge@....com>
Subject: Re: [PATCH 2/9] staging: fsl-mc: fix device ref counting

On Fri, Feb 03, 2017 at 10:17:53AM +0000, Laurentiu Tudor wrote:
> Hi Greg,
> 
> Thanks for having a look. Comment below.
> 
> On 02/03/2017 11:56 AM, Greg KH wrote:
> > On Wed, Feb 01, 2017 at 05:43:22AM -0600, laurentiu.tudor@....com wrote:
> >> From: Laurentiu Tudor <laurentiu.tudor@....com>
> >>
> >> Drop unneeded get_device() call at device creation
> >> and, as per documentation, drop reference count
> >> after using device_find_child() return.
> >>
> >> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@....com>
> >> ---
> >>   drivers/staging/fsl-mc/bus/dprc-driver.c | 1 +
> >>   drivers/staging/fsl-mc/bus/fsl-mc-bus.c  | 1 -
> >>   2 files changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c
> >> index 4e416d8..e4b0341 100644
> >> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
> >> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
> >> @@ -188,6 +188,7 @@ static void dprc_add_new_devices(struct fsl_mc_device *mc_bus_dev,
> >>   		child_dev = fsl_mc_device_lookup(obj_desc, mc_bus_dev);
> >>   		if (child_dev) {
> >>   			check_plugged_state_change(child_dev, obj_desc);
> >> +			put_device(&child_dev->dev);
> >>   			continue;
> >>   		}
> >>
> >> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> >> index cc20dc4..7c6a43b 100644
> >> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> >> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
> >> @@ -537,7 +537,6 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
> >>   		goto error_cleanup_dev;
> >>   	}
> >>
> >> -	(void)get_device(&mc_dev->dev);
> >
> > This implies that your device reference counting is totally wrong and
> > messed up.  Does this fix anything?  Break anything?  It should do
> > something different now...
> 
> It fixes the refcounting in the sense that I'm now seeing the error
> that i think you were referring to in your previous reviews,
> when we hot unplug a device:
> 
> "Device 'foo.N' does not have a release() function, it is broken and 
> must be fixed."
> 
> See next patch that adds the required callback.
> 
> Regarding this particular get_device(), i have no clue why the
> original author placed it here. I've looked over other bus 
> implementations and didn't see something similar.

Ah, that makes more sense, thanks.  I've now applied this and the next
patch and will wait for you to respin the rest.

thanks,

greg k-h