netdev - Re: [PATCH net-next v5 1/5] rust: core abstractions for network PHY drivers

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: <eb59a6ce-48f3-4fcc-87cd-4ec4b948617d@lunn.ch>
Date: Sat, 21 Oct 2023 17:35:28 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Benno Lossin <benno.lossin@...ton.me>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org,
	rust-for-linux@...r.kernel.org, miguel.ojeda.sandonis@...il.com,
	tmgross@...ch.edu, boqun.feng@...il.com, wedsonaf@...il.com,
	greg@...ah.com
Subject: Re: [PATCH net-next v5 1/5] rust: core abstractions for network PHY
 drivers

> I think Rust will make a big difference:
> - you cannot access data protected by a lock without locking the
>    lock beforehand.
> - you cannot forget to unlock a lock.

It is going to be interesting to look at this in 5 to 10 years time.
By then we hopefully have Rust drivers in subsystems which do the
locking in the core and those which leave it to the drivers. We can
then see if Rust written drivers which have to handle locking do
better than C drivers, or is it still better to do it all in the core.

> >> We already have exclusive access to the `phy_device`, so in Rust
> >> you would not need to lock anything to also have exclusive access to the
> >> embedded `mii_bus`.
> > 
> > I would actually say its not the PHY drivers problem at all. The
> > mii_bus is a property of the MDIO layers, and it is the MDIO layers
> > problem to impose whatever locking it needs for its properties.
> 
> Since the MDIO layer would provide its own serialization, in Rust
> we would not protect the `mdio_device` with a lock. In this case
> it could just be a coincidence that both locks are locked, since
> IIUC `phy_device` is locked whenever callbacks are called.
> 
> > Also, mii_bus is not embedded. Its a pointer to an mii_bus. The phy
> > lock protects the pointer. But its the MDIO layer which needs to
> > protect what the pointer points to.
> 
> Oh I overlooked the `*`. Then it depends what type of pointer that is,
> is the `mii_bus` unique or is it shared? If it is shared, then Rust
> would also need another lock there.

There can be up to 32 PHY drivers using one mii_bus, but in practice
you never get this density. Because there can be multiple PHYs this is
why the mii_bus has a lock, to serialise accesses from those PHYs to
the bus.

And MDIO is to some extend a generic bus. Not everything on an MII bus
is a PHY. Some Ethernet switches are MDIO devices, and they often take
up multiple addresses on the bus. But the locking is all the same.

PHYLIB core holds a reference to the MII bus, so the bus is not going
to go away before the PHY goes away. This is all standard Linux
bus/clients locking. It gets a bit messy with hot-plug, devices like
USB devices. The physical hardware can disappear at any time, but the
software representation stays around until it gets cleaned up in a
controlled manor. So a read/write on a bus can fail because its
physically gone, but you don't have to worry about the mii_bus
structure disappearing.

    Andrew