netdev - Re: [PATCH net-next v7 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20231121.214732.541476521256381764.fujita.tomonori@gmail.com>
Date: Tue, 21 Nov 2023 21:47:32 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: aliceryhl@...gle.com
Cc: andrew@...n.ch, benno.lossin@...ton.me, fujita.tomonori@...il.com,
 miguel.ojeda.sandonis@...il.com, netdev@...r.kernel.org,
 rust-for-linux@...r.kernel.org, tmgross@...ch.edu, wedsonaf@...il.com
Subject: Re: [PATCH net-next v7 1/5] rust: core abstractions for network
 PHY drivers

On Fri, 17 Nov 2023 15:42:46 +0000
Alice Ryhl <aliceryhl@...gle.com> wrote:

> Anyway. If you don't want to write down the tribal knowledge here, then
> I suggest this simpler wording:
> 
> /// # Invariants
> ///
> /// Referencing a `phy_device` using this struct asserts that you are in
> /// a context where all methods defined on this struct are safe to call.
> #[repr(transparent)]
> pub struct Device(Opaque<bindings::phy_device>);
> 
> This invariant is much less precise, but at least it is correct.
> 
> Other safety comments may then be:
> 
> 	/// Gets the id of the PHY.
> 	pub fn phy_id(&self) -> u32 {
> 	    let phydev = self.0.get();
> 	    // SAFETY: The struct invariant ensures that we may access
> 	    // this field without additional synchronization.
> 	    unsafe { (*phydev).phy_id }
> 	}
> 
> And:
> 
> 	unsafe extern "C" fn soft_reset_callback(
> 	    phydev: *mut bindings::phy_device,
> 	) -> core::ffi::c_int {
> 	    from_result(|| {
> 	        // SAFETY: This callback is called only in contexts
> 		// where we hold `phy_device->lock`, so the accessors on
> 		// `Device` are okay to call.
> 	        let dev = unsafe { Device::from_raw(phydev) };
> 	        T::soft_reset(dev)?;
> 	        Ok(0)
> 	    })
> 	}
> 
> And:
> 
> 	unsafe extern "C" fn resume_callback(phydev: *mut bindings::phy_device) -> core::ffi::c_int {
> 	    from_result(|| {
> 	        // SAFETY: The C core code ensures that the accessors on
> 		// `Device` are okay to call even though `phy_device->lock`
> 		// might not be held.
> 	        let dev = unsafe { Device::from_raw(phydev) };
> 	        T::resume(dev)?;
> 	        Ok(0)
> 	    })
> 	}

With these comments, What I should write on from_raw() function as a
safety comment?

/// # Safety
///
/// For the duration of 'a, the pointer must point at a valid
/// `phy_device`, and the caller must ensure that an user of this struct
/// in a context where all methods defined on this struct are safe to
/// call.
unsafe fn from_raw<'a>(ptr: *mut bindings::phy_device) -> &'a mut Self {