netdev - Re: Cutting the link on ndo_stop - phy_stop or phy

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:   Wed, 5 Jun 2019 13:16:31 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Ioana Ciornei <ioana.ciornei@....com>
Subject: Re: Cutting the link on ndo_stop - phy_stop or phy_disconnect?

On Wed, Jun 05, 2019 at 02:46:13AM +0300, Vladimir Oltean wrote:
> On Wed, 5 Jun 2019 at 02:24, Russell King - ARM Linux admin
> <linux@...linux.org.uk> wrote:
> > No - but that has nothing to do with phylib internals, more to do with
> > the higher levels of networking.  ndo_stop() will not be called unless
> > ndo_open() has already been called.  In other words, setting an already
> > down device down via "ip link set dev eth0 down" is a no-op.
> >
> > So, let's a common scenario.  You power up a board.  The PHY comes up
> > and establishes a link.  The boot loader runs, loads the kernel, which
> > then boots.  Your network driver is a module, and hasn't been loaded
> > yet.  The link is still up.
> >
> > The modular network driver gets loaded, and initialises.  Userspace
> > does not bring the network device up, and the network driver does not
> > attach or connect to the PHY (which is actually quite common).  So,
> > the link is still up.
> >
> > The modular PHY driver gets loaded, and binds to the PHY.  The link
> > is still up.
> 
> I would rather say, 'even if the link is not up, Linux brings it up
> (possibly prematurely) via phy_resume'.
> But let's consider the case where the link *was* up. The general idea
> is 'implement your workarounds in whatever other way, that link is
> welcome!'.

I think you've missed some of the nuances about my example scenario.

If your MAC driver expects the MII pins to be silent after it probes,
this will not be the case in the scenario that I've given you.  The
PHY won't be silenced here, even with your proposed changes.

> > Userspace configures the network interface, which causes the PHY
> > device to be attached to the network device, and phy_start() to be
> > called on it - the negotiation advertisement is configured, and
> > negotiation restarted if necessary.

This is where your suggested modifications first take effect.

What I'm stating is that if you write your network driver to require
that the PHY link is down after the network driver is probed but before
ndo_open is called, in the above exact scenario, that will not be the
case and your network driver may malfunction.

Having the kernel rely on a certain boot loader behaviour is very bad.

You also have to consider that the previous context to the kernel
booting may _not_ be the boot loader - for example, if the kernel
supports crash dump kexec, then the previous context to the crash
kernel is the kernel which crashed, which may well have established a
link on the network interface.

So, relying on the state of the hardware from the boot loader is a
recipe for a buggy driver.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up