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  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:   Fri, 10 May 2019 22:28:06 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Vicente Bergas <vicencb@...il.com>,
        Serge Semin <fancer.lancer@...il.com>,
        Russell King <rmk+kernel@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: net: phy: realtek: regression, kernel null pointer dereference

On 10.05.2019 17:05, Vicente Bergas wrote:
> Hello,
> there is a regression on linux v5.1-9573-gb970afcfcabd with a kernel null
> pointer dereference.
> The issue is the commit f81dadbcf7fd067baf184b63c179fc392bdb226e
>  net: phy: realtek: Add rtl8211e rx/tx delays config
> which uncovered a bug in phy-core when attempting to call
>  phydev->drv->read_page
> which can be null.
> The patch to drivers/net/phy/phy-core.c below fixes the kernel null pointer
> dereference. After applying the patch, there is still no network. I have
> also tested the patch to drivers/net/phy/realtek.c, but no success. The
> system hangs forever while initializing eth0.
> 
> Any suggestions?
> 
The page operation callbacks are missing in the RTL8211E driver.
I just submitted a fix adding these callbacks to few Realtek PHY drivers
including RTl8211E. This should fix the issue.
Nevertheless your proposed patch looks good to me, just one small change
would be needed and it should be splitted.

The change to phy-core I would consider a fix and it should be fine to
submit it to net (net-next is closed currently).

Adding the warning to the Realtek driver is fine, but this would be
something for net-next once it's open again.

> Regards,
>  Vicenç.
> 
Heiner

> --- a/drivers/net/phy/phy-core.c
> +++ b/drivers/net/phy/phy-core.c
> @@ -648,11 +648,17 @@
> 
> static int __phy_read_page(struct phy_device *phydev)
> {
> +    if (!phydev->drv->read_page)
> +        return -EOPNOTSUPP;
> +   
>     return phydev->drv->read_page(phydev);
> }
> 
> static int __phy_write_page(struct phy_device *phydev, int page)
> {
> +    if (!phydev->drv->write_page)
> +        return -EOPNOTSUPP;
> +
>     return phydev->drv->write_page(phydev, page);
> }
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -214,8 +214,10 @@
>      * for details).
>      */
>     oldpage = phy_select_page(phydev, 0x7);
> -    if (oldpage < 0)
> -        goto err_restore_page;
> +    if (oldpage < 0) {
> +        dev_warn(&phydev->mdio.dev, "Unable to set rgmii delays\n");

Here phydev_warn() should be used.

> +        return 0;
> +    }
> 
>     ret = phy_write(phydev, RTL821x_EXT_PAGE_SELECT, 0xa4);
>     if (ret)
> 
> 

Powered by blists - more mailing lists