Re: [RFC PATCH] net: phy/mdio: enable mmd indirect access through phy_mii_ioctl()

On Thu, Nov 04, 2021 at 12:35:17PM +0000, Russell King (Oracle) wrote:

On Thu, Nov 04, 2021 at 12:17:47PM +0100, Tobias Waldekranz wrote:

quoted

Except that there is a way: https://github.com/wkz/mdio-tools

I'm guessing that this hasn't had much in the way of review, as it has
a nice exploitable bug - you really want "pc" to be unsigned in
mdio_nl_eval(), otherwise one can write a branch instruction that makes
"pc" negative.

Also it looks like one can easily exploit this to trigger any of your
BUG_ON()/BUG() statements, thereby crashing while holding the MDIO bus
lock causing a denial of service attack.

I also see nothing that protects against any user on a system being
able to use this interface, so the exploits above can be triggered by
any user. Moreover, this lack of protection means any user on the
system can use this interface to write to a PHY.

Given that some PHYs today contain firmware, this gives anyone access
to reprogram the PHY firmware, possibly introducing malicious firmware.

I hope no one is using this module in a production environment.

It also leaks the reference count on the MDIO bus class device.
mdio_find_bus(), rather class_find_device_by_name() takes a reference
on the struct device that you never drop. See the documentation for
class_find_device() for the statement about this:

 * Note, you will need to drop the reference with put_device() after use.

Of course, mdio_find_bus() documentation should _really_ have mentioned
this fact too.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!