netdev - Re: [PATCH net-next v2 2/6] rust: net::phy support probe callback

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

Message-ID: <CAH5fLghBvd3phRWBzayTVQWyKqBYLNw9dsYKPmMHYZ1_jaWL7g@mail.gmail.com>
Date: Thu, 1 Aug 2024 11:07:52 +0200
From: Alice Ryhl <aliceryhl@...gle.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org, 
	rust-for-linux@...r.kernel.org, tmgross@...ch.edu, 
	miguel.ojeda.sandonis@...il.com, benno.lossin@...ton.me
Subject: Re: [PATCH net-next v2 2/6] rust: net::phy support probe callback

On Wed, Jul 31, 2024 at 10:57 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> > > > +    /// `phydev` must be passed by the corresponding callback in `phy_driver`.
> > > > +    unsafe extern "C" fn probe_callback(phydev: *mut bindings::phy_device) -> core::ffi::c_int {
> > > > +        from_result(|| {
> > > > +            // SAFETY: This callback is called only in contexts
> > > > +            // where we can exclusively access to `phy_device`, so the accessors on
> > > > +            // `Device` are okay to call.
> > >
> > > This one is slightly different to other callbacks. probe is called
> > > without the mutex. Instead, probe is called before the device is
> > > published. So the comment is correct, but given how important Rust
> > > people take these SAFETY comments, maybe it should indicate it is
> > > different to others?
> >
> > Interesting. Given that we don't hold the mutex, does that mean that
> > some of the methods on Device are not safe to call in this context? Or
> > is there something else that makes it okay to call them despite not
> > holding the mutex?
>
> probe is always the first method called on a device driver to match it
> to a device. Traditionally, if probe fails, the device is destroyed,
> since there is no driver to drive it. probe needs to complete
> successfully before the phy_device structure is published so a MAC
> driver can reference it. If it is not published, nothing can have
> access to it, so you don't need to worry about parallel activities on
> it.
>
> And a PHY driver does not need a probe function. Historically, probe
> was all about, can this driver drive this hardware. However, since we
> have ID registers in the hardware, we already know the driver can
> drive the hardware. So probe is now about setting up whatever needs
> setting up. For PHY drivers, there is often nothing, no local state
> needed, etc. So the probe is optional.

Makes sense. Thanks for the explanation!

Alice