netdev - Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

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: <CANr-f5wCpcPM+FbeW+x-JmZt0-WmE=b5Ys1Pa_G7p8v3nLyCcQ@mail.gmail.com>
Date:   Sun, 12 Sep 2021 22:49:25 +0200
From:   Gerhard Engleder <gerhard@...leder-embedded.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     netdev <netdev@...r.kernel.org>, Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Russell King <linux@...linux.org.uk>,
        Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

> This reverts commit 3ac8eed62596387214869319379c1fcba264d8c6.
>
> I am not actually sure I follow the patch author's logic, because the
> change does more than it says on the box, but this patch breaks
> suspend/resume on NXP LS1028A and probably on any other systems which
> have PHY devices with no driver bound, because the patch has removed the
> "!phydev->drv" check without actually explaining why that is fine.

The wrong assumption was that the driver is set for every device during probe
before suspend. Intention of the patch was only clean up of
to_phy_driver() usage.

>  static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
>  {
> +       struct device_driver *drv = phydev->mdio.dev.driver;
> +       struct phy_driver *phydrv = to_phy_driver(drv);
>         struct net_device *netdev = phydev->attached_dev;
>
> -       if (!phydev->drv->suspend)
> +       if (!drv || !phydrv->suspend)
>                 return false;
>
>         /* PHY not attached? May suspend if the PHY has not already been

I suggest to add the "!phydev->drv" check, but others may know it
better than me.

Gerhard