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
| ||
|
Message-ID: <Y9WchuzdL8HBrqq7@boqun-archlinux> Date: Sat, 28 Jan 2023 14:07:02 -0800 From: Boqun Feng <boqun.feng@...il.com> To: Martin Rodriguez Reboredo <yakoyoku@...il.com> Cc: wedsonaf@...il.com, alex.gaynor@...il.com, bjorn3_gh@...tonmail.com, gary@...yguo.net, linux-kernel@...r.kernel.org, ojeda@...nel.org, rust-for-linux@...r.kernel.org Subject: Re: [PATCH 2/5] rust: types: introduce `ForeignOwnable` On Sat, Jan 28, 2023 at 05:46:50PM -0300, Martin Rodriguez Reboredo wrote: > On 1/28/23 14:05, Boqun Feng wrote: > > On Sat, Jan 28, 2023 at 11:53:45AM -0300, Martin Rodriguez Reboredo wrote: > > [...] > >>> + /// Borrows a foreign-owned object. > >>> + /// > >>> + /// # Safety > >>> + /// > >>> + /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for > >>> + /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet. > >>> + /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow_mut`] > >>> + /// for this object must have been dropped. > >>> + unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> Self::Borrowed<'a>; > >>> + > >>> + /// Mutably borrows a foreign-owned object. > >>> + /// > >>> + /// # Safety > >>> + /// > >>> + /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for > >>> + /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet. > >>> + /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and > >>> + /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped. > >>> + unsafe fn borrow_mut<T: ForeignOwnable>(ptr: *const core::ffi::c_void) -> ScopeGuard<T, fn(T)> { > >>> + // SAFETY: The safety requirements ensure that `ptr` came from a previous call to > >>> + // `into_foreign`. > >>> + ScopeGuard::new_with_data(unsafe { T::from_foreign(ptr) }, |d| { > >>> + d.into_foreign(); > >>> + }) > >>> + } > >> > >> Could these three methods have a borrowing equivalent? When I was > >> working on some features for the USB module I've stumbled upon the case > >> of having to encode a pointer (with a pivot) and I cannot do it without > >> taking ownership of the pointer. > >> > > > > *const T is Copy, so you can still use it after pass it to a function or > > a new binding, e.g. > > > > pub fn use_ptr(ptr: *const i32) { .. } > > > > let p: *const i32 = some_func(); > > > > let q = p; > > > > // q is just a copy of p > > use_ptr(p); > > // passing to a function parameter is just copying > > use_ptr(p); > > > > maybe I'm missing something subtle, but if you have an example I can > > help take a look. > > > > Regards, > > Boqun > > > > I'll use a much more simple example. If I want to take the byte offset > between two `ForeignWrapper`s I'd have to take ownership of them, but I > don't see it desirable in some cases. > > fn byte_offset<P: PointerWrapper>(ptr: P, pivot: P) -> isize { > unsafe { > ptr.into_pointer().cast::<u8>() > .byte_offset(pivot.into_pointer().cast()) > } > } > > But if there was an `as_pointer(&self) -> *const c_void` method then the > above function will be able to borrow both `ForeignWrapper`s. > > fn byte_offset<P: PointerWrapper>(ptr: &P, pivot: &P) -> isize { > unsafe { > ptr.as_pointer().cast::<u8>() > .byte_offset(pivot.as_pointer().cast()) > } > } > > Obviously those methods that borrow will announce invariancies in their > doc comments. If these can exist then great and if not then another > solution could be explored. > For this particular use case, IIUC, what you need is exactly `AsRef`: fn byte_offset<T, P: AsRef<T>>(ptr: &P, pivot: &P) -> isize { unsafe { (ptr.as_ref() as *const _).cast::<u8>() .byte_offset((pivot.as_ref() *const _).cast()) } } or you can implement a as_pointer() helper function to avoid the duplicate `.as_ref() as *const _`: fn as_pointer<T, P: AsRef<T>>(ptr: &P) -> *const T { ptr.as_ref() as *const T } Although, we need to `impl AsRef<T> for Arc<T>` to make it work for `Arc<T>`, which is currently missing. Regards, Boqun > >>> + > >>> + /// Converts a foreign-owned object back to a Rust-owned one. > >>> + /// > >>> + /// # Safety > >>> + /// > >>> + /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for > >>> + /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet. > >>> + /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and > >>> + /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped. > >>> + unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self; > >>> +} > >>> + > >>> /// Runs a cleanup function/closure when dropped. > >>> /// > >>> /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running. > >>> -- > >>> 2.34.1 > >> > >> Aside from these comments I observe that there's a possibility to make > >> ForeignOwnable a const trait and have non const implementors. Otherwise > >> if these things are out of scope, no problem whatsoever and this has my > >> OK. > >> > >> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@...il.com>
Powered by blists - more mailing lists