netdev - Re: [PATCH net-next v1 2/4] rust: net::phy support C45 helpers

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: <CALNs47t0FDS59xckUV0QkozbX-RAs8U3woN_sBc0TVm8d=dKNA@mail.gmail.com>
Date: Mon, 15 Apr 2024 23:25:22 -0400
From: Trevor Gross <tmgross@...ch.edu>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: netdev@...r.kernel.org, andrew@...n.ch, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH net-next v1 2/4] rust: net::phy support C45 helpers

On Mon, Apr 15, 2024 at 6:47 AM FUJITA Tomonori
<fujita.tomonori@...il.com> wrote:
> +    /// Reads a given C45 PHY register.
> +    /// This function reads a hardware register and updates the stats so takes `&mut self`.
> +    pub fn c45_read(&mut self, devad: u8, regnum: u16) -> Result<u16> {
> +        let phydev = self.0.get();
> +        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Self`.
> +        // So it's just an FFI call.

Depending on the response to Andrew's notes, these SAFETY comments
will probably need to be updated to say why we know C45 is supported.

> +        let ret = unsafe {
> +            bindings::mdiobus_c45_read(
> +                (*phydev).mdio.bus,
> +                (*phydev).mdio.addr,
> +                devad as i32,

This could probably also be from/.into()

> +                regnum.into(),
> +            )
> +        };
> +        if ret < 0 {
> +            Err(Error::from_errno(ret))
> +        } else {
> +            Ok(ret as u16)
> +        }

Could this be simplified with to_result?

> +    }
> +
> +    /// Writes a given C45 PHY register.
> +    pub fn c45_write(&mut self, devad: u8, regnum: u16, val: u16) -> Result {
> +        let phydev = self.0.get();
> +        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Self`.
> +        // So it's just an FFI call.
> +        to_result(unsafe {
> +            bindings::mdiobus_c45_write(
> +                (*phydev).mdio.bus,
> +                (*phydev).mdio.addr,
> +                devad as i32,

Same as above

> +                regnum.into(),
> +                val,
> +            )
> +        })
> +    }
> +