netdev - Re: [PATCH net] usbnet: smsc95xx: Fix deadlock on runtime resume

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: <20220427153851.GA15329@wunner.de>
Date:   Wed, 27 Apr 2022 17:38:51 +0200
From:   Lukas Wunner <lukas@...ner.de>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Steve Glendinning <steve.glendinning@...well.net>,
        UNGLinuxDriver@...rochip.com, Oliver Neukum <oneukum@...e.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
        linux-usb@...r.kernel.org, Andre Edich <andre.edich@...rochip.com>,
        Oleksij Rempel <o.rempel@...gutronix.de>,
        Martyn Welch <martyn.welch@...labora.com>,
        Gabriel Hojda <ghojda@...urs.ro>,
        Christoph Fritz <chf.fritz@...glemail.com>,
        Lino Sanfilippo <LinoSanfilippo@....de>,
        Philipp Rosenberger <p.rosenberger@...bus.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH net] usbnet: smsc95xx: Fix deadlock on runtime resume

On Wed, Apr 27, 2022 at 05:24:50PM +0200, Andrew Lunn wrote:
> On Wed, Apr 27, 2022 at 08:41:49AM +0200, Lukas Wunner wrote:
> > Commit 05b35e7eb9a1 ("smsc95xx: add phylib support") amended
> > smsc95xx_resume() to call phy_init_hw().  That function waits for the
> > device to runtime resume even though it is placed in the runtime resume
> > path, causing a deadlock.
> 
> You have looked at this code, tried a few different things, so this is
> probably a dumb question.
> 
> Do you actually need to call phy_init_hw()?
> 
> mdio_bus_phy_resume() will call phy_init_hw(). So long as you first
> resume the MAC and then the PHY, shouldn't this just work?

mdio_bus_phy_resume() is only called for system sleep.  But this is about
*runtime* PM.

mdio_bus_phy_pm_ops does not define runtime PM callbacks.  So runtime PM
is disabled on PHYs, no callback is invoked for them when the MAC runtime
suspends, hence the onus is on the MAC to runtime suspend the PHY (which
is a child of the MAC).  Same on runtime resume.

Let's say I enable runtime PM on the PHY and use pm_runtime_force_suspend()
to suspend the PHY from the MAC's ->runtime_suspend callback.  At that
point the MAC already has status RPM_SUSPENDING.  Boom, deadlock.

The runtime PM core lacks the capability to declare that children should
be force runtime suspended before a device can runtime suspend, that's
the problem.

Thanks,

Lukas