netdev - Re: net: phy: realtek: regression, kernel null pointer dereference

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]

Date:   Mon, 13 May 2019 15:42:26 +0300
From:   Serge Semin <fancer.lancer@...il.com>
To:     Vicente Bergas <vicencb@...il.com>
Cc:     Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <rmk+kernel@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: net: phy: realtek: regression, kernel null pointer dereference

Hello,

On Mon, May 13, 2019 at 02:19:17PM +0200, Vicente Bergas wrote:
> On Monday, May 13, 2019 12:51:05 PM CEST, Serge Semin wrote:
> > On Mon, May 13, 2019 at 01:29:42PM +0300, Serge Semin wrote:
> > > Hello Vincente,
> > > 
> > > On Sat, May 11, 2019 at 05:06:04PM +0200, Vicente Bergas wrote: ...
> > 
> > Hmm, just figured out, that in the datasheet RXDLY/TXDLY pins are
> > actually grounded, so
> > phy-mode="rgmii" should work for you. Are you sure that on your actual
> > hardware the
> > resistors aren't placed differently?
> 
> That is correct, the schematic has pull-down resistors and placeholders for
> pull-up resistors. On the board I can confirm the pull-ups are not
> populated and the pull-downs are.
> But the issue seems unrelated to this.
> 
> I have traced it down to a deadlock while trying to acquire a mutex.
> It hangs here:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/realtek.c?id=47782361aca2#n220
> while waiting for this mutex:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/mdio_bus.c?id=47782361aca2#n692
> 
> Regards,
>  Vicenç.
> 

Ahh, I see. Then using lock-less version of the access methods must fix the
problem. You could try something like this:
-------------------------------------------------------------------------------
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 761ce3b1e7bd..14b61da1f32a 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -217,12 +217,12 @@ static int rtl8211e_config_init(struct phy_device *phydev)
 	if (oldpage < 0)
 		goto err_restore_page;
 
-	ret = phy_write(phydev, RTL821x_EXT_PAGE_SELECT, 0xa4);
+	ret = __phy_write(phydev, RTL821x_EXT_PAGE_SELECT, 0xa4);
 	if (ret)
 		goto err_restore_page;
 
-	ret = phy_modify(phydev, 0x1c, RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
-			 val);
+	ret = __phy_modify(phydev, 0x1c, RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
+			   val);
 
 err_restore_page:
 	return phy_restore_page(phydev, oldpage, ret);
-------------------------------------------------------------------------------

-Sergey

> > The current config register state can be read from the 0x1c extension
> > page. Something
> > like this:
> > --- a/drivers/net/phy/realtek.c
> > +++ b/drivers/net/phy/realtek.c
> > @@ -221,6 +221,9 @@ static int rtl8211e_config_init(struct phy_device
> > *phydev)
> >  	if (ret)
> >  		goto err_restore_page;
> > +	ret = phy_read(phydev, 0x1c);
> > +	dev_info(&phydev->mdio.dev, "PHY config register %08x\n", ret);
> > +
> >  	ret = phy_modify(phydev, 0x1c, RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
> >  			 val);
> > -Sergey
>