linux-kernel - Re: [PATCHv14 2/3] usb: USB Type-C connector class

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: <20170105164048.GA30628@kroah.com>
Date:   Thu, 5 Jan 2017 17:40:48 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Oliver Neukum <oneukum@...e.com>, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org
Subject: Re: [PATCHv14 2/3] usb: USB Type-C connector class

On Thu, Jan 05, 2017 at 05:54:02PM +0200, Mika Westerberg wrote:
> > +
> > +	ret = device_register(&alt->dev);
> > +	if (ret) {
> > +		int i;
> > +
> > +		dev_err(parent, "failed to register alternate mode (%d)\n",
> > +			ret);
> > +
> > +		put_device(&alt->dev);
> > +
> > +		for (i = 0; i < alt->n_modes; i++)
> > +			kfree(alt->modes[i].desc);
> > +		kfree(alt);
> 
> Just checking: ->release() is not called when device_register() fails
> and that's why you release memory here?

If so, that's wrong.  Please see the very good documentation for
device_register in the kernel itself, which says:

	* NOTE: _Never_ directly free @dev after calling this function, even
	* if it returned an error! Always use put_device() to give up the
	* reference initialized in this function instead.

Please read the rest of the documentation there as well, it should
answer all of these types of questions...

Sometimes I wonder why I even wrote that stuff if no one ever reads
it...

Heikki, did you get others at Intel to review this?  I don't see their
signed-off-by: on the patches.  Please use the internal Intel kernel
developer mailing list for this type of thing, I thought I said I was
going to require that before I would accept these patches after the last
round of review I did.

I'm not going to look at these until you do that, sorry.  You have
access to good resources, please use them and don't abuse the community
reviewers for basic things like this.

thanks,

greg k-h