netdev - Re: [PATCH] staging: octeon-ethernet: trivial: Avoid OOPS if phydev is not set

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] [day] [month] [year] [list]

Message-ID: <20131015094750.GA11731@mwanda>
Date:	Tue, 15 Oct 2013 12:47:50 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Aaro Koskinen <aaro.koskinen@....fi>
Cc:	support@...iumnetworks.com, David Daney <david.daney@...ium.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	driverdev-devel@...uxdriverproject.org,
	Sebastian Pöhn <sebastian.poehn@...il.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH] staging: octeon-ethernet: trivial: Avoid OOPS if phydev
 is not set

On Mon, Oct 14, 2013 at 10:49:37PM +0300, Aaro Koskinen wrote:
> On Mon, Oct 14, 2013 at 10:16:49PM +0300, Dan Carpenter wrote:
> > On Mon, Oct 14, 2013 at 09:39:06PM +0300, Aaro Koskinen wrote:
> > > It's initialized in cvm_oct_phy_setup_device():
> > > 
> > > 	priv->phydev = of_phy_connect(dev, phy_node, cvm_oct_adjust_link, 0,
> >       ^^^^^^^^^^^^                                 ^^^^^^^^^^^^^^^^^^^
> > 
> > Sorry I should have explained better.
> > 
> > We use cvm_oct_adjust_link() to initialize priv->phydev but
> > cvm_oct_adjust_link() depends on priv->phydev.  It seems like we would
> > hit the NULL dereference every time.  Weird huh?
> 
> It doesn't happen on my system (EdgeRouter Lite). I think you need to
> explain even more better. :-)
> 
> What you mean by "We use cvm_oct_adjust_link() to initialize
> priv->phydev..."? Sorry, maybe I'm just missing something really
> obvious...

Hm...  I didn't followed the state machine all the way through so I'm
not actually sure how this is called.  But it's weird that priv->phydev
is initialized on the left side of the assignment but dereferenced in
the function mentioned on the right side if the assignment.

Also this patch should not be marked as "trivial" in the subject.
"trivial" is only for spelling mistakes in comments etc.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html