From patchwork Fri Dec 13 19:55:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tamir Duberstein X-Patchwork-Id: 13907840 Received: from mail-qt1-f173.google.com (mail-qt1-f173.google.com [209.85.160.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DD5A9193436; Fri, 13 Dec 2024 20:00:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.173 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734120053; cv=none; b=aAMqnAdi8X9W/JOQcf95NA4lUldFP91y6QaOFVnwmkIY4bEIWqrsafpW28guxOE7IbdIwqDx56VLC2bScPn4RgsiS99U0cCin9sQKG68wPsv1Mul9j7oqi7B9EwgWRPqTi/TV3NOYVn2NID6FE4h0YzAqbrb8/c7TEJ247A2fko= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734120053; c=relaxed/simple; bh=Bbpg/7UMPj5te+dk/j85/v3rHU/1o0zpC2YnON+XL04=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=U4Ux/XGUDjVgCI4bRP9knURntt6slwvTlaprj8oswwYnbK1kbLhoMclHwaiF6Trg9vvEmICinOSJRSriO3ht2j3mdTr8aJjK1XUtzbSXvq5wkoIrcWPyY5U+Ef/ehsvaJ5UoM4ZTAk7xN6GHAoB8cJ6iSozIgyayma8xcNI35OE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=i5nnWS/m; arc=none smtp.client-ip=209.85.160.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="i5nnWS/m" Received: by mail-qt1-f173.google.com with SMTP id d75a77b69052e-46769b34cbfso39429911cf.0; Fri, 13 Dec 2024 12:00:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1734120051; x=1734724851; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=X6r2J9Pe21NUB0VM4JX+T13WMqxEcL57MxKGDLLt49w=; b=i5nnWS/mnx1wx7pat2hKgDoHhQwJ9Smx+5r2DcFzoYwfHprZTxQG8chQjnw2Fl7T5I EK4v1S3jaDQawgLigll0IriggOSWQXGNkArDkI7u/5E0tuHYUnweSOkUpQPaQ9k4om0I jTpf16pXItbMhT/JX8Y7wZJ4or5up7fAKJVGCC2UQx5hBJ9cP17eZBaZBS18OmOBjRrB FDhGY3/hITNlT1guDRJeip94YAt5z9dJcxbg/NhfT2MDSN40x8jLwswTpezHcsDlwenX YMx1Rtj4/U2cQLoVYLLaoKPJ3z9jnEcfJVSl936bpUD2zzg0u/WmxjDJOGQ1javWtXT8 NDOg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734120051; x=1734724851; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=X6r2J9Pe21NUB0VM4JX+T13WMqxEcL57MxKGDLLt49w=; b=amopXGxSoiQ8LboQgpxB/Uz5rS47gZCQk2CII0IWFMv9nP0B4qh41oyyEtDc6pMQQe Cxf4/gUI6D5xf62x8xqMz4rCJ4YlscLpQdDjUKGCjHtJ42QoYE1wyIMSAuAwy00Hwi7c Jhyc9YNHDf7EleIp19KR0Ch+kD0bGYIqZtNsWYmuMA8vhpHmDNhsqBmzKO+TjcmGhbgI V5jHJkF6k8LRvDWw9BkI/33gd55b4tK9WP/NqziOcMCIXJEo+jBXytahRDxaD3SfunOi gZoUzWIJU4BHHS01tCJKl9nh15adSgmRtXvj3w9QicAHGJpwdXrHWRzx3PIoi9EX03l+ mRag== X-Forwarded-Encrypted: i=1; AJvYcCU1eZ4qtyKe/fHLNCjCwzqaiQObIxHy+JQB5Lq5KoleYUUiphAXBDZdWNNK/6hI5BKUaTGHV2NEL0NY61RI@vger.kernel.org, AJvYcCX81zVNcLI7CrWVjkW6vflotDiHOwRz3nf1NwyDmdODakc7kplD+WFDSAvI2iMcV7XRd65YDwUt77F2P6Yp@vger.kernel.org, AJvYcCXrXo+G55lvTpvEkh8Kz3XbOsqY7mHrasJNF2SVO2I+qUDfgFk+AwDrx1GNn9N4uBTzJv3JazMQWsQFK8DlMPg=@vger.kernel.org X-Gm-Message-State: AOJu0YxCECnKWQXqm7dkDJO71Yobp+pM9ot5UCV9kM++1BSZd9rLr44G UwqKWZKL8GCydtOcmxlqq9E+lw2N8DhPKY7CATfys3IJeRNP2XqGB9ZbNYc7 X-Gm-Gg: ASbGncsmC+Jt8qSiyTZYNFYcBx0T7y4Ugr76o0UoXBiJeRNmRP6NNIYqPXyH7fsH+SI asbXf5XKuOgKjWQF8kkINcFnTyrJarBJr1KJpYUVmVNsjRDDbK+2UCrO3swakJ8887r4Kv5rGOo l5d553GkTe+XjCbhp1FHu6xG/LkmOh9E14O2HCGKNFSAhG+bP1LW8u842bOtqsm3SoXqaG/g6fM kaDGlrUzwqNdsmvS/silJ4W02lwE7en+TQ/J7imhyyzPuYa8HT7AHP42dx2ildLWKJwFwjwG8oc bXBMT9YduZQsD5KAewk2bHhkbyxD8lbd/JIwPPB9WUMI+bo8IIxtUeyQb1Np X-Google-Smtp-Source: AGHT+IH1u2x5xjOFu8/NYjekhZ+f+breAozeCQAT8WxY03eVS7il7RkK21bYeo+11Hhnaa4F65rl3A== X-Received: by 2002:a05:620a:2b44:b0:7b1:4fba:b02e with SMTP id af79cd13be357-7b6fbecc681mr698581585a.12.1734119738220; Fri, 13 Dec 2024 11:55:38 -0800 (PST) Received: from 1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa ([2620:10d:c091:600::1:e1cd]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-467b2ca087esm1047261cf.28.2024.12.13.11.55.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Dec 2024 11:55:37 -0800 (PST) From: Tamir Duberstein Date: Fri, 13 Dec 2024 14:55:17 -0500 Subject: [PATCH v13 1/2] rust: types: add `ForeignOwnable::PointedTo` Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20241213-rust-xarray-bindings-v13-1-8655164e624f@gmail.com> References: <20241213-rust-xarray-bindings-v13-0-8655164e624f@gmail.com> In-Reply-To: <20241213-rust-xarray-bindings-v13-0-8655164e624f@gmail.com> To: Danilo Krummrich , Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Alice Ryhl , Trevor Gross , Matthew Wilcox Cc: =?utf-8?q?Ma=C3=ADra_Canal?= , Asahi Lina , rust-for-linux@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Tamir Duberstein X-Mailer: b4 0.15-dev Allow implementors to specify the foreign pointer type; this exposes information about the pointed-to type such as its alignment. This requires the trait to be `unsafe` since it is now possible for implementors to break soundness by returning a misaligned pointer. Encoding the pointer type in the trait (and avoiding pointer casts) allows the compiler to check that implementors return the correct pointer type. This is preferable to directly encoding the alignment in the trait using a constant as the compiler would be unable to check it. Reviewed-by: Andreas Hindborg Signed-off-by: Tamir Duberstein --- rust/kernel/alloc/kbox.rs | 38 ++++++++++++++++++++------------------ rust/kernel/miscdevice.rs | 6 +++--- rust/kernel/sync/arc.rs | 21 ++++++++++++--------- rust/kernel/types.rs | 46 +++++++++++++++++++++++++++++++--------------- 4 files changed, 66 insertions(+), 45 deletions(-) diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs index 4ffc4e1b22b2b7c2ea8e8ed5b7f7a8534625249f..4e7a0ce9cc9c24f2e828f41e9105acc4048333d5 100644 --- a/rust/kernel/alloc/kbox.rs +++ b/rust/kernel/alloc/kbox.rs @@ -349,68 +349,70 @@ fn try_init(init: impl Init, flags: Flags) -> Result } } -impl ForeignOwnable for Box +// SAFETY: The `into_foreign` function returns a pointer that is well-aligned. +unsafe impl ForeignOwnable for Box where A: Allocator, { + type PointedTo = T; type Borrowed<'a> = &'a T; type BorrowedMut<'a> = &'a mut T; - fn into_foreign(self) -> *mut crate::ffi::c_void { - Box::into_raw(self).cast() + fn into_foreign(self) -> *mut Self::PointedTo { + Box::into_raw(self) } - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self { // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous // call to `Self::into_foreign`. - unsafe { Box::from_raw(ptr.cast()) } + unsafe { Box::from_raw(ptr) } } - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> &'a T { + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> &'a T { // SAFETY: The safety requirements of this method ensure that the object remains alive and // immutable for the duration of 'a. - unsafe { &*ptr.cast() } + unsafe { &*ptr } } - unsafe fn borrow_mut<'a>(ptr: *mut core::ffi::c_void) -> &'a mut T { - let ptr = ptr.cast(); + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> &'a mut T { // SAFETY: The safety requirements of this method ensure that the pointer is valid and that // nothing else will access the value for the duration of 'a. unsafe { &mut *ptr } } } -impl ForeignOwnable for Pin> +// SAFETY: The `into_foreign` function returns a pointer that is well-aligned. +unsafe impl ForeignOwnable for Pin> where A: Allocator, { + type PointedTo = T; type Borrowed<'a> = Pin<&'a T>; type BorrowedMut<'a> = Pin<&'a mut T>; - fn into_foreign(self) -> *mut crate::ffi::c_void { + fn into_foreign(self) -> *mut Self::PointedTo { // SAFETY: We are still treating the box as pinned. - Box::into_raw(unsafe { Pin::into_inner_unchecked(self) }).cast() + Box::into_raw(unsafe { Pin::into_inner_unchecked(self) }) } - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self { // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous // call to `Self::into_foreign`. - unsafe { Pin::new_unchecked(Box::from_raw(ptr.cast())) } + unsafe { Pin::new_unchecked(Box::from_raw(ptr)) } } - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Pin<&'a T> { + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> Pin<&'a T> { // SAFETY: The safety requirements for this function ensure that the object is still alive, // so it is safe to dereference the raw pointer. // The safety requirements of `from_foreign` also ensure that the object remains alive for // the lifetime of the returned value. - let r = unsafe { &*ptr.cast() }; + let r = unsafe { &*ptr }; // SAFETY: This pointer originates from a `Pin>`. unsafe { Pin::new_unchecked(r) } } - unsafe fn borrow_mut<'a>(ptr: *mut core::ffi::c_void) -> Pin<&'a mut T> { - let ptr = ptr.cast(); + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> Pin<&'a mut T> { // SAFETY: The safety requirements for this function ensure that the object is still alive, // so it is safe to dereference the raw pointer. // The safety requirements of `from_foreign` also ensure that the object remains alive for diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index e58807ad28dc644fa384e9c1fb41fd6e53abea7a..ac6b591a83ad558f12ce2746b54e7f76d8b99c6f 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -193,7 +193,7 @@ impl VtableHelper { }; // SAFETY: The open call of a file owns the private data. - unsafe { (*file).private_data = ptr.into_foreign() }; + unsafe { (*file).private_data = ptr.into_foreign().cast() }; 0 } @@ -209,7 +209,7 @@ impl VtableHelper { // SAFETY: The release call of a file owns the private data. let private = unsafe { (*file).private_data }; // SAFETY: The release call of a file owns the private data. - let ptr = unsafe { ::from_foreign(private) }; + let ptr = unsafe { ::from_foreign(private.cast()) }; T::release(ptr); @@ -227,7 +227,7 @@ impl VtableHelper { // SAFETY: The ioctl call of a file can access the private data. let private = unsafe { (*file).private_data }; // SAFETY: Ioctl calls can borrow the private data of the file. - let device = unsafe { ::borrow(private) }; + let device = unsafe { ::borrow(private.cast()) }; match T::ioctl(device, cmd, arg as usize) { Ok(ret) => ret as c_long, diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index eb5cd8b360a3507a527978aaf96dbc3a80d4ae2c..8e29c332db86ae869d81f75de9c21fa73174de9a 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -130,9 +130,10 @@ pub struct Arc { _p: PhantomData>, } +#[doc(hidden)] #[pin_data] #[repr(C)] -struct ArcInner { +pub struct ArcInner { refcount: Opaque, data: T, } @@ -330,18 +331,20 @@ pub fn into_unique_or_drop(self) -> Option>> { } } -impl ForeignOwnable for Arc { +// SAFETY: The `into_foreign` function returns a pointer that is well-aligned. +unsafe impl ForeignOwnable for Arc { + type PointedTo = ArcInner; type Borrowed<'a> = ArcBorrow<'a, T>; type BorrowedMut<'a> = Self::Borrowed<'a>; - fn into_foreign(self) -> *mut crate::ffi::c_void { - ManuallyDrop::new(self).ptr.as_ptr().cast() + fn into_foreign(self) -> *mut Self::PointedTo { + ManuallyDrop::new(self).ptr.as_ptr() } - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self { // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous // call to `Self::into_foreign`. - let inner = unsafe { NonNull::new_unchecked(ptr.cast::>()) }; + let inner = unsafe { NonNull::new_unchecked(ptr) }; // SAFETY: By the safety requirement of this function, we know that `ptr` came from // a previous call to `Arc::into_foreign`, which guarantees that `ptr` is valid and @@ -349,17 +352,17 @@ unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self { unsafe { Self::from_inner(inner) } } - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> ArcBorrow<'a, T> { + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> ArcBorrow<'a, T> { // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous // call to `Self::into_foreign`. - let inner = unsafe { NonNull::new_unchecked(ptr.cast::>()) }; + let inner = unsafe { NonNull::new_unchecked(ptr) }; // SAFETY: The safety requirements of `from_foreign` ensure that the object remains alive // for the lifetime of the returned value. unsafe { ArcBorrow::new(inner) } } - unsafe fn borrow_mut<'a>(ptr: *mut core::ffi::c_void) -> ArcBorrow<'a, T> { + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> ArcBorrow<'a, T> { // SAFETY: The safety requirements for `borrow_mut` are a superset of the safety // requirements for `borrow`. unsafe { Self::borrow(ptr) } diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 0dfaf45a755c7ce702027918e5fd3e97c407fda4..0cf93c293b884004a6ed64c2c09723efa7986270 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -18,7 +18,19 @@ /// /// This trait is meant to be used in cases when Rust objects are stored in C objects and /// eventually "freed" back to Rust. -pub trait ForeignOwnable: Sized { +/// +/// # Safety +/// +/// Implementers must ensure that [`into_foreign`] returns a pointer which meets the alignment +/// requirements of [`PointedTo`]. +/// +/// [`into_foreign`]: Self::into_foreign +/// [`PointedTo`]: Self::PointedTo +pub unsafe trait ForeignOwnable: Sized { + /// Type used when the value is foreign-owned. In practical terms only defines the alignment of + /// the pointer. + type PointedTo; + /// Type used to immutably borrow a value that is currently foreign-owned. type Borrowed<'a>; @@ -27,16 +39,18 @@ pub trait ForeignOwnable: Sized { /// Converts a Rust-owned object to a foreign-owned one. /// - /// The foreign representation is a pointer to void. There are no guarantees for this pointer. - /// For example, it might be invalid, dangling or pointing to uninitialized memory. Using it in - /// any way except for [`from_foreign`], [`try_from_foreign`], [`borrow`], or [`borrow_mut`] can - /// result in undefined behavior. + /// # Guarantees + /// + /// The return value is guaranteed to be well-aligned, but there are no other guarantees for + /// this pointer. For example, it might be null, dangling, or point to uninitialized memory. + /// Using it in any way except for [`ForeignOwnable::from_foreign`], [`ForeignOwnable::borrow`], + /// [`ForeignOwnable::try_from_foreign`] can result in undefined behavior. /// /// [`from_foreign`]: Self::from_foreign /// [`try_from_foreign`]: Self::try_from_foreign /// [`borrow`]: Self::borrow /// [`borrow_mut`]: Self::borrow_mut - fn into_foreign(self) -> *mut crate::ffi::c_void; + fn into_foreign(self) -> *mut Self::PointedTo; /// Converts a foreign-owned object back to a Rust-owned one. /// @@ -46,7 +60,7 @@ pub trait ForeignOwnable: Sized { /// must not be passed to `from_foreign` more than once. /// /// [`into_foreign`]: Self::into_foreign - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self; + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self; /// Tries to convert a foreign-owned object back to a Rust-owned one. /// @@ -58,7 +72,7 @@ pub trait ForeignOwnable: Sized { /// `ptr` must either be null or satisfy the safety requirements for [`from_foreign`]. /// /// [`from_foreign`]: Self::from_foreign - unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option { + unsafe fn try_from_foreign(ptr: *mut Self::PointedTo) -> Option { if ptr.is_null() { None } else { @@ -81,7 +95,7 @@ unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option { /// /// [`into_foreign`]: Self::into_foreign /// [`from_foreign`]: Self::from_foreign - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Self::Borrowed<'a>; + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> Self::Borrowed<'a>; /// Borrows a foreign-owned object mutably. /// @@ -109,21 +123,23 @@ unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option { /// [`from_foreign`]: Self::from_foreign /// [`borrow`]: Self::borrow /// [`Arc`]: crate::sync::Arc - unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> Self::BorrowedMut<'a>; + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> Self::BorrowedMut<'a>; } -impl ForeignOwnable for () { +// SAFETY: The `into_foreign` function returns a pointer that is dangling, but well-aligned. +unsafe impl ForeignOwnable for () { + type PointedTo = (); type Borrowed<'a> = (); type BorrowedMut<'a> = (); - fn into_foreign(self) -> *mut crate::ffi::c_void { + fn into_foreign(self) -> *mut Self::PointedTo { core::ptr::NonNull::dangling().as_ptr() } - unsafe fn from_foreign(_: *mut crate::ffi::c_void) -> Self {} + unsafe fn from_foreign(_: *mut Self::PointedTo) -> Self {} - unsafe fn borrow<'a>(_: *mut crate::ffi::c_void) -> Self::Borrowed<'a> {} - unsafe fn borrow_mut<'a>(_: *mut crate::ffi::c_void) -> Self::BorrowedMut<'a> {} + unsafe fn borrow<'a>(_: *mut Self::PointedTo) -> Self::Borrowed<'a> {} + unsafe fn borrow_mut<'a>(_: *mut Self::PointedTo) -> Self::BorrowedMut<'a> {} } /// Runs a cleanup function/closure when dropped.