8000 LIST: rust: add improved version of `ForeignOwnable::borrow_mut` · Rust-for-Linux/linux@61c3a60 · GitHub
[go: up one dir, main page]
Skip to content

Commit 61c3a60

Browse files
Darksonnmetaspace
Darksonn
authored and
metaspace
committed
LIST: rust: add improved version of ForeignOwnable::borrow_mut
Previously, the `ForeignOwnable` trait had a method called `borrow_mut` that was intended to provide mutable access to the inner value. However, the method accidentally made it possible to change the address of the object being modified, which usually isn't what we want. (And when we want that, it can be done by calling `from_foreign` and `into_foreign`, like how the old `borrow_mut` was implemented.) In this patch, we introduce an alternate definition of `borrow_mut` that solves the previous problem. Conceptually, given a pointer type `P` that implements `ForeignOwnable`, the `borrow_mut` method gives you the same kind of access as an `&mut P` would, except that it does not let you change the pointer `P` itself. This is analogous to how the existing `borrow` method provides the same kind of access to the inner value as an `&P`. Note that for types like `Arc`, having an `&mut Arc<T>` only gives you immutable access to the inner `T`. This is because mutable references assume exclusive access, but there might be other handles to the same reference counted value, so the access isn't exclusive. The `Arc` type implements this by making `borrow_mut` return the same type as `borrow`. Signed-off-by: Alice Ryhl <aliceryhl@google.com> Reviewed-by: Boqun Feng <boqun.feng@gmail.com> Reviewed-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> --- Source: https://lore.kernel.org/all/20230710074642.683831-1-aliceryhl@google.com/
1 parent 4855a78 commit 61c3a60

File tree

2 files changed

+83
-29
lines changed

2 files changed

+83
-29
lines changed

‎rust/kernel/sync/arc.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -277,26 +277,35 @@ impl<T: ?Sized> Arc<T> {
277277

278278
impl<T: 'static> ForeignOwnable for Arc<T> {
279279
type Borrowed<'a> = ArcBorrow<'a, T>;
280+
// Mutable access to the `Arc` does not give any extra abilities over
281+
// immutable access.
282+
type BorrowedMut<'a> = ArcBorrow<'a, T>;
280283

281284
fn into_foreign(self) -> *const core::ffi::c_void {
282285
ManuallyDrop::new(self).ptr.as_ptr() as _
283286
}
284287

288+
unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self {
289+
// SAFETY: By the safety requirement of this function, we know that `ptr` came from
290+
// a previous call to `Arc::into_foreign`, which guarantees that `ptr` is valid and
291+
// holds a reference count increment that is transferrable to us.
292+
unsafe { Self::from_inner(NonNull::new_unchecked(ptr as _)) }
293+
}
294+
285295
unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> {
286296
// SAFETY: By the safety requirement of this function, we know that `ptr` came from
287297
// a previous call to `Arc::into_foreign`.
288-
let inner = NonNull::new(ptr as *mut ArcInner<T>).unwrap();
298+
let inner = unsafe { NonNull::new_unchecked(ptr as *mut ArcInner<T>) };
289299

290-
// SAFETY: The safety requirements of `from_foreign` ensure that the object remains alive
291-
// for the lifetime of the returned value.
300+
// SAFETY: The safety requirements ensure that we will not give up our
301+
// foreign-owned refcount while the `ArcBorrow` is still live.
292302
unsafe { ArcBorrow::new(inner) }
293303
}
294304

295-
unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self {
296-
// SAFETY: By the safety requirement of this function, we know that `ptr` came from
297-
// a previous call to `Arc::into_foreign`, which guarantees that `ptr` is valid and
298-
// holds a reference count increment that is transferrable to us.
299-
unsafe { Self::from_inner(NonNull::new(ptr as _).unwrap()) }
305+
unsafe fn borrow_mut<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> {
306+
// SAFETY: The safety requirements for `borrow_mut` are a superset of the safety
307+
// requirements for `borrow`.
308+
unsafe { Self::borrow(ptr) }
300309
}
301310
}
302311

‎rust/kernel/types.rs

Lines changed: 66 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,66 +20,111 @@ use core::{
2020
/// This trait is meant to be used in cases when Rust objects are stored in C objects and
2121
/// eventually "freed" back to Rust.
2222
pub trait ForeignOwnable: Sized {
23-
/// Type of values borrowed between calls to [`ForeignOwnable::into_foreign`] and
24-
/// [`ForeignOwnable::from_foreign`].
23+
/// Type used to immutably borrow a value that is currently foreign-owned.
2524
type Borrowed<'a>;
2625

26+
/// Type used to mutably borrow a value that is currently foreign-owned.
27+
type BorrowedMut<'a>;
28+
2729
/// Converts a Rust-owned object to a foreign-owned one.
2830
///
2931
/// The foreign representation is a pointer to void.
3032
fn into_foreign(self) -> *const core::ffi::c_void;
3133

32-
/// Borrows a foreign-owned object.
34+
/// Converts a foreign-owned object back to a Rust-owned one.
35+
///
36+
/// # Safety
37+
///
38+
/// The provided pointer must have been returned by a previous call to [`into_foreign`], and it
39+
/// must not be passed to `from_foreign` more than once.
40+
///
41+
/// [`into_foreign`]: Self::into_foreign
42+
unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
43+
44+
/// Borrows a foreign-owned object immutably.
45+
///
46+
/// This method provides a way to access a foreign-owned value from Rust immutably. It provides
47+
/// you with exactly the same abilities as an `&Self` when the value is Rust-owned.
3348
///
3449
/// # Safety
3550
///
36-
/// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
37-
/// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
51+
/// The provided pointer must have been returned by a previous call to [`into_foreign`], and if
52+
/// the pointer is ever passed to [`from_foreign`], then that call must happen after the end of
53+
/// the lifetime 'a.
54+
///
55+
/// [`into_foreign`]: Self::into_foreign
56+
/// [`from_foreign`]: Self::from_foreign
3857
unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> Self::Borrowed<'a>;
3958

40-
/// Converts a foreign-owned object back to a Rust-owned one.
59+
/// Borrows a foreign-owned object mutably.
60+
///
61+
/// This method provides a way to access a foreign-owned value from Rust mutably. It provides
62+
/// you with exactly the same abilities as an `&mut Self` when the value is Rust-owned, except
63+
/// that this method does not let you swap the foreign-owned object for another. (That is, it
64+
/// does not let you change the address of the void pointer that the foreign code is storing.)
65+
///
66+
/// Note that for types like [`Arc`], an `&mut Arc<T>` only gives you immutable access to the
67+
/// inner value, so this method also only provides immutable access in that case.
68+
///
69+
/// In the case of `Box<T>`, this method gives you the ability to modify the inner `T`, but it
70+
/// does not let you change the box itself. That is, you cannot change which allocation the box
71+
/// points at.
4172
///
4273
/// # Safety
4374
///
44-
/// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
45-
/// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
46-
/// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] for
47-
/// this object must have been dropped.
48-
unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
75+
/// The provided pointer must have been returned by a previous call to [`into_foreign`], and if
76+
/// the pointer is ever passed to [`from_foreign`], then that call must happen after the end of
77+
/// the lifetime 'a.
78+
///
79+
/// The lifetime 'a must not overlap with the lifetime of any other call to [`borrow`] or
80+
/// `borrow_mut` on the same object.
81+
///
82+
/// [`into_foreign`]: Self::into_foreign
83+
/// [`from_foreign`]: Self::from_foreign
84+
/// [`borrow`]: Self::borrow
85+
/// [`Arc`]: crate::sync::Arc
86+
unsafe fn borrow_mut<'a>(ptr: *const core::ffi::c_void) -> Self::BorrowedMut<'a>;
4987
}
5088

5189
impl<T: 'static> ForeignOwnable for Box<T> {
5290
type Borrowed<'a> = &'a T;
91+
type BorrowedMut<'a> = &'a mut T;
5392

5493
fn into_foreign(self) -> *const core::ffi::c_void {
5594
Box::into_raw(self) as _
5695
}
5796

58-
unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> &'a T {
59-
// SAFETY: The safety requirements for this function ensure that the object is still alive,
60-
// so it is safe to dereference the raw pointer.
61-
// The safety requirements of `from_foreign` also ensure that the object remains alive for
62-
// the lifetime of the returned value.
63-
unsafe { &*ptr.cast() }
64-
}
65-
6697
unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self {
6798
// SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
6899
// call to `Self::into_foreign`.
69100
unsafe { Box::from_raw(ptr as _) }
70101
}
102+
103+
unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> &'a T {
104+
// SAFETY: The safety requirements of this method ensure that the object remains alive and
105+
// immutable for the duration of 'a.
106+
unsafe { &*ptr.cast() }
107+
}
108+
109+
unsafe fn borrow_mut<'a>(ptr: *const core::ffi::c_void) -> &'a mut T {
110+
// SAFETY: The safety requirements of this method ensure that the pointer is valid and that
111+
// nothing else will access the value for the duration of 'a.
112+
unsafe { &mut *ptr.cast_mut().cast() }
113+
}
71114
}
72115

73116
impl ForeignOwnable for () {
74117
type Borrowed<'a> = ();
118+
type BorrowedMut<'a> = ();
75119

76120
fn into_foreign(self) -> *const core::ffi::c_void {
77121
core::ptr::NonNull::dangling().as_ptr()
78122
}
79123

80-
unsafe fn borrow<'a>(_: *const core::ffi::c_void) -> Self::Borrowed<'a> {}
81-
82124
unsafe fn from_foreign(_: *const core::ffi::c_void) -> Self {}
125+
126+
unsafe fn borrow<'a>(_: *const core::ffi::c_void) -> Self::Borrowed<'a> {}
127+
unsafe fn borrow_mut<'a>(_: *const core::ffi::c_void) -> Self::BorrowedMut<'a> {}
83128
}
84129

85130
/// Runs a cleanup function/closure when dropped.

0 commit comments

Comments
 (0)
0