netdev - Correct way to access MDIO bus

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-next>] [day] [month] [year] [list]

Message-ID: <20150825000934.GK7557@n2100.arm.linux.org.uk>
Date:	Tue, 25 Aug 2015 01:09:34 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	netdev@...r.kernel.org, David Miller <davem@...emloft.net>,
	Giuseppe CAVALLARO <peppe.cavallaro@...com>,
	Ben Hutchings <bhutchings@...arflare.com>
Subject: Correct way to access MDIO bus - phy.c seems buggy

Hi,

While trying to track down instability in the FEC driver, I've come
across this question: what is the correct way to access the MDIO bus?

Is it via:

	bus->write()

where 'bus' is a struct mii_bus, or should it be via mdiobus_write()?

What I'm seeing in the FEC driver is two thread trying to access the
MDIO bus simultaneously - one thread trying to do a read, and another
trying to do a write.  The result is far from pretty with the current
mainline code, because we can end up re-initialising a spinlock while
it's held by the fec interrupt handler.

I think the correct answer is that mdiobus_write() should be used,
which makes drivers/net/phy/phy.c horribly buggy, as it bypasses the
locking at the mdiobus level by doing this:

mmd_phy_indirect()
{
	bus->write(bus, addr, MII_MMD_CTRL, devad);
	bus->write(bus, addr, MII_MMD_DATA, prtad);
	bus->write(bus, addr, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
}

However, it's not as simple as that, because the whole set of writes
need to be done atomically.  The mdio bus lock needs to be taken around
the internals of phy_read_mmd_indirect() and phy_write_mmd_indirect().

This bug can be provoked by running an ethtool command which accesses
the phy in a tight loop on a SMP platform.  For example:

	while :; do ethtool --show-eee eth0; done

Patch will follow tomorrow.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html