History

rust: kernel: 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`.

Alice Ryhl committed 1 year ago

Commit c27e7051 parent c6b97533 files · +86 −13Browse files

Unified · Split

3 files
  1. rust/kernel/alloc/kbox.rs +21 −0
  2. rust/kernel/sync/arc.rs +7 −0
  3. rust/kernel/types.rs +58 −13
rust/kernel/alloc/kbox.rs+21−0
@@ -354,6 +354,7 @@ where · 353 unmodified lines
354 A: Allocator,354 A: Allocator,
355{355{
356 type Borrowed<'a> = &'a T;356 type Borrowed<'a> = &'a T;
357 type BorrowedMut<'a> = &'a mut T;
357 358
358 fn into_foreign(self) -> *mut crate::ffi::c_void {359 fn into_foreign(self) -> *mut crate::ffi::c_void {
359 Box::into_raw(self).cast()360 Box::into_raw(self).cast()
@@ -370,6 +371,13 @@ where · 10 unmodified lines
370 // immutable for the duration of 'a.371 // immutable for the duration of 'a.
371 unsafe { &*ptr.cast() }372 unsafe { &*ptr.cast() }
372 }373 }
374
375 unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> &'a mut T {
376 let ptr = ptr.cast();
377 // SAFETY: The safety requirements of this method ensure that the pointer is valid and that
378 // nothing else will access the value for the duration of 'a.
379 unsafe { &mut *ptr }
380 }
373}381}
374 382
375impl<T: 'static, A> ForeignOwnable for Pin<Box<T, A>>383impl<T: 'static, A> ForeignOwnable for Pin<Box<T, A>>
@@ -377,6 +385,7 @@ where · 1 unmodified line
377 A: Allocator,385 A: Allocator,
378{386{
379 type Borrowed<'a> = Pin<&'a T>;387 type Borrowed<'a> = Pin<&'a T>;
388 type BorrowedMut<'a> = Pin<&'a mut T>;
380 389
381 fn into_foreign(self) -> *mut crate::ffi::c_void {390 fn into_foreign(self) -> *mut crate::ffi::c_void {
382 // SAFETY: We are still treating the box as pinned.391 // SAFETY: We are still treating the box as pinned.
@@ -399,6 +408,18 @@ where · 16 unmodified lines
399 // SAFETY: This pointer originates from a `Pin<Box<T>>`.408 // SAFETY: This pointer originates from a `Pin<Box<T>>`.
400 unsafe { Pin::new_unchecked(r) }409 unsafe { Pin::new_unchecked(r) }
401 }410 }
411
412 unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> Pin<&'a mut T> {
413 let ptr = ptr.cast();
414 // SAFETY: The safety requirements for this function ensure that the object is still alive,
415 // so it is safe to dereference the raw pointer.
416 // The safety requirements of `from_foreign` also ensure that the object remains alive for
417 // the lifetime of the returned value.
418 let r = unsafe { &mut *ptr };
419
420 // SAFETY: This pointer originates from a `Pin<Box<T>>`.
421 unsafe { Pin::new_unchecked(r) }
422 }
402}423}
403 424
404impl<T, A> Deref for Box<T, A>425impl<T, A> Deref for Box<T, A>
rust/kernel/sync/arc.rs+7−0
@@ -344,6 +344,7 @@ impl<T: ?Sized> Arc<T> { · 343 unmodified lines
344 344
345impl<T: 'static> ForeignOwnable for Arc<T> {345impl<T: 'static> ForeignOwnable for Arc<T> {
346 type Borrowed<'a> = ArcBorrow<'a, T>;346 type Borrowed<'a> = ArcBorrow<'a, T>;
347 type BorrowedMut<'a> = Self::Borrowed<'a>;
347 348
348 fn into_foreign(self) -> *mut crate::ffi::c_void {349 fn into_foreign(self) -> *mut crate::ffi::c_void {
349 ManuallyDrop::new(self).ptr.as_ptr().cast()350 ManuallyDrop::new(self).ptr.as_ptr().cast()
@@ -369,6 +370,12 @@ impl<T: 'static> ForeignOwnable for Arc<T> { · 19 unmodified lines
369 // for the lifetime of the returned value.370 // for the lifetime of the returned value.
370 unsafe { ArcBorrow::new(inner) }371 unsafe { ArcBorrow::new(inner) }
371 }372 }
373
374 unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> ArcBorrow<'a, T> {
375 // SAFETY: The safety requirements for `borrow_mut` are a superset of the safety
376 // requirements for `borrow`.
377 unsafe { Self::borrow(ptr) }
378 }
372}379}
373 380
374impl<T: ?Sized> Deref for Arc<T> {381impl<T: ?Sized> Deref for Arc<T> {
rust/kernel/types.rs+58−13
@@ -19,26 +19,33 @@ use core::{ · 18 unmodified lines
19/// This trait is meant to be used in cases when Rust objects are stored in C objects and19/// This trait is meant to be used in cases when Rust objects are stored in C objects and
20/// eventually "freed" back to Rust.20/// eventually "freed" back to Rust.
21pub trait ForeignOwnable: Sized {21pub trait ForeignOwnable: Sized {
22 /// Type of values borrowed between calls to [`ForeignOwnable::into_foreign`] and22 /// Type used to immutably borrow a value that is currently foreign-owned.
23 /// [`ForeignOwnable::from_foreign`].
24 type Borrowed<'a>;23 type Borrowed<'a>;
25 24
25 /// Type used to mutably borrow a value that is currently foreign-owned.
26 type BorrowedMut<'a>;
27
26 /// Converts a Rust-owned object to a foreign-owned one.28 /// Converts a Rust-owned object to a foreign-owned one.
27 ///29 ///
28 /// The foreign representation is a pointer to void. There are no guarantees for this pointer.30 /// The foreign representation is a pointer to void. There are no guarantees for this pointer.
29 /// For example, it might be invalid, dangling or pointing to uninitialized memory. Using it in31 /// For example, it might be invalid, dangling or pointing to uninitialized memory. Using it in
30 /// any way except for [`ForeignOwnable::from_foreign`], [`ForeignOwnable::borrow`],32 /// any way except for [`from_foreign`], [`try_from_foreign`], [`borrow`], or [`borrow_mut`] can
31 /// [`ForeignOwnable::try_from_foreign`] can result in undefined behavior.33 /// result in undefined behavior.
34 ///
35 /// [`from_foreign`]: Self::from_foreign
36 /// [`try_from_foreign`]: Self::try_from_foreign
37 /// [`borrow`]: Self::borrow
38 /// [`borrow_mut`]: Self::borrow_mut
32 fn into_foreign(self) -> *mut crate::ffi::c_void;39 fn into_foreign(self) -> *mut crate::ffi::c_void;
33 40
34 /// Converts a foreign-owned object back to a Rust-owned one.41 /// Converts a foreign-owned object back to a Rust-owned one.
35 ///42 ///
36 /// # Safety43 /// # Safety
37 ///44 ///
38 /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for45 /// The provided pointer must have been returned by a previous call to [`into_foreign`], and it
39 /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.46 /// must not be passed to `from_foreign` more than once.
40 /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] for47 ///
41 /// this object must have been dropped.48 /// [`into_foreign`]: Self::into_foreign
42 unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self;49 unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self;
43 50
44 /// Tries to convert a foreign-owned object back to a Rust-owned one.51 /// Tries to convert a foreign-owned object back to a Rust-owned one.
@@ -48,8 +55,9 @@ pub trait ForeignOwnable: Sized { · 3 unmodified lines
48 ///55 ///
49 /// # Safety56 /// # Safety
50 ///57 ///
51 /// `ptr` must either be null or satisfy the safety requirements for58 /// `ptr` must either be null or satisfy the safety requirements for [`from_foreign`].
52 /// [`ForeignOwnable::from_foreign`].59 ///
60 /// [`from_foreign`]: Self::from_foreign
53 unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option<Self> {61 unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option<Self> {
54 if ptr.is_null() {62 if ptr.is_null() {
55 None63 None
@@ -60,17 +68,53 @@ pub trait ForeignOwnable: Sized { · 4 unmodified lines
60 }68 }
61 }69 }
62 70
63 /// Borrows a foreign-owned object.71 /// Borrows a foreign-owned object immutably.
72 ///
73 /// This method provides a way to access a foreign-owned value from Rust immutably. It provides
74 /// you with exactly the same abilities as an `&Self` when the value is Rust-owned.
64 ///75 ///
65 /// # Safety76 /// # Safety
66 ///77 ///
67 /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for78 /// The provided pointer must have been returned by a previous call to [`into_foreign`], and if
68 /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.79 /// the pointer is ever passed to [`from_foreign`], then that call must happen after the end of
80 /// the lifetime 'a.
81 ///
82 /// [`into_foreign`]: Self::into_foreign
83 /// [`from_foreign`]: Self::from_foreign
69 unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Self::Borrowed<'a>;84 unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Self::Borrowed<'a>;
85
86 /// Borrows a foreign-owned object mutably.
87 ///
88 /// This method provides a way to access a foreign-owned value from Rust mutably. It provides
89 /// you with exactly the same abilities as an `&mut Self` when the value is Rust-owned, except
90 /// that the address of the object must not be changed.
91 ///
92 /// Note that for types like [`Arc`], an `&mut Arc<T>` only gives you immutable access to the
93 /// inner value, so this method also only provides immutable access in that case.
94 ///
95 /// In the case of `Box<T>`, this method gives you the ability to modify the inner `T`, but it
96 /// does not let you change the box itself. That is, you cannot change which allocation the box
97 /// points at.
98 ///
99 /// # Safety
100 ///
101 /// The provided pointer must have been returned by a previous call to [`into_foreign`], and if
102 /// the pointer is ever passed to [`from_foreign`], then that call must happen after the end of
103 /// the lifetime 'a.
104 ///
105 /// The lifetime 'a must not overlap with the lifetime of any other call to [`borrow`] or
106 /// `borrow_mut` on the same object.
107 ///
108 /// [`into_foreign`]: Self::into_foreign
109 /// [`from_foreign`]: Self::from_foreign
110 /// [`borrow`]: Self::borrow
111 /// [`Arc`]: crate::sync::Arc
112 unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> Self::BorrowedMut<'a>;
70}113}
71 114
72impl ForeignOwnable for () {115impl ForeignOwnable for () {
73 type Borrowed<'a> = ();116 type Borrowed<'a> = ();
117 type BorrowedMut<'a> = ();
74 118
75 fn into_foreign(self) -> *mut crate::ffi::c_void {119 fn into_foreign(self) -> *mut crate::ffi::c_void {
76 core::ptr::NonNull::dangling().as_ptr()120 core::ptr::NonNull::dangling().as_ptr()
@@ -79,6 +123,7 @@ impl ForeignOwnable for () { · 2 unmodified lines
79 unsafe fn from_foreign(_: *mut crate::ffi::c_void) -> Self {}123 unsafe fn from_foreign(_: *mut crate::ffi::c_void) -> Self {}
80 124
81 unsafe fn borrow<'a>(_: *mut crate::ffi::c_void) -> Self::Borrowed<'a> {}125 unsafe fn borrow<'a>(_: *mut crate::ffi::c_void) -> Self::Borrowed<'a> {}
126 unsafe fn borrow_mut<'a>(_: *mut crate::ffi::c_void) -> Self::BorrowedMut<'a> {}
82}127}
83 128
84/// Runs a cleanup function/closure when dropped.129/// Runs a cleanup function/closure when dropped.

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t